-
Notifications
You must be signed in to change notification settings - Fork 5.1k
Fixing performance regression in EventSource #117549
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.
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR improves performance by fetching EventMetadata
by reference instead of by value to avoid repeated struct copies and constructor invocations.
- Replaced direct
m_eventData[eventId]
indexing withCollectionsMarshal.GetValueRefOrNullRef
in exception handling and event serialization paths. - Updated
WriteEventWithRelatedActivityIdCore
,WriteEventVarargs
,SerializeEventArgs
,LogEventArgsMismatches
, andIsEnabledByDefault
to use reference-based metadata access.
src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventSource.cs
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventSource.cs
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventSource.cs
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventSource.cs
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventSource.cs
Show resolved
Hide resolved
Tagging subscribers to this area: @tarekgh, @tommcdon, @steveisok, @pjanotti |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
This PR is intended to close #117203 by fetching EventMetadata by reference instead of by value. Accessing the uninitialized TraceLoggingEventTypes field of EventMetadata structs will run a constructor; if the struct is accessed by value, this constructor runs repeatedly on WriteEventVarargs, in addition to the typical overhead of accessing by value vs by reference.
Below are performance benchmark test results on my local machine (Windows x64)
Microsoft.Extensions.Logging.EventSourceLogger
System.Diagnostics.Perf_EventSource
System.Diagnostics.Tracing.Perf_EventListener
Also closes dotnet/perf-autofiling-issues#58568