microsoft / perfview

PerfView is a CPU and memory performance-analysis tool
http://channel9.msdn.com/Series/PerfView-Tutorial
MIT License
4.16k stars 708 forks source link

Rundown at end of profiling doesn't work for dynamic or collectible methods #1017

Open kouvel opened 5 years ago

kouvel commented 5 years ago

From https://github.com/dotnet/coreclr/pull/27238 (comment: https://github.com/dotnet/coreclr/pull/27238#issuecomment-543421341)

Looks like PerfView currently only does a rundown when profiling is stopped, and relies on .NET Core to perform its own rundown on graceful process shutdown, in order to identify methods that were already jitted prior to when profiling was started.

This system does not work well for dynamic and collectible methods because their lifetime may expire prior to the rundown. On top of that, other dynamic or collectible methods may be allocated in the same memory thereafter, so the late rundown information may also lie to PerfView about where the samples actually went.

Proposal:

brianrob commented 5 years ago

The unload case here is interesting and I'm not sure if/how it is handled today. There have been debates about whether rundown should occur at the beginning or the end of the trace. So far, there has never been a strong argument for one over the other, given that the rundown will always have to be augmented with events during the trace to ensure completeness. I would propose that if we need to make a change, from a cost/benefit perspective, we keep rundown at the end as is, but augment it with data from collectible assemblies at load/unload time so that PerfView knows about any code that may have run during the assembly lifetime. The load/unload also represents the bounds during which samples can be mapped to that assembly.

kouvel commented 5 years ago

That would work too and may be easier to implement. We may already be getting method unload events (not the DCStopVerbose event), but maybe not the IL to native map. If a mini-rundown could be down on module unload along with the necessary info, that would be great.