perf in LogMessage#4453
perf in LogMessage#4453SimonCropp wants to merge 11 commits intomicrosoft:mainfrom SimonCropp:perf-in-LogMessage
Conversation
|
I have synced the Polyfill package. |
|
@Evangelink We need to add the updated version to https://github.com/dotnet/source-build-reference-packages/tree/main/src/textOnlyPackages/src/polyfill/ I'll take care of that. |
|
I opened dotnet/source-build-reference-packages#1105 |
| 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)) |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
@Youssef1313 want me to burn this PR, and submit another that marks it as obsolete?
There was a problem hiding this comment.
I'll let @Evangelink answer that on whether there are valid use cases for Logger.LogMessage or if people are using that API
I assume logging is in the hot path. so a few more lines for better oerf is justified
move over to using EnumerateInvocationList