[browser] profilers linking and instrumentation in AOT #114080

pavelsavara · 2025-03-31T19:21:43Z

Linking

link profilers only when requested with workload
cleanup test-main.js

AOT and instrumentation for `ep-rt-mono-runtime-provider`

use callspec for filtering via DOTNET_WasmPerfInstrumentation env variable
removed filtering via mono_wasm_instrument_method
initialize instrumenting in AOT via mono_profiler_init_ds_sample when --profile=ds_sample is passed
refactor callspec parser mono_profhelper_parse_profiler_args into helper.c

Fixes

ep_write_empty_profile_event - to make dotnet-gcdump work
fix FRAME_TYPE_IL_STATE stack frame sampling
add needs_LMF when MONO_PROFILER_CALL_INSTRUMENTATION_SAMPLEPOINT
fix mini_profiler_emit_samplepoint to emit ctx only when requested

Contributes to #76316

- use callspec for filtering via DOTNET_WasmPerfInstrumentation env variable - fix FRAME_TYPE_IL_STATE stack frame sampling - initialize instrumenting in AOT via mono_profiler_init_ds_sample when --profile=ds_sample is passed - refactor callspec parser into helper.c - add method_needs_stack_walk when MONO_PROFILER_CALL_INSTRUMENTATION_SAMPLEPOINT - link profilers only when requested with workload

dotnet-policy-service · 2025-03-31T19:22:14Z

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

Copilot

Pull Request Overview

This PR refactors the profiling and instrumentation configuration for browser AOT by removing legacy support and consolidating profiler initialization. It also updates documentation and cleans up related runtime and binding code.

Updates features documentation to reflect new instrumentation configuration options.
Removes legacy support for the "--profile=" argument in favor of environment-variable based control.
Refactors and cleans up the profiler helper functions and corresponding imports.

Reviewed Changes

Copilot reviewed 6 out of 20 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
src/mono/wasm/features.md	Updated instrumentation XML to document new callspec options.
src/mono/browser/test-main.js	Removed legacy argument parsing for "--profile=" support.
src/mono/browser/runtime/types/internal.ts	Removed the instrumentation method declaration from runtime types.
src/mono/browser/runtime/profiler.ts	Deleted the legacy mono_wasm_instrument_method export.
src/mono/browser/runtime/exports-binding.ts	Removed references to mono_wasm_instrument_method for consistency.
src/mono/browser/runtime/diagnostics/index.ts	Removed assignment to the deprecated instrumentation method.

Files not reviewed (14)

src/mono/browser/browser.proj: Language not supported
src/mono/browser/build/BrowserWasmApp.targets: Language not supported
src/mono/browser/runtime/CMakeLists.txt: Language not supported
src/mono/browser/runtime/driver.c: Language not supported
src/mono/mono/eventpipe/ep-rt-mono-runtime-provider.c: Language not supported
src/mono/mono/eventpipe/ep-rt-mono.h: Language not supported
src/mono/mono/metadata/profiler.c: Language not supported
src/mono/mono/mini/CMakeLists.txt: Language not supported
src/mono/mono/mini/method-to-ir.c: Language not supported
src/mono/mono/profiler/CMakeLists.txt: Language not supported
src/mono/mono/profiler/browser.c: Language not supported
src/mono/mono/profiler/helper.c: Language not supported
src/mono/sample/wasm/Directory.Build.targets: Language not supported
src/mono/wasm/build/WasmApp.Common.targets: Language not supported

Comments suppressed due to low confidence (5)

src/mono/wasm/features.md:390

[nitpick] Consider clarifying the documentation regarding multiple entries to avoid confusion about which configuration is applied at runtime.

<WasmPerfInstrumentation>N:Sample</WasmPerfInstrumentation>

src/mono/browser/test-main.js:131

With the removal of '--profile=' processing, please ensure that the 'runArgs.profilers' array and any related initialization are also removed to prevent future confusion.

