Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf in LogMessage #4453

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

perf in LogMessage #4453

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
4351b0a
perf in LogMessage
SimonCropp Dec 26, 2024
173b29c
Merge branch 'main' into perf-in-LogMessage
SimonCropp Dec 27, 2024
006b903
Update Logger.cs
SimonCropp Dec 27, 2024
5ef3b84
.
SimonCropp Dec 27, 2024
72c59c4
Merge branch 'main' into perf-in-LogMessage
SimonCropp Dec 28, 2024
aeaa33e
Update Directory.Packages.props
SimonCropp Dec 28, 2024
d74b740
Merge branch 'main' into perf-in-LogMessage
Evangelink Dec 28, 2024
e135fb6
Merge branch 'main' into perf-in-LogMessage
Evangelink Dec 28, 2024
3de4e5a
Merge branch 'main' into perf-in-LogMessage
SimonCropp Dec 29, 2024
ac28c8f
Merge branch 'main' into perf-in-LogMessage
Youssef1313 Dec 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Directory.Packages.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
<PackageVersion Include="Microsoft.TestPlatform.ObjectModel" Version="$(MicrosoftNETTestSdkVersion)" />
<PackageVersion Include="Microsoft.TestPlatform.TranslationLayer" Version="$(MicrosoftNETTestSdkVersion)" />
<PackageVersion Include="Microsoft.WindowsAppSDK" Version="1.0.0" />
<PackageVersion Include="Polyfill" Version="7.5.0" />
<PackageVersion Include="Polyfill" Version="7.9.1" />
<PackageVersion Include="System.Threading.Tasks.Extensions" Version="$(SystemThreadingTasksExtensionsVersion)" />
<!-- CVE-2019-0820 -->
<PackageVersion Include="System.Diagnostics.TextWriterTraceListener" Version="4.3.0" />
Expand Down
5 changes: 2 additions & 3 deletions src/TestFramework/TestFramework/Logger.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,12 @@ public static void LogMessage(string format, params object?[] args)
? format
: string.Format(CultureInfo.InvariantCulture, format, args);

object?[] parameters = [message];
// Making sure all event handlers are called in sync on same thread.
foreach (Delegate invoker in OnLogMessage.GetInvocationList())
foreach (LogMessageHandler invoker in DelegatePolyfill.EnumerateInvocationList(OnLogMessage))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: we don't call Logger.LogMessage at all. I don't know how much consumers will be calling this, but it doesn't seem to be a hot path.

@Evangelink I'm not sure if there are "valid" use cases for this Logger class or not. If not, we should consider breaking in v4.

{
try
{
invoker.GetMethodInfo()!.Invoke(invoker.Target, parameters);
invoker(message);
}
catch (Exception)
{
Expand Down
Loading