search

dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
14.56k stars 4.55k forks source link

New GCLowVolume keyword for ETW/EventPipe events when verbose events are enabled for other keywords #11775

Open mjsabby opened 5 years ago

mjsabby commented 5 years ago

To do continuous monitoring of an application where you would like MethodLoad events for jitted code to support symbolic lookup, and would also like GC events you end up getting AllocationTicks as well because the GC Keyword's verbosity level 5 gives you that data.

I'm proposing we add a new GCLowVolume keyword so that we can get verbose data for MethodLoads and continue to have GC monitoring that does not impact performance (due to the high event volume).

tommcdon commented 5 years ago

cc @Maoni0 @andy-ms @noahfalk @jorive

Maoni0 commented 5 years ago

it sounds like what you are asking is the ability to specify different levels for different keywords. from a feature area's POV, there's nothing that prevents you from doing that - each event will check if it's enabled based on keyword + level already. I'm curious - @vancem is there a way currently to specify to collect with different levels for different keywords with perfview?

mjsabby commented 5 years ago

It maybe that the event does that, but as far as I know ETW does not allow per event verbosity levels, and only supports verbosity at the provider level.

You could image going the other way, i.e. make a new keyword for Method Load data being available in a new keyword at non verbose levels, and that will be fine with me as well. I had to pick one direction.

vancem commented 5 years ago

@mjsabby quiet a while ago (years now), we moved the Method*Verbose events (which have the symbolic information) from the Verbose level to the Informational level, for precisely the reason you state. I allows you to get symbols information for the JIT compiled code without having to have verbose events (in particular the GCAllocationTick event), if you desire that.

Does that solve your problem? It is already there (and has been for a while). You can simply turn your verbosity down it Informational to avoid the verbose GC events.

mjsabby commented 5 years ago

It does, but I don't see that actually occurring so I'll have to confirm. If I recall not setting the level to verbose caused the dynamic methods not to be emitted.

vancem commented 5 years ago

If there are bugs, we should fix them (but I would not have expected dynamic methods to be treated differently, so I am surprised it would not just work). We should confirm.

Maoni0 commented 5 years ago

@mjsabby could you please verify if you are seeing the designed behavior (Method events on information level)? if not we should fix that.

mjsabby commented 5 years ago

On information level we see Microsoft-Windows-DotNETRuntime/Method/Load, whereas on verbose we see Microsoft-Windows-DotNETRuntime/Method/LoadVerbose. The verbose events carry the method name, namespace & signature. The regular events do not.

I still think that it makes sense to add a new keyword for GC like GCLowVolume because it may be that there are other verbose events that are useful like JIT inlining information, and it is fine to want them because eventually an application reaches steady state but the GC events continue to be voluminous.

Furthermore, it is probably a regression to do this at Information level. Because one could argue that even at Level 4 of Method events there is enough information being emitted that in conjunction with ModuleLoad events you could open the PE file and read the method name, method namespace and signature from the metadata.

mjsabby commented 5 years ago

I looked into implementing this and it would require a lot of changes to the GC events. Instead maybe we can add a SupressGCAllocationTick keyword that will suppress the AllocationTicks. This way the change is only in a single place in the GC. @Maoni0 thoughts?

noahfalk commented 5 years ago

@mjsabby - I am guessing there is going to be a general pattern here of people wanting verbose events from one keyword and not from another. I want to make sure that if we do something we are following a path that scales towards a general solution, even if we only implement a little part of it to start. My instinct is that GCLowVolume could probably be generalized but SupressGCAllocationTick feels very one-off (not any significant analysis behind this, just a hunch). The team is really heads down trying to get 3.0 wrapped up and with all the work in flight I don't think we've got time to engage on the design for this right now. I'm going to suggest we let this one rest for now and return to it once we've got fewer features in-progress.