if (currentArg.startsWith("--profile=")) {

src/mono/browser/runtime/types/internal.ts:244

The removal of 'mono_wasm_instrument_method' from runtime types is part of this refactor; please verify that no lingering references to this method remain elsewhere in the codebase.

mono_wasm_instrument_method: (method:MonoMethod) => number,

src/mono/browser/runtime/profiler.ts:119

Ensure that all invocations of 'mono_wasm_instrument_method' have been properly replaced following its removal to avoid unintended side-effects.

export function mono_wasm_instrument_method (method:MonoMethod):number {

src/mono/browser/runtime/diagnostics/index.ts:34

With the deprecation of 'mono_wasm_instrument_method', confirm that this assignment is no longer needed and that all related functionality has been updated accordingly.

runtimeHelpers.mono_wasm_instrument_method = (method: MonoMethod): number => {

src/mono/wasm/build/WasmApp.Common.targets

Co-authored-by: Marek Fišera <mara@neptuo.com>

src/mono/browser/build/BrowserWasmApp.targets

msbuild cleanup

lateralusX

Overall the native code looks good, a couple of smaller comments and some ideas around naming to make the different profiling "modes" more clear.

As discussed off line, adding push/pop LMF around every AOT call will add overhead, but that might be the price to pay for detailed stack walks including AOT frames, so we should probably start out that way and if we find it to be an issue we could potentially handle that later. We do have a callspec when compiling the AOT code, so in theory we could go with a different subset to reduce impact, while keeping a broader set for the interpreted code.

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

lateralusX · 2025-04-03T12:34:44Z

src/mono/mono/mini/method-to-ir.c

 				}
 #endif
 			}
+#ifdef TARGET_WASM
+			if (cfg->prof_flags & MONO_PROFILER_CALL_INSTRUMENTATION_SAMPLEPOINT) {


We discussed this offline, question is how much additional overhead this will add. Also, the prof_flags represent the calling method in this case, question is if this needs to check the callspec for the called method? If you don't sample the called method, then there won't be any need for the LMF, right?

It would be ideal to propagate the LMF to all possible callers (recursively) of whatever wants to be sampled.
This only does, "include me in samples of others, because I was also instrumented".
I guess good enough.

src/mono/mono/profiler/browser.c

lateralusX

Native code LGTM!

src/mono/browser/runtime/profiler.ts

src/mono/browser/runtime/startup.ts

maraf · 2025-04-04T07:59:47Z

src/tasks/Microsoft.NET.Sdk.WebAssembly.Pack.Tasks/GenerateWasmBootJson.cs

+        Profilers ??= Array.Empty<string>();
+        var browserProfiler = Profilers.FirstOrDefault(p => p.StartsWith("browser:"));
+        if (browserProfiler != null)
+        {
+            result.environmentVariables ??= new();
+            result.environmentVariables["DOTNET_WasmPerfInstrumentation"] = browserProfiler.Substring("browser:".Length);
+        }


You can move this code into the helper and share it

nope, let's delete the other one

Perhaps we can make the whole json generation "helper" ? In another PR

Co-authored-by: Marek Fišera <mara@neptuo.com>

pavelsavara · 2025-04-04T11:26:47Z

/ba-g CI timeouts and unrelated failures

lewing · 2025-04-08T23:18:02Z

It seems like this caused some wasm regressions at least

before regression

(func Wasm_Browser_Bench_Sample_Sample_VectorTask_EqualsInt32_RunStep(param $0 i32, $1 i32))
 block
  local.get $0
  i32.eqz
  br.if
  local.get $0
  i32.const -1
  i32.eq
  br.if
  local.get $0
  local.get $0
  v128.load offset:16    [SIMD]
  local.get $0
  v128.load offset:32    [SIMD]
  i32x4.eq    [SIMD]
  i32x4.all.true    [SIMD]
  i32.store8 offset:48
  return
 
 call mini_llvmonly_throw_nullref_exception
 unreachable

after regression

(func Wasm_Browser_Bench_Sample_Sample_VectorTask_EqualsInt32_RunStep(param $0 i32, $1 i32))
local $2 3 i32
local $3 v128
global.get $global:0
i32.const 48
i32.sub
local.tee $1
global.set $global:0
i32.const 3955246
i32.load8.u
i32.eqz
if
 i32.const 1042482
 call mono_aot_Wasm_Browser_Bench_Sample_init_method
 i32.const 3955246
 i32.const 1
 i32.store8

i32.const 3947384
i32.load align:2
i32.const 0
call mono_profiler_raise_method_enter
local.get $0
if
 local.get $0
 v128.load offset:32    [SIMD]
 local.set $5
 local.get $1
 i32.const 4132240
 i32.load align:2
 local.tee $3
 i32.load align:2
 i32.store offset:36 align:2
 local.get $3
 local.get $1
 i32.const 36
 i32.add
 i32.store align:2
 local.get $1
 i32.const 3947384
 i32.load align:2
 i32.store offset:44 align:2
 i32.const 3947392
 i32.load align:2
 local.tee $2
 i32.load offset:4 align:2
 local.set $4
 local.get $2
 i32.load align:2
 local.set $2
 local.get $1
 local.get $5
 v128.store offset:16 align:4    [SIMD]
 local.get $1
 local.get $1
 v128.load offset:16 align:4    [SIMD]
 v128.store align:4    [SIMD]
 local.get $0
 i32.const 16
 i32.add
 local.get $1
 local.get $4
 local.get $2
 call.indirect (func (param i32, i32, i32) (result i32))
 local.set $2
 local.get $3
 local.get $1
 i32.load offset:36 align:2
 i32.store align:2
 local.get $0
 local.get $2
 i32.store8 offset:48
 local.get $1
 i32.const 48
 i32.add
 global.set $global:0
 return

call mini_llvmonly_throw_nullref_exception
unreachable

pavelsavara · 2025-04-09T09:30:03Z

It seems like this caused some wasm regressions at least

Yes, thanks, my fault.
#114428

But that would not explain non-browser impact.

pavelsavara added arch-wasm WebAssembly architecture area-Diagnostics-mono os-browser Browser variant of arch-wasm labels Mar 31, 2025

pavelsavara added this to the 10.0.0 milestone Mar 31, 2025

pavelsavara self-assigned this Mar 31, 2025

cleanup

4662a30

pavelsavara marked this pull request as ready for review March 31, 2025 19:47

Copilot AI review requested due to automatic review settings March 31, 2025 19:47

pavelsavara requested review from lewing, maraf, akoeplinger, BrzVlad, steveisok, vitek-karas, lateralusX and thaystg as code owners March 31, 2025 19:47

Copilot AI reviewed Mar 31, 2025

View reviewed changes

kg mentioned this pull request Apr 3, 2025

SIGKILL (OOM?) while running LibraryImportGenerator.Tests w/o actionable log messages or artifacts #114241

Closed

maraf approved these changes Apr 1, 2025

View reviewed changes

src/mono/wasm/build/WasmApp.Common.targets Outdated Show resolved Hide resolved

pavelsavara and others added 2 commits April 1, 2025 09:22

Update src/mono/wasm/build/WasmApp.Common.targets

3335c4c

Co-authored-by: Marek Fišera <mara@neptuo.com>

check allowed combinations

27717a8

javiercn approved these changes Apr 1, 2025

View reviewed changes

maraf reviewed Apr 1, 2025

View reviewed changes

src/mono/browser/build/BrowserWasmApp.targets Show resolved Hide resolved

src/mono/browser/build/BrowserWasmApp.targets Show resolved Hide resolved

feedback

1c35a51

This was referenced Apr 3, 2025

System.Net.WebSockets.Client.Tests timeout #114153

Closed

System.Net.Security.Unit.Tests timeout #114176

Closed

pavelsavara added 2 commits April 3, 2025 08:20

Merge branch 'main' into browser_profilers_aot

4bdafdd

feedback

035eed5

This was referenced Apr 3, 2025

[QUIC & HTTP/3] Handshake Timeout on tests #104426

Open

/root/helix/work/correlation/scripts/<hash>/execute.sh: Permission denied dotnet/dnceng#3412

Open

System.Net.Security.Tests timeout #114152

Closed

pavelsavara added 2 commits April 3, 2025 12:20

normalize callspec passing with dev tools profiler

b15cfe1

msbuild cleanup

feeedback

ec81572

lateralusX reviewed Apr 3, 2025

View reviewed changes

pavelsavara added 3 commits April 3, 2025 18:11

Merge branch 'main' into browser_profilers_aot

2cd66ca

feedback

a182f78

fix

e6194f1

This was referenced Apr 3, 2025

System.Runtime.Tests.WorkItemExecution "crash" on tvOS #106319

Open

[Apple-mobile] System.Tests.StringTests.[Starts,Ends]WithNoMatch_StringComparison test failure #108832

Open

Merge branch 'main' into browser_profilers_aot

dc41432

lateralusX approved these changes Apr 4, 2025

View reviewed changes

maraf reviewed Apr 4, 2025

View reviewed changes

pavelsavara and others added 4 commits April 4, 2025 10:35

Update src/mono/browser/runtime/profiler.ts

860c092

Co-authored-by: Marek Fišera <mara@neptuo.com>

Update src/mono/browser/runtime/profiler.ts

f30de43

Co-authored-by: Marek Fišera <mara@neptuo.com>

Update src/mono/browser/runtime/startup.ts

421a594

Co-authored-by: Marek Fišera <mara@neptuo.com>

fix

deb7cbb

build-analysis bot mentioned this pull request Apr 4, 2025

Error reported in diagnostic logs. Please examine the log for more details dotnet/dnceng#1928

Open

3 tasks

pavelsavara merged commit 39265ca into dotnet:main Apr 4, 2025
166 of 179 checks passed

pavelsavara deleted the browser_profilers_aot branch April 4, 2025 14:12

matouskozak mentioned this pull request Apr 8, 2025

[Perf] Linux/arm64: 4 Regressions on 4/4/2025 5:42:38 PM +00:00 dotnet/perf-autofiling-issues#53175

Closed

pavelsavara mentioned this pull request Apr 9, 2025

[browser] remove profiler from samples #114428

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[browser] profilers linking and instrumentation in AOT #114080

[browser] profilers linking and instrumentation in AOT #114080

pavelsavara commented Mar 31, 2025 •

edited

Loading

dotnet-policy-service bot commented Mar 31, 2025

Copilot AI left a comment

lateralusX left a comment

lateralusX Apr 3, 2025

pavelsavara Apr 3, 2025

lateralusX left a comment

maraf Apr 4, 2025

pavelsavara Apr 4, 2025

pavelsavara Apr 4, 2025

pavelsavara commented Apr 4, 2025

lewing commented Apr 8, 2025

pavelsavara commented Apr 9, 2025

[browser] profilers linking and instrumentation in AOT #114080

[browser] profilers linking and instrumentation in AOT #114080

Conversation

pavelsavara commented Mar 31, 2025 • edited Loading

Linking

AOT and instrumentation for ep-rt-mono-runtime-provider

Fixes

dotnet-policy-service bot commented Mar 31, 2025

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

lateralusX left a comment

Choose a reason for hiding this comment

lateralusX Apr 3, 2025

Choose a reason for hiding this comment

pavelsavara Apr 3, 2025

Choose a reason for hiding this comment

lateralusX left a comment

Choose a reason for hiding this comment

maraf Apr 4, 2025

Choose a reason for hiding this comment

pavelsavara Apr 4, 2025

Choose a reason for hiding this comment

pavelsavara Apr 4, 2025

Choose a reason for hiding this comment

pavelsavara commented Apr 4, 2025

lewing commented Apr 8, 2025

pavelsavara commented Apr 9, 2025

pavelsavara commented Mar 31, 2025 •

edited

Loading

AOT and instrumentation for `ep-rt-mono-runtime-provider`