-
Notifications
You must be signed in to change notification settings - Fork 5k
Support disabling event tracing #110622
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
Support disabling event tracing #110622
Conversation
80f08f1
to
377fc28
Compare
We had a similar build issue when bringing up Android runtime builds on CoreCLR, at one point it disabled the FEATURE_EVENT_TRACE using: <_CoreClrBuildArg Include="-cmakeargs -DFEATURE_EVENT_TRACE=0"/> and that caused the following error: C:\git\runtime\src\coreclr\inc\profilepriv.h(388,34): error C2061: syntax error: identifier 'EventPipeProvider' [C:\git\runtime\artifacts\obj\coreclr\windows.x64.Debug\ide\debug\daccess\daccess.vcxproj] So looks like this PR will fix that issue! One thought, looks like the profilepriv.h introduce the typedef |
8041494
to
8d3e23d
Compare
8d3e23d
to
e08ca5c
Compare
e08ca5c
to
7586491
Compare
src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventPipe.Internal.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventPipe.cs
Outdated
Show resolved
Hide resolved
87f4651
to
b516f10
Compare
b516f10
to
e284106
Compare
@cshung I had one last small comment on one define usage. Once that is adjusted do you believe this PR is ready to go? |
Thanks for all the review! I will fix the last define usage.
From my perspective, it is ready to go. To goal of this PR is simply to allow people to experiment with turning off I believe the most recent "Experimentally switch off ..." commit showed that it is the case, so I think this PR acomplished its goal. Of course, we aren't going to merge that commit. |
e284106
to
bab902e
Compare
27995ad
to
eaf6248
Compare
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!
eaf6248
to
3bac234
Compare
3bac234
to
a824dee
Compare
These fixes make it possible to turn off
FEATURE_EVENT_TRACE
during build.@janvorli