Send tiered compilation settings rundown event on EventPipe detach

[kouvel]

No description provided.

[kouvel]

Many other rundown events are missing even after enabling the provider, such as importantly Method/DCStopVerbose. So I'm not sure if it's going through the normal rundown path, but for now anyway this change seems to be working and I'm seeing the event. Eventually after the rest of rundown is fixed, maybe this change would be redundant and can be reverted.

[noahfalk]

Ideally this code should be somewhere under the call-tree starting with ETW::EnumerationLog::EndRundown() and you would get there at this callstack:

ETW::EnumerationLog::EndRundown()
EventPipeSession::ExecuteRundown()
EventPipe::DisableInternal()

Putting it under EndRundown() has a few advantages:
a) That is the point where rundown is actually executing vs. the current location is still going to be intermingled with threads writing arbitrary things. Those pre-existing couple events in DisableInternal() are not in an ideal place either.
b) EndRundown() is shared with ETW, EventPipe, and LTTng vs the current location is EventPipe specific
c) @jorive is still actively doing work in the Disable() code-path and I don't want to add more moving parts to it if we can help it

If there is something blocking us from doing the long term solution I'm open to short term workarounds, but then I'd request there is some work item that tracks what needs to be fixed (repro scenario, any bug investigation notes) in order to move from the short-term to the long-term.

[cshung]

Many other rundown events are missing even after enabling the provider, such as importantly Method/DCStopVerbose. So I'm not sure if it's going through the normal rundown path, but for now anyway this change seems to be working and I'm seeing the event. Eventually after the rest of rundown is fixed, maybe this change would be redundant and can be reverted.

How do you know Method/DCStopVerbose is required as a rundown event? It should be nice if we can come up with a list of those so that I can use to verify that rundown does produce those.

If we identify something is missing from the trace, we should file an issue to track the missing of them.

[kouvel]

How do you know Method/DCStopVerbose is required as a rundown event?

There has to be some event associating a jitted code address with a method name. If I attach to a process after all jitting is finished, I don't see any stacks shown. When I last checked with ETL profiles, for native-to-IL-to-source mapping (to show samples by source line) the native-to-IL map event needed to be paired with the DCStopVerbose.

There may be other events, though I don't know much more. In my high level view ideally a rundown would be done upon attach (StartRundown) to collect info about loaded modules and jitted methods at the time of attach, and then a rundown on detach shouldn't be necessary because we would get live events from jitting and module load/unload from that point onwards. I believe PerfView by default when collecting ETL traces only does a rundown on detach (EndRundown), I'm not sure if that would work for dynamic or collectible methods, where the method was loaded before attach and gets unloaded after attach and before detach, as the rundown on detach would not provide info about that method.

[kouvel]

Ideally this code should be somewhere under the call-tree starting with ETW::EnumerationLog::EndRundown() and you would get there at this callstack

Thanks, I had it there initially except I had put it in the StartRundown method, as I intended for the event to be sent on attach. When I didn't see the event I had followed what is done for the RuntimeInformation event and only later realized that PerfView only does a rundown on detach by default (and would only go through the EndRundown path). I could be mistaken though, my knowledge here is very limited.

[noahfalk]

In my high level view ideally a rundown would be done upon attach (StartRundown) to collect info

I wasn't involved in the initial decision making so I don't know if this is accurate, but I assume one of the considerations is how much initiating the trace session will perturb the process. Rundown can be quite expensive so if you imagine attaching to process that is currently reproing a delicate issue or needs to remain responsive in production then stalling to write a 10K rundown events might not work well. I expect these concerns are pretty scenario specific and you could easily imagine deciding that for certain small sets of events it was better to emit them early, but for other more voluminous events you would defer them.

[kouvel]

Yea I figured that might be a reason, would be nice to have an option to enumerate on start though, I haven't tried but I don't think it would work currently.

[kouvel]

I had issues with the case where the process exits while tracing is happening too, where I don't think a rundown happens currently.

[noahfalk]

would be nice to have an option to enumerate on start though, I haven't tried but I don't think it would work currently.

I'm not sure if we'd get to it priority-wise as 3.0 is just about done, but from a technical standpoint I agree it would be a nice option for us to have available. If you want to open an issue it feels like something we should look into.

[kouvel]

We do seem to get Method/UnloadVerbose when the process exits, so that should be enough but still not getting the stacks I expect to see (in ETL traces).

[kouvel]

I'll look into opening an issue, it would be useful for tracing shutdown scenarios, or if taking a long trace and unexpectedly the process exits. I agree it's not that important right now.

[kouvel]

Does the change look ok?

[noahfalk]

LGTM. Thanks Kount!

[noahfalk]

Thanks for cleaning this one up too : )

[noahfalk]

cc @dotnet/dotnet-diag

[josalem]

As far as I'm aware, ETW::EnumerationLog::EndRundown is the only way we start the rundown, so we shouldn't end up with this firing at the beginning and end of a session. @jorive would know for sure though. I agree that we should light up the ability to send rundown events in other manners besides at the end.

[kouvel]

I've only seen EndRundown being called currently. Once StartRundown becomes possible, I imagine only one type of rundown would be done, or there may be a different kind of partial rundown on end that may take a different path or have a parameter to specify partial.