[eventpipe] Use MonoImage:module_name when filename is NULL #73018

lambdageek · 2022-07-28T18:42:10Z

When assemblies are bundled (for example in WASM), the filename field is set to NULL; use the module name (from the metadata Module table).

Contributes to #72481

With this PR and with #72859 I can generate a trace and run

dotnet-pgo create-mibc --trace example.nettrace -r ./bin/Debug/AppBundle/managed/'*.dll' --output /tmp/out.mibc

and generate a .mibc file.

…is null When assemblies are bundled (for example in WASM), the filename field is set to NULL and the name field is set to the basename of the assembly.

ghost · 2022-07-28T18:42:28Z

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

Issue Details

When assemblies are bundled (for example in WASM), the filename field is set to NULL and the name field is set to the basename of the assembly.

Contributes to #72481

With this PR and with #72859 I can generate a trace and run

dotnet-pgo create-mibc --trace example.nettrace -r ./bin/Debug/AppBundle/managed/'*.dll' --output /tmp/out.mibc

and generate a .mibc file.

Author:	lambdageek
Assignees:	-
Labels:	`arch-wasm`, `area-Tracing-mono`
Milestone:	7.0.0

lambdageek · 2022-07-28T18:44:24Z

Sample PerfTrace line for a ModuleLoad event:

ThreadID="694,952" ProcessorNumber="0" ModuleID="38,139,048" AssemblyID="0" ModuleFlags="Manifest" ModuleILPath="System.Runtime.InteropServices.JavaScript.dll" ModuleNativePath="" ManagedPdbSignature="8c78d9aa-29ad-ff33-07d8-215908aea400" ManagedPdbAge="1" ManagedPdbBuildPath="/Users/alklig/work/dotnet-runtime/runtime/artifacts/obj/mono/Wasm.Browser.EventPipe.Sample/wasm/Debug/browser-wasm/linked/System.Runtime.InteropServices.JavaScript.pdb" NativePdbSignature="00000000-0000-0000-0000-000000000000" NativePdbAge="0" NativePdbBuildPath="" ModuleILFileName="System.Runtime.InteropServices.JavaScript.dll"

without this PR, the ModuleILPath was ""

And here's a DomainModuleLoad. Here too the ModuleILPath was "" before:

ThreadID="694,952" ProcessorNumber="0" ModuleID="38,139,048" AssemblyID="0" AppDomainID="11,531,752" ModuleFlags="Manifest" ModuleILPath="System.Runtime.InteropServices.JavaScript.dll" ModuleNativePath="" ClrInstanceID="9"

src/mono/mono/eventpipe/ep-rt-mono.c

radical · 2022-07-28T20:57:35Z

src/mono/mono/eventpipe/ep-rt-mono.c

