[browser] Fix EventPipe CPU sampling stall under coarse browser timer resolution#129848
Merged
pavelsavara merged 1 commit intoJun 26, 2026
Merged
Conversation
…esolution update_sample_frequency divided by ms_since_last_sample which can round to 0 with coarse browser timer resolution, producing +Inf that saturates skips_per_period to INT_MAX and permanently disables sampling. Skip recompute when no wall-clock time elapsed and clamp the averaged skip count. Re-enables the EventPipe CPU sample tests previously disabled by dotnet#129584.
pavelsavara
requested review from
ilonatommy,
lateralusX,
maraf,
steveisok and
vitek-karas
as code owners
pavelsavara
added
arch-wasm
Contributor
|
Tagging subscribers to 'arch-wasm': @lewing, @pavelsavara |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adjusts Mono’s browser EventPipe CPU sampling skip-calculation to avoid pathological behavior when the browser’s performance.now() resolution is coarse (e.g., repeated 0 deltas), and re-enables the previously-disabled Blazor EventPipe CPU sampling tests.
Changes:
- Guard
update_sample_frequency()to only recomputeskips_per_periodwhenms_since_last_sample > 0, and clamp the computed skip range. - Re-enable Blazor EventPipe CPU sampling tests by removing the
ActiveIssueskips.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/mono/mono/eventpipe/ep-rt-mono-runtime-provider.c | Adds elapsed-time guard and clamps adaptive skip computation for single-threaded browser sampling. |
| src/mono/wasm/Wasm.Build.Tests/Blazor/EventPipeDiagnosticsTests.cs | Removes ActiveIssue attributes to re-enable CPU sampling test coverage. |
maraf
approved these changes
Jun 25, 2026
pavelsavara
changed the title
Member
Author
|
Member
Author
|
/ba-g infra |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #129584.
Problem
On WebAssembly the EventPipe CPU sample profiler could permanently stop emitting samples.
update_sample_frequencyrecomputesskips_per_periodby dividingsample_skip_counterbyms_since_last_sample. With the coarse browser timer resolution,ms_since_last_samplecan round to0during a burst of instrumentation callbacks. Dividing by0yields+Inf, which — once cast tointunder WASM's saturating, non-trapping float-to-int conversion — pinsskips_per_periodatINT_MAX, permanently disabling sampling.Fix
ms_since_last_sample > 0); otherwise keep the previousskips_per_period.[1, 10000]) so sampling stays responsive, matching the CoreCLR single-threaded sampler behavior.This re-enables the EventPipe CPU sample Blazor tests (
BlazorEventPipeTestWithCpuSamplesAOTandBlazorEventPipeTestWithCpuSamples) that were disabled by the issue.Note
This PR description was generated with assistance from GitHub Copilot.