EventPipe module names have unexpected .il suffix

[noahfalk]

When viewing an EventPipe trace in PerfView or WPA the call stack views show symbols such as system.private.corelib.il!System.Threading.ThreadPoolWorkQueue.Dispatch(). Looking at similar data collected from ETW would show System.Private.CoreLib.dll as the module name. I haven't tracked down where the .il name comes from but I suspect it is a strong produced during CLR's module event. It would be nice if we could get the profiler to ultimately show the .dll name rather than .il. That might mean changing strings included in the trace, selecting a different string among multiple that are included in the trace, or adding rules to TraceEvent that fixes up the name. It requires more investigation to understand the origin of the current name and figure out what is the best way to change the outcome.

[leculver]

Oddly enough I spent a few hours not sleeping tonight and looking into this. Yay insomnia.

This isn't a runtime issue. eventtrace.cpp SendModuleEvent() emits correct .dll paths via GetPEImage()->GetPath(). ModuleNativePath is always empty for non-NGEN assemblies. GetSimpleName() returns the metadata assembly name without any extension. The issue isn't there.

The .il suffix is introduced by TraceEvent here when it sees a module with an empty ModuleNativePath but non-empty NativePdbSignature and ManagedPdbSignature, it treats this as the "First Generation ReadyToRun" case and creates a synthetic .il.dll path (e.g., System.Private.CoreLib.dll -> System.Private.CoreLib.il.dll) so that it has two distinct TraceModuleFile entries to hang separate IL and native PDB information on.

This condition fires for all R2R framework assemblies on modern .NET because R2R assemblies have two CodeView debug directory entries (one for the IL PDB, one for the native PDB) but no separate native module file...so ModuleNativePath is empty while both PDB signatures are populated. Non-R2R assemblies (facades like System.Runtime.dll, user assemblies) don't have a native PDB entry, so they're unaffected.

The synthetic .il.dll path then leaks into TraceModuleFile.Name, which computes the display name by stripping the file extension from FilePath giving us something like System.Private.CoreLib.il.

Several places already work around this: dotnet-pgo strips .il from module names. TraceEvent's own GetSourceLine() strips .il for source resolution. PerfView's IsMscorlib() checks for "system.private.corelib.il" as a special case.

With all that said, we could:

1. Close this issue as "by-design" because this is something that TraceEvent is introducing...er, presumably by design.
2. We could fix TraceEvent to not produce this, here: microsoft/perfview@main...leculver:perfview:issue_3102 @brianrob, thoughts? This might be a breaking change.
3. You could update the code here in this repo to strip out the .il in places it's received.

Any thoughts on that?

[brianrob]

What I believe is happening here is that when a R2R image is loaded, we create both a native image and an IL image so that if/when IL is jitted, it will show up differently. For example, anything in System.Private.CoreLib that is native will show up in the System.Private.CoreLib module, but anything that is jitted, including rejitted or tiered-jitted will show up under System.Private.CoreLib.Il. This tends to be useful because you often get a sense of what was re-jitted, what wasn't, and how much time was spent in the R2R vs. jitted code.

If we feel strongly about this, I don't think changing this behavior would be too disruptive, and would likely not have downstream effects. I personally like having the .il suffix there, but I could be convinced otherwise if we have feedback that says this is confusing, etc.

[noahfalk]

Thanks for taking a look! 👍

I can't point to any direct feedback other than my own opinion on this one. For me, and I'd speculate for others, the ".il" behavior feels like a net-negative. I'd also say if I was making a top-10 priority list for profiling improvements this wouldn't make the cut. If you decide to close it I'll be fine.

If it matters these were my thoughts on why the current behavior is net-negative:

• I do think jitted vs. not-jitted is useful information in the right context, but I am guessing the majority of profiling usage (>75%) isn't that context. It would be preferable to have this information be opt-in rather than opt-out.
• I've forgotten the details, but the issue text suggests that this .il convention is only being applied to R2R assemblies reported via EventPipe in practice. Ideally if we are going to have a naming convention for jitted code it should apply consistently regardless of whether the assembly was R2R compiled and regardless whether EventPipe or ETW was used to collect the data.
• If an expert like Lee needs more than a couple minutes to discover why the ".il" is present I'd guess the average TraceEvent/PerfView user also doesn't understand the significance of it when they see it.

[leculver]

I looked at what the change would look like here in the diagnostics repo. It's a not too bad but it is a bit messy to filter it here. Since multiple tools are working around it, I think the right fix is to change this upstream. I opened microsoft/perfview#2364 for this. Thanks!