@@ -3784,7 +3784,7 @@ get_module_event_data (
 		if (image && image->aot_module)
 			module_data->module_flags |= MODULE_FLAGS_NATIVE_MODULE;

-		module_data->module_il_path = image && image->filename ? image->filename : "";
+		module_data->module_il_path = image && image->filename ? image->filename : (image && image->name ? image->name : "");


Just curious, in what case would we get module_il_path = ""?

Hm. good point. image->name is always non-null. If it's not set explicitly (for example when doing Aseembly.Load(byte[]) it's set to "data-%p". @lateralusX is it better to have "" or some arbitrary made-up name?

Not sure, this value can be used by tooling to locate and potentially load the module. if we have "" I guess there is bigger chance that the tool will understand that its missing, compared to having some made up value that it probably will try to use and fail.

This is what CoreCLR does:

// if we do not have a module path yet, we put the module name if(bIsDynamicAssembly || ModuleILPath==NULL || wcslen(ModuleILPath) <= 2) { moduleName.SetUTF8(pModule->GetSimpleName()); ModuleILPath = (PWCHAR)moduleName.GetUnicode(); ModuleNativePath = (PWCHAR)pEmptyString; }

So they assume they always have a valid module name or it will end up with an empty string (SString moduleName = SString::Empty();).

So if we should have same behavior in case we have missing module name it should revert to "".

Ok, changed it to use the module name if available and fall back to ""

src/mono/mono/eventpipe/ep-rt-mono.c

module name comes from the metadata Module table and is something like "Foo.dll" even if the assembly is loaded from a bundle or from Assembly.Load(byte[]), whereas image->name might be "data-0x00abcdef" for byte loads

lateralusX · 2022-07-29T14:13:25Z

I guess the title should be Use MonoImage:filename ?

lambdageek · 2022-08-01T15:12:19Z

(accidentally pushed unrelated work to this PR; reverted back to reviewed version)

azure-pipelines · 2022-08-01T15:12:25Z

No pipelines are associated with this pull request.

[eventpipe] Use MonoImage:name when available, if MonoImage:filename …

11abaa0

…is null When assemblies are bundled (for example in WASM), the filename field is set to NULL and the name field is set to the basename of the assembly.

lambdageek added arch-wasm WebAssembly architecture area-Tracing-mono labels Jul 28, 2022

lambdageek added this to the 7.0.0 milestone Jul 28, 2022

lambdageek requested a review from lateralusX as a code owner July 28, 2022 18:42

ghost assigned lambdageek Jul 28, 2022

lateralusX approved these changes Jul 28, 2022

View reviewed changes

lambdageek commented Jul 28, 2022

View reviewed changes

src/mono/mono/eventpipe/ep-rt-mono.c Outdated Show resolved Hide resolved

Update src/mono/mono/eventpipe/ep-rt-mono.c

4152c28

radical reviewed Jul 28, 2022

View reviewed changes

lambdageek force-pushed the wasm-ep-module-bundle-names branch from 4fcc7f5 to 6e145cc Compare July 29, 2022 14:07

lateralusX reviewed Jul 29, 2022

View reviewed changes

src/mono/mono/eventpipe/ep-rt-mono.c Outdated Show resolved Hide resolved

use the module name, not image->name

2009c30

module name comes from the metadata Module table and is something like "Foo.dll" even if the assembly is loaded from a bundle or from Assembly.Load(byte[]), whereas image->name might be "data-0x00abcdef" for byte loads

lambdageek force-pushed the wasm-ep-module-bundle-names branch from 6e145cc to 2009c30 Compare July 29, 2022 14:09

lambdageek changed the title ~~[eventpipe] Use MonoImage:name when available, if MonoImage:filename is null~~ [eventpipe] Use MonoImage:name when available, otherwise module_name Jul 29, 2022

lateralusX approved these changes Jul 29, 2022

View reviewed changes

lambdageek changed the title ~~[eventpipe] Use MonoImage:name when available, otherwise module_name~~ [eventpipe] Use MonoImage:module_name when filename is NULL Jul 29, 2022

lambdageek requested review from lewing and pavelsavara as code owners August 1, 2022 15:06

lambdageek force-pushed the wasm-ep-module-bundle-names branch from 8c6d99c to 2009c30 Compare August 1, 2022 15:11

lambdageek mentioned this pull request Aug 1, 2022

[wasm-mt] Fixup C exports initialization after startup.ts refactoring #73158

Merged

lambdageek merged commit 73eb0cd into dotnet:main Aug 1, 2022

ghost locked as resolved and limited conversation to collaborators Sep 1, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[eventpipe] Use MonoImage:module_name when filename is NULL #73018

[eventpipe] Use MonoImage:module_name when filename is NULL #73018

lambdageek commented Jul 28, 2022 •

edited

Loading

ghost commented Jul 28, 2022

lambdageek commented Jul 28, 2022 •

edited

Loading

radical Jul 28, 2022

lambdageek Jul 28, 2022

lateralusX Jul 29, 2022 •

edited

Loading

lateralusX Jul 29, 2022 •

edited

Loading

lambdageek Jul 29, 2022

lateralusX commented Jul 29, 2022

lambdageek commented Aug 1, 2022 •

edited

Loading

azure-pipelines bot commented Aug 1, 2022

[eventpipe] Use MonoImage:module_name when filename is NULL #73018

[eventpipe] Use MonoImage:module_name when filename is NULL #73018

Conversation

lambdageek commented Jul 28, 2022 • edited Loading

ghost commented Jul 28, 2022

lambdageek commented Jul 28, 2022 • edited Loading

radical Jul 28, 2022

Choose a reason for hiding this comment

lambdageek Jul 28, 2022

Choose a reason for hiding this comment

lateralusX Jul 29, 2022 • edited Loading

Choose a reason for hiding this comment

lateralusX Jul 29, 2022 • edited Loading

Choose a reason for hiding this comment

lambdageek Jul 29, 2022

Choose a reason for hiding this comment

lateralusX commented Jul 29, 2022

lambdageek commented Aug 1, 2022 • edited Loading

azure-pipelines bot commented Aug 1, 2022

lambdageek commented Jul 28, 2022 •

edited

Loading

lambdageek commented Jul 28, 2022 •

edited

Loading

lateralusX Jul 29, 2022 •

edited

Loading

lateralusX Jul 29, 2022 •

edited

Loading

lambdageek commented Aug 1, 2022 •

edited

Loading