Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[wasm] browser profiler #77449

Merged
merged 6 commits into from
Nov 1, 2022
Merged

Conversation

pavelsavara
Copy link
Member

@pavelsavara pavelsavara commented Oct 25, 2022

reporting both interpreter, AOT and some Javascript calls directly to chrome dev tools via performance.measure()

Chrome
image

FireFox
image

Profiler

  • mono profiler infrastructure allows to register multiple profiler plugins. This an implementation of one of them.
  • is implemented in src\mono\mono\profiler\browser.c only for WASM platform
  • it registers method enter/leave events.
  • There is no filter for the methods, so it would set flags and trigger for all managed methods.
  • it calls out into browser profiler via performance.measure API
  • interp is always calling the profiler callback, when installed
  • AOT compiler will emit icall to profiler for each flagged method
    • this means that profiler is running as part of JIT process in the AOT compiler.
  • JavaScript code of startup and interop methods are annotated

Usage

<PropertyGroup>
  <WasmProfilers>browser;</WasmProfilers>
</PropertyGroup>

And

await dotnet
    .withConfig({
        browserProfilerOptions: {}
    })
    .run()

And start profiling in browser dev tools

Notes

  • alternatively we could implement plugin into EventPipe, which is itself mono profiler plugin. This solution is simpler.
  • when enabled: simple wasm to js call per event, method name building on first call to a method
  • no runtime overhead when not enabled in user project. Just some bytes for the new code in wasm.

Future work

  • enable method filtering by name, namespace, ...
  • max depth
  • start and end time
  • start and end method
  • report GC events and class loading and config to enable/disable it

Changes

  • new profiler browser.c
  • JS startup annotation
  • new static module libmono-profiler-browser.a it is always linked into dotnet.wasm in both Debug and Release configurations
  • profiler statically linked into AOT compiler on WASM target
  • updated advanced sample to enable profiler
  • cache for method names on JS side
  • fix for MonoAOTCompiler build task to pass --profiler argument to AOT compiler

Contributes to #76316

@pavelsavara pavelsavara added this to the 8.0.0 milestone Oct 25, 2022
@pavelsavara pavelsavara self-assigned this Oct 25, 2022
@ghost
Copy link

ghost commented Oct 25, 2022

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

Issue Details

null

Author: pavelsavara
Assignees: pavelsavara
Labels:

arch-wasm, area-System.Runtime.InteropServices.JavaScript

Milestone: 8.0.0

@pavelsavara

This comment was marked as resolved.

@jakobbotsch

This comment was marked as resolved.

@pavelsavara pavelsavara changed the title [wasm] browser profiller [wasm] browser profiler Oct 25, 2022
@pavelsavara pavelsavara force-pushed the wasm_browser_profiller branch from 16b5ee6 to 1df3637 Compare October 25, 2022 22:15
Copy link
Member

@radical radical left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

incomplete review, just sharing some comments.

src/mono/mono/metadata/profiler.c Show resolved Hide resolved
src/mono/mono/metadata/profiler.c Outdated Show resolved Hide resolved
src/mono/mono/profiler/browser.c Outdated Show resolved Hide resolved
src/mono/wasm/build/WasmApp.Native.targets Show resolved Hide resolved
Copy link
Member

@lambdageek lambdageek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM except there seems to be some copy/paste errors in the invoke-cs methods

src/mono/wasm/runtime/invoke-js.ts Outdated Show resolved Hide resolved
src/mono/wasm/runtime/invoke-cs.ts Outdated Show resolved Hide resolved
@lewing lewing merged commit 0e24ea7 into dotnet:main Nov 1, 2022
radical added a commit to radical/runtime that referenced this pull request Nov 1, 2022
This reverts commit 0e24ea7.

This breaks with v8 10.9.130 with:
```
Error: Invalid 'startMark' argument: No numeric 'startTime' field
    at pe (/home/helixbot/work/B2E7090F/w/A520093B/e/performance/artifacts/bin/for-running/MicroBenchmarks/659adab0-5c86-4e74-8913-672bd56fd58e/bin/net7.0/browser-wasm/AppBundle/dotnet.js:3:12994)
    at /home/helixbot/work/B2E7090F/w/A520093B/e/performance/artifacts/bin/for-running/MicroBenchmarks/659adab0-5c86-4e74-8913-672bd56fd58e/bin/net7.0/browser-wasm/AppBundle/dotnet.js:5:95874
exiting due to exception: Error: Invalid 'startMark' argument: No numeric 'startTime' field
```

It works fine with older versions like `10.7.193` (current stable).
radical added a commit that referenced this pull request Nov 2, 2022
This reverts commit 0e24ea7.

It broke all the perf pipeline wasm builds. It happens with v8 `10.9.130`:

```
Error: Invalid 'startMark' argument: No numeric 'startTime' field
    at pe (/home/helixbot/work/B2E7090F/w/A520093B/e/performance/artifacts/bin/for-running/MicroBenchmarks/659adab0-5c86-4e74-8913-672bd56fd58e/bin/net7.0/browser-wasm/AppBundle/dotnet.js:3:12994)
    at /home/helixbot/work/B2E7090F/w/A520093B/e/performance/artifacts/bin/for-running/MicroBenchmarks/659adab0-5c86-4e74-8913-672bd56fd58e/bin/net7.0/browser-wasm/AppBundle/dotnet.js:5:95874
exiting due to exception: Error: Invalid 'startMark' argument: No numeric 'startTime' field
```

- It works fine with older versions like `10.7.193` (current stable).
- It wasn't caught in the CI checks because those are using an older version of v8.
@ghost ghost locked as resolved and limited conversation to collaborators Dec 1, 2022
@pavelsavara pavelsavara deleted the wasm_browser_profiller branch September 2, 2024 15:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants