Add a new CompilationVerbose ETW Keyword #25544
Conversation
| @@ -3081,7 +3083,7 @@ | |||
| symbol="MethodLoad_V2" message="$(string.RuntimePublisher.MethodLoad_V2EventMessage)"/> | |||
|
|
|||
| <event value="159" version="0" level="win:Verbose" template="R2RGetEntryPoint" | |||
There was a problem hiding this comment.
If we are going to start adding FooVerbose style keywords, do we want to also require that level is set to verbose? The issue that @mjsabby was running into earlier is that we are forcing people to set level = Verbose as soon as they want one area of verbose data regardless of whether they wanted verbose data in any other area. My thought is that it should be sufficient to put noisy events behind *Verbose keywords without any additional requirement on the verbosity level. My remaining concern down that route was whether it has a reasonable migration path for events we've already got or is there a different organizational scheme that works out more cleanly. I hadn't had time to do the research so the issue stalled. This feels like it is venturing into the same territory.
There was a problem hiding this comment.
Can't you turn different flags on at different levels? It's not currently the default in perfview but for specific scenarios that's something we could look at.
For this specific one, I think it being verbose in both regards is fine. I don't think we would ever recommend anyone turn this on anywhere. It's really for diagnostic purposes.
There was a problem hiding this comment.
We chatted offline and decided to change the name from CompilationVerbose to CompilationDiagnostic. It kicks the can down the road a bit on the broader verbosity discussion, but at least avoids muddying the water on the Verbose keyword naming and gets immediate needs unblocked.
Can't you turn different flags on at different levels?
Not within the same session, each provider is given a single level which is shared across all keywords. You could use multiple ETW sessions to do it, with the attendant increase in complexity.
There was a problem hiding this comment.
I think this is running into the same territory as what @mjsabby was running into. I originally was going to put this in the Compilation keyword, which has a bunch of events with level==Informational. The issue becomes that you can't enable those events in the CLR provider using the PerfView default without opting into these. Thus, we do need to do something to work around the fact that you can't enable lower verbosity events in the Compilation keyword without enabling this one.
One option is to have a "Diagnostic" provider, but that felt heavy to me and still doesn't provide the non-breaking change migration path. Thus, I landed here on a keyword. I'm happy to change the name to CompilationDiagnostic.
src/vm/ClrEtwAll.man
Outdated
| @@ -79,6 +79,8 @@ | |||
| message="$(string.RuntimePublisher.EventSourceKeywordMessage)" symbol="CLR_EVENTSOURCE_KEYWORD" /> | |||
| <keyword name="CompilationKeyword" mask="0x1000000000" | |||
| message="$(string.RuntimePublisher.CompilationKeywordMessage)" symbol="CLR_COMPILATION_KEYWORD" /> | |||
| <keyword name="CompilationVerboseKeyword" mask="0x2000000000" | |||
Add a new CompilationVerbose ETW keyword and move the R2RGetEntryPoint event under this keyword. This allows us to continue enabling Verbose events on the .NET runtime provider (which we've done for a long time) without significantly perturbing the results with this very verbose event.
The R2RGetEntryPoint event as well as other verbose compilation events, such as a tiered compilation counter hit event, can be added to this keyword in the future.
Fixes #25492