Skip to content

Conversation

@stephentoub
Copy link
Member

Description

There are a multitude of call sites to EventTrace.EasyTraceEvent that pass value type arguments as the param1/param2/param3 arguments, e.g.

EventTrace.EasyTraceEvent(EventTrace.Keyword.KeywordGraphics, EventTrace.Event.WClientScheduleRender, nextTickNeeded.TotalMilliseconds);

EventTrace.EasyTraceEvent(EventTrace.Keyword.KeywordGraphics | EventTrace.Keyword.KeywordPerf, EventTrace.Event.WClientUceNotifyPresent, _lastPresentationTime, (Int64)presentationResults);

These, however, are defined to take object, which means the arguments are boxed. And that boxing happens before we even check whether the provider is enabled, as the call sites aren't guarded by checks for IsEnabled. That ends up meaning we potentially do lots of boxing only to immediately find that the data isn't required as tracing is disabled or isn't at a sufficient level. The fix this commit provides is to make these tracing methods generic, such that nothing need be boxed until after we know tracing is enabled.

Customer Impact

Less allocation leading to less garbage collection leading to fewer pauses and hiccups in the user experience.

Regression

No

Testing

Just CI

Risk

Minimal. We're simply avoiding some unnecessary work by passing state as generics rather than as object.

There are a multitude of call sites to EventTrace.EasyTraceEvent that pass value type arguments as the param1/param2/param3 arguments.  These, however, are defined to take `object`, which means the arguments are boxed.  And that boxing happens before we even check whether the provider is enabled, as the call sites aren't guarded by checks for IsEnabled.  That ends up meaning we potentially do lots of boxing only to immediately find that the data isn't required as tracing is disabled or isn't at a sufficient level.  The fix this commit provides is to make these tracing methods generic, such that nothing need be boxed until after we know tracing is enabled.
@stephentoub stephentoub requested a review from a team as a code owner June 23, 2021 03:32
@ghost ghost added the PR metadata: Label to tag PRs, to facilitate with triage label Jun 23, 2021
@ghost ghost requested review from SamBent, fabiant3 and ryalanms June 23, 2021 03:32
@ryalanms ryalanms merged commit 9f77f68 into dotnet:main Aug 17, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Apr 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

PR metadata: Label to tag PRs, to facilitate with triage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants