[browser][coreCLR] Diagnostic server improvements for CoreCLR on WASM#128644
Conversation
- ds_rt_browser_performance_measure - support for JS diagnostic client during startup
|
Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag |
There was a problem hiding this comment.
Pull request overview
This PR updates the Browser/WASM diagnostics stack across the JS diagnostics client and native EventPipe plumbing, primarily to (1) change EventPipeThreadSamplingRate “unset” handling so 0 can mean max-frequency sampling, and (2) improve browser diagnostic startup + profiling integrations.
Changes:
- Change
EventPipeThreadSamplingRatesemantics: useUINT32_MAXas the “unset” sentinel and allow0to mean max-frequency sampling. - Add a new browser diagnostics export
ds_rt_browser_performance_measure(native→JS) that bridges toperformance.measure()for DevTools-friendly timeline entries. - Enable and improve browser diagnostic startup and runtime behavior (default startup diagnostic port + JS message-queue perf + robustness tweaks).
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/native/rollup.config.plugins.js | Expands circular-dependency warning suppression for additional diagnostics bundles. |
| src/native/libs/System.Native.Browser/native/index.ts | Exposes ds_rt_browser_performance_measure from the native browser export surface. |
| src/native/libs/System.Native.Browser/native/diagnostics.ts | Adds a TS wrapper forwarding ds_rt_browser_performance_measure into the diagnostics exports table. |
| src/native/libs/System.Native.Browser/diagnostics/index.ts | Registers the new performance.measure() bridge into the diagnostics exports table. |
| src/native/libs/System.Native.Browser/diagnostics/dotnet-gcdump.ts | Adds startup-client support and issues ResumeRuntime on session start. |
| src/native/libs/System.Native.Browser/diagnostics/dotnet-cpu-profiler.ts | Adds startup-client support, issues ResumeRuntime, and changes default duration. |
| src/native/libs/System.Native.Browser/diagnostics/dotnet-counters.ts | Adds startup-client support and issues ResumeRuntime on session start. |
| src/native/libs/System.Native.Browser/diagnostics/diagnostic-server.ts | Enables default DOTNET_DiagnosticPorts=js://ready configuration at init time. |
| src/native/libs/System.Native.Browser/diagnostics/diagnostic-server-ws.ts | Adds defensive error handling around WebSocket.send. |
| src/native/libs/System.Native.Browser/diagnostics/diagnostic-server-js.ts | Adds startup diagnostic client support and wires built-in scenarios to startup mode. |
| src/native/libs/System.Native.Browser/diagnostics/common.ts | Replaces Array.shift() receive-queue consumption with a head pointer (O(1) per recv). |
| src/native/libs/System.Native.Browser/diagnostics/client-commands.ts | Fixes intervalSeconds defaulting (?? instead of ` |
| src/native/libs/Common/JavaScript/types/exchange.ts | Extends the diagnostics exports table/types to include the new performance-measure export. |
| src/native/libs/Common/JavaScript/cross-module/index.ts | Maps the new diagnostics export table slot to ds_rt_browser_performance_measure. |
| src/native/eventpipe/ep.c | Updates sampling-rate override logic to treat UINT32_MAX as unset and allow 0. |
| src/native/eventpipe/ep-shared-config.h.in | Guards against PERFTRACING_DISABLE_THREADS macro redefinition. |
| src/mono/mono/eventpipe/ep-rt-mono.h | Updates Mono’s sampling-rate “unset” default to G_MAXUINT32 (and fixes a typo). |
| src/coreclr/nativeaot/Runtime/eventpipe/ep-rt-aot.h | Updates NativeAOT sampling-rate “unset” default to UINT32_MAX. |
| src/coreclr/inc/clrconfigvalues.h | Changes INTERNAL_EventPipeThreadSamplingRate default to UINT32_MAX and updates description text. |
Comments suppressed due to low confidence (1)
src/native/libs/System.Native.Browser/diagnostics/diagnostic-server-js.ts:177
createDiagConnectionJsmay both setstartupJsClient(via the built-in js://gcdump/counters/cpu-samples paths) and also resolvenextJsClientfromglobalThis.dotnetDiagnosticClient. SinceconnectNewClient()always prefersstartupJsClient, the resolvednextJsClientcan be ignored/discarded wheninitializeJsClient()is called, which is surprising if a custom client is provided. Consider defining a clear precedence (e.g., only auto-start built-in scenarios when nodotnetDiagnosticClientis provided, or skip resolvingnextJsClientwhen a startup client is already queued) and enforcing the single-client invariant for the startup path too.
export function createDiagConnectionJs(socketHandle: number, scenarioName: string): DiagnosticSession {
if (!fromScenarioNameOnce) {
fromScenarioNameOnce = true;
if (scenarioName.startsWith("js://gcdump")) {
collectGcDump({}, true);
}
if (scenarioName.startsWith("js://counters")) {
collectMetrics({}, true);
}
if (scenarioName.startsWith("js://cpu-samples")) {
collectCpuSamples({}, true);
}
const dotnetDiagnosticClient: FnClientProvider = (globalThis as any).dotnetDiagnosticClient;
if (typeof dotnetDiagnosticClient === "function") {
nextJsClient.resolve(dotnetDiagnosticClient(scenarioName));
}
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
src/native/libs/System.Native.Browser/diagnostics/diagnostic-server-js.ts:177
- createDiagConnectionJs can set a startup client via collectGcDump/collectMetrics/collectCpuSamples (which calls setupJsClient(..., startup=true)), and in the same code path it may also resolve nextJsClient from globalThis.dotnetDiagnosticClient(scenarioName). When the advert arrives, connectNewClient will always prefer startupJsClient and then call initializeJsClient(), which resets nextJsClient—effectively dropping the custom dotnetDiagnosticClient that was already resolved. Consider making these paths mutually exclusive (e.g., don’t resolve dotnetDiagnosticClient when auto-starting a built-in scenario), or unify startup handling so there is a single client queue (avoid startupJsClient side-channel).
export function createDiagConnectionJs(socketHandle: number, scenarioName: string): DiagnosticSession {
if (!fromScenarioNameOnce) {
fromScenarioNameOnce = true;
if (scenarioName.startsWith("js://gcdump")) {
collectGcDump({}, true);
}
if (scenarioName.startsWith("js://counters")) {
collectMetrics({}, true);
}
if (scenarioName.startsWith("js://cpu-samples")) {
collectCpuSamples({}, true);
}
const dotnetDiagnosticClient: FnClientProvider = (globalThis as any).dotnetDiagnosticClient;
if (typeof dotnetDiagnosticClient === "function") {
nextJsClient.resolve(dotnetDiagnosticClient(scenarioName));
}
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Split from #126324 for smaller code review
Summary
This PR makes several improvements to the EventPipe diagnostic infrastructure for the Browser target:
Adds
ds_rt_browser_performance_measure— a new native-to-JS interop that callsperformance.measure(), enabling the runtime to emit browser Performance Timeline entries for profiling/tracing visibility in DevTools.Enables startup diagnostic port by default — removes the
WASM-TODOguard so thatDOTNET_DiagnosticPorts=js://readyis set automatically, allowing diagnostic scenarios (gcdump, counters, cpu-samples) to connect at startup before the runtime is fully running.Improves diagnostic message queue performance — replaces
Array.shift()with a head-pointer approach inDiagnosticConnectionBase.recv(), avoiding O(n) array copies on every message consumption.