[Mono] Add Mono Profiler events into EventPipe. #55264
Conversation
|
//CC @josalem |
Needed since instrumentation uses mono_profiler_raise_exception_clause used without wrapper and that in turn will cause incomplete stacks due to transitioning into native code without informing unwinder.
Adding new MONO_DIAGNOSTICS env variable that can include diagnostic specific configs as well as --diagnostic-ports, meaning that its possible to use that instead of DOTNET_DiagnosticPorts variable. It also add variable to set some mono profiler settings needed very early during startup to get GC alloc as well as exception clause checks. In order to set needed options early in process, EventPipe component calls a specific component_init method setting up needed config.
|
@lambdageek Detected an issue with the enable/disable callback that I need to adjust. |
There was a problem hiding this comment.
CC @brianrob since this is rather major change to the manifest. I'd like his input before we merge if possible.
The changes look good to me, but I can't really comment on the Mono end of things (looks like Aleksey already went through that though). That being said, I don't think manually reading through ~7k lines of changes will catch all issues. If possible, I'd like to see some tests added before we snap .net6 for RC.
The majority of changes is in ep-rt-mono.c and is mainly not new changes, its mainly GIT not being able to trace injections in the beginning of the file together with changes in the middle and then some extensions to the end. @lambdageek has already gone through the changes in that file. We would like to get this in for P7 since we have some SDK teams interested in using it. I view this new provider as being internal or preview mode for .net6 and primarily for internal tooling. Regarding tests, do we have anything that tests emitting specific events defined in ClrEtwAll.man from runtime? There are a lot of events defined in that manifest file, some are public, some are private and some are not used at all anymore, where do we have tests checking low level native events emitted from runtime? I think we should take this in steps, land this for P7 in preview/private mode so we can get feedback from SDK and tools teams and then see if we should expose it further. NOTE that the MonoProfiler events in ClrEtwAll.man will only be emitted by MonoVM. |
|
The format of the events implemented in this PR closely mirror the format used by mono log profiler. Those events have already been available for many years in mono/mono, so this PR mainly makes sure we get the same information we already had in mono log profiler into EventPipe and nettrace, making it easier to transition tooling currently using mono log profiler over to EventPipe and nettrace file format with the exception of heapshots (might be added later) and sample hits (emitted by EventPipe sampling profiler). |
Add --diagnostic-mono-profiler-callspec= to accept Mono callspec string. Split keywords to enable method tracing and instrumentation, enables ability to start instrumenting (but not emitting events) in one session and then enable emitting events in later session. If a callspec is used, instrumentation will be enabled on component init.
|
@lambdageek added ability to set a callspec making enter/leave events more useful. |
|
@josalem thanks for including me. I reviewed the manifest changes, and they look good to me. The manifest change itself is fairly self-contained, in that it doesn't touch any of the existing manifests, and just creates a new one, so for the most part, the risk is mostly contained to the Mono paths here. One point worth calling out is that the keywords values in the new provider skip lots of potential values (e.g. it skips from 0x10 for Loader to 0x4000 to JIT. This is all legal, and should work fine, but I wanted to point this out because compressing these down can make it easier in the future when adding keywords, so you don't have to look for holes in the existing values. Totally your call if you want to address this or not. |
Mainly reused values from runtime provider where they overlapped, so there was an intention for the holes. Since this is a Mono specific provider, we can plug them when needed going forward or reuse more of the runtime keywords that currently are defined where we have holes. |
Sounds good. FWIW, I'm not sure how much benefit we'll gain from trying to match these up, since the events themselves are all different, but then again, it won't hurt either. |
When reused they are closely related to the kind/group of events emitted, JIT keyword will enable JIT events, Loader keyword will enable loader events, Thread keyword thread events etc. |
|
Hello @josalem! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
Add Mono profiler events into EventPipe opening up for ability to emit events directly from Mono profiler callbacks into a EventPipe provider. PR maps all existing Mono profiler callbacks to Mono specific EventPipe events in new native EventPipe provider, Microsoft-DotNETRuntimeMonoProfiler.
PR also prepares for GC events and some of them will work as is, but a couple of events fires from within GC STW and since EventPipe is not async safe, those needs additional work to be correctly handled, and are currently disabled, gc_event, gc_moves, gc_resize, gc_roots.
A couple of events needs to be enabled in Mono profiler to fire (even when enabled in EventPipe) like exception_clause and gc_allocation. Added a new env variable, MONO_DIAGNOSTICS that opens up ability to configure this. I added ability to add additional config into that variable, and you can also use it to set DOTNET_DiagnosticPorts variable as well, meaning you can control all setting MONO_DIAGNOSTICS env variable. For example:
MONO_DIAGNOSTICS=--diagnostic-ports=127.0.0.1:9000,connect,suspend
will be the same as setting DOTNET_DiagnosticPorts=127.0.0.1:9000,connect,suspend
MONO_DIAGNOSTICS=---diagnostic-mono-profiler=alloc,exception
will enable both mono profiler GC alloc as well as exception clause instrumentation (needs to be set before starting profiler).
And if you just want to use one env variable, you can combine above in MONO_DIAGNOSTICS:
MONO_DIAGNOSTICS=--diagnostic-ports=127.0.0.1:9000,connect,suspend ---diagnostic-mono-profiler=alloc,exception