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.

Sign up for GitHub

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

[browser] single threaded diagnostic server #111910

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

[browser] single threaded diagnostic server #111910

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
3cd951f
wip
pavelsavara Jan 28, 2025
3bbee71
fix
pavelsavara Jan 28, 2025
fe063de
fix
pavelsavara Jan 28, 2025
d6c5821
chmod a+rw
pavelsavara Jan 28, 2025
cdad8b2
fix
pavelsavara Jan 29, 2025
46aa990
Merge branch 'main' into browser_st_ep_step1
pavelsavara Jan 29, 2025
3b711be
revert chmod
pavelsavara Jan 29, 2025
685f663
put it back
pavelsavara Jan 29, 2025
2af88c3
feedback
pavelsavara Jan 31, 2025
a4d0ac8
Merge branch 'main' into browser_st_ep_step1
pavelsavara Jan 31, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions src/coreclr/nativeaot/Runtime/eventpipe/ep-rt-aot.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -512,6 +512,26 @@ ep_rt_sample_profiler_write_sampling_event_for_threads (
ep_rt_aot_sample_profiler_write_sampling_event_for_threads (sampling_thread, sampling_event);
}

static
inline
void
ep_rt_sample_profiler_enabled (void)
{
STATIC_CONTRACT_NOTHROW;

// no-op
}

static
inline
void
ep_rt_sample_profiler_disabled (void)
{
STATIC_CONTRACT_NOTHROW;

// no-op
}

static
inline
void
Expand Down
20 changes: 20 additions & 0 deletions src/coreclr/vm/eventing/eventpipe/ep-rt-coreclr.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -567,6 +567,26 @@ ep_rt_sample_profiler_write_sampling_event_for_threads (
ep_rt_coreclr_sample_profiler_write_sampling_event_for_threads (sampling_thread, sampling_event);
}

static
inline
void
ep_rt_sample_profiler_enabled (void)
{
STATIC_CONTRACT_NOTHROW;

// no-op
}

static
inline
void
ep_rt_sample_profiler_disabled (void)
{
STATIC_CONTRACT_NOTHROW;

// no-op
}

static
inline
void
Expand Down
4 changes: 2 additions & 2 deletions src/mono/System.Private.CoreLib/System.Private.CoreLib.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@
<FeatureSingleThread Condition="('$(TargetsBrowser)' == 'true' or '$(TargetsWasi)' == 'true') and '$(WasmEnableThreads)' != 'true'">true</FeatureSingleThread>
<FeaturePortableTimer Condition="('$(TargetsBrowser)' != 'true' and '$(TargetsWasi)' != 'true') or '$(FeatureWasmManagedThreads)' == 'true'">true</FeaturePortableTimer>
<FeaturePortableThreadPool Condition="('$(TargetsBrowser)' != 'true' and '$(TargetsWasi)' != 'true') or '$(FeatureWasmManagedThreads)' == 'true'">true</FeaturePortableThreadPool>
<FeaturePerfTracing Condition="('$(TargetsBrowser)' != 'true' and '$(TargetsWasi)' != 'true')">true</FeaturePerfTracing>
<FeaturePerfTracing Condition="('$(TargetsWasi)' != 'true')">true</FeaturePerfTracing>
<FeatureObjCMarshal Condition="'$(TargetsOSX)' == 'true' or '$(TargetsMacCatalyst)' == 'true' or '$(TargetsiOS)' == 'true' or '$(TargetstvOS)' == 'true'">true</FeatureObjCMarshal>
</PropertyGroup>

Expand All @@ -135,7 +135,7 @@
<ILLinkDescriptorsXmls Include="$(ILLinkDirectory)ILLink.Descriptors.xml" />
<ILLinkDescriptorsXmls Include="$(ILLinkDirectory)ILLink.Descriptors.OSX.xml" Condition="'$(TargetsOSX)' == 'true' or '$(TargetsMacCatalyst)' == 'true' or '$(TargetsiOS)' == 'true' or '$(TargetstvOS)' == 'true'" />
<ILLinkDescriptorsXmls Include="$(CoreLibSharedDir)ILLink\ILLink.Descriptors.Shared.xml" />
<ILLinkDescriptorsXmls Condition="'$(FeaturePerfTracing)' == 'true'" Include="$(CoreLibSharedDir)ILLink\ILLink.Descriptors.EventSource.xml" />
<ILLinkDescriptorsLibraryBuildXml Condition="'$(FeaturePerfTracing)' == 'true'" Include="$(CoreLibSharedDir)ILLink\ILLink.Descriptors.EventSource.xml" />
Copy link
Member

Choose a reason for hiding this comment

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

Should this be moved to https://github.com/dotnet/runtime/blob/bfa02766bdde4657ddd1f734e274d43e6ba3c036/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems#L67 where others are?

Copy link
Member Author

Choose a reason for hiding this comment

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

I wonder what is rooting it for x64 NAOT, when trimmed

Copy link
Member

Choose a reason for hiding this comment

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

This make sure we don't link out the InitializeDefaultEventSources method called during Mono startup to setup runtime specific event sources. Mono root this function through above file and CoreClr does a similar thing, but CoreClr uses additional tasks to root methods called by runtime by scanning different source files, like corlib.h and generate a ILLink.Descriptors.xml as part of build. NativeAOT uses ModuleInitializer preventing this function from being linked out when running ilc on the published app but not exactly sure how its rooting it as part of S.P.C build, but I assume it ends up with similar mechanism.

Copy link
Member

Choose a reason for hiding this comment

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

btw, why do we need to change fro ILLinkDescriptorsXmls to ILLinkDescriptorsLibraryBuildXml for just his specific item?

Copy link
Member Author

Choose a reason for hiding this comment

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

We run ILLink again during publish build step on customer dev machine. This file needs to be there to protect it from trimming. With ILLinkDescriptorsXmls it didn't work for me. Is this the right way ?

Copy link
Member

@lateralusX lateralusX Jan 31, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

For publish on dev machines, EventSource related logic is kept using linker msbuild property, EventSourceSupport. That is how it works on all other platforms so I assume it should work the same on WASM. It should keep all EventSource related managed code.


<ILLinkSubstitutionsXmls Include="$(ILLinkDirectory)ILLink.Substitutions.$(Platform).xml" Condition="Exists('$(ILLinkDirectory)ILLink.Substitutions.$(Platform).xml')" />
<ILLinkSubstitutionsXmls Include="$(ILLinkDirectory)ILLink.Substitutions.iOS.xml" Condition="'$(TargetsiOS)' == 'true' or '$(TargetstvOS)' == 'true'" />
Expand Down
4 changes: 4 additions & 0 deletions src/mono/browser/browser.proj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -424,6 +424,8 @@
<_CmakeEnvironmentVariable Include="ENABLE_JS_INTEROP_BY_VALUE=0" Condition="'$(WasmEnableJsInteropByValue)' == 'false'"/>
<_CmakeEnvironmentVariable Include="WASM_ENABLE_SIMD=1" Condition="'$(WasmEnableSIMD)' != 'false'" />
<_CmakeEnvironmentVariable Include="WASM_ENABLE_SIMD=0" Condition="'$(WasmEnableSIMD)' == 'false'" />
<_CmakeEnvironmentVariable Include="FEATURE_PERFTRACING=1" Condition="'$(FeaturePerfTracing)' != 'false'" />
Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe there is same problem that Maraf reported

<_CmakeEnvironmentVariable Include="FEATURE_PERFTRACING=0" Condition="'$(FeaturePerfTracing)' == 'false'" />
<_CmakeEnvironmentVariable Include="WASM_ENABLE_EH=1" Condition="'$(WasmEnableExceptionHandling)' != 'false'" />
<_CmakeEnvironmentVariable Include="WASM_ENABLE_EH=0" Condition="'$(WasmEnableExceptionHandling)' == 'false'" />
<_CmakeEnvironmentVariable Include="RUN_AOT_COMPILATION=1" Condition="'$(RunAOTCompilation)' == 'true'" />
Expand Down Expand Up @@ -568,6 +570,8 @@
<_MonoRollupEnvironmentVariable Include="WasmEnableThreads:$(WasmEnableThreads)" />
<_MonoRollupEnvironmentVariable Include="WASM_ENABLE_SIMD:1" Condition="'$(WasmEnableSIMD)' != 'false'" />
<_MonoRollupEnvironmentVariable Include="WASM_ENABLE_SIMD:0" Condition="'$(WasmEnableSIMD)' == 'false'" />
<_MonoRollupEnvironmentVariable Include="FEATURE_PERFTRACING:1" Condition="'$(FeaturePerfTracing)' != 'false'" />
Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe there is same problem that Maraf reported

<_MonoRollupEnvironmentVariable Include="FEATURE_PERFTRACING:0" Condition="'$(FeaturePerfTracing)' == 'false'" />
<_MonoRollupEnvironmentVariable Include="WASM_ENABLE_EH:1" Condition="'$(WasmEnableExceptionHandling)' != 'false'" />
<_MonoRollupEnvironmentVariable Include="WASM_ENABLE_EH:0" Condition="'$(WasmEnableExceptionHandling)' == 'false'" />
<_MonoRollupEnvironmentVariable Include="ENABLE_JS_INTEROP_BY_VALUE:1" Condition="'$(WasmEnableJsInteropByValue)' == 'true'" />
Expand Down
3 changes: 3 additions & 0 deletions src/mono/browser/build/BrowserWasmApp.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
<!-- Post Wasm MVP features -->
<WasmEnableExceptionHandling Condition="'$(WasmEnableExceptionHandling)' == ''">true</WasmEnableExceptionHandling>
<WasmEnableSIMD Condition="'$(WasmEnableSIMD)' == ''">$(WasmEnableExceptionHandling)</WasmEnableSIMD>
<FeaturePerfTracing Condition="'$(FeaturePerfTracing)' == '' and '$(Configuration)' == 'Debug'">true</FeaturePerfTracing>
</PropertyGroup>

<Import Project="$(WasmCommonTargetsPath)WasmApp.Common.targets" />
Expand Down Expand Up @@ -362,6 +363,8 @@
<EmscriptenEnvVars Include="ENABLE_JS_INTEROP_BY_VALUE=0" Condition="'$(WasmEnableJsInteropByValue)' != 'true'" />
<EmscriptenEnvVars Include="WASM_ENABLE_SIMD=1" Condition="'$(WasmEnableSIMD)' != 'false'" />
<EmscriptenEnvVars Include="WASM_ENABLE_SIMD=0" Condition="'$(WasmEnableSIMD)' == 'false'" />
<EmscriptenEnvVars Include="FEATURE_PERFTRACING=1" Condition="'$(FeaturePerfTracing)' != 'false'" />
Copy link
Member

Choose a reason for hiding this comment

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

Is this in contradiction with default value set on L6?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nope, this is just passing the value (which doesn't have to be same as default) to emscripten as env variable.

Copy link
Member

Choose a reason for hiding this comment

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

Configuration=Debug => FeaturePerfTracing=true => FEATURE_PERFTRACING=1
Configuration=Release => FeaturePerfTracing='' => FEATURE_PERFTRACING=1

What is the reason to set FeaturePerfTracing based on Configuration?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you!

Copy link
Member Author

Choose a reason for hiding this comment

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

CoreCLR includes EP always. In Browser, I would like to include it when workload+Debug. Unless user specified FeaturePerfTracing explicitly. I missed the Release -> empty branch. Fixed now.

Copy link
Member

@lateralusX lateralusX Jan 31, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

In Mono we have components meaning that an app can either statically or dynamically enable parts of the runtime when linking the app, using the same runtime build. EventPipe uses this (hot reload, debugger as well) and our mobile platforms uses it to enable/disable EventPipe using msbuild flags when building the app. Recommendation is to exclude it for release builds that should be pushed to app stores, but include it for debug and profiling builds of the apps used for debugging or testing.

<EmscriptenEnvVars Include="FEATURE_PERFTRACING=0" Condition="'$(FeaturePerfTracing)' == 'false'" />
<EmscriptenEnvVars Include="WASM_ENABLE_EH=1" Condition="'$(WasmEnableExceptionHandling)' != 'false'" />
<EmscriptenEnvVars Include="WASM_ENABLE_EH=0" Condition="'$(WasmEnableExceptionHandling)' == 'false'" />
<EmscriptenEnvVars Include="ENABLE_AOT_PROFILER=$([System.Convert]::ToInt32($(WasmProfilers.Contains('aot'))))" />
Expand Down
2 changes: 1 addition & 1 deletion src/mono/browser/build/WasmApp.InTree.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
<!-- This is temporary hack for https://github.com/dotnet/runtime/issues/61294 -->
<ItemGroup>
<_MonoRuntimeComponentDontLink Include="libmono-component-debugger-stub-static.a" />
<_MonoRuntimeComponentDontLink Include="libmono-component-diagnostics_tracing-static.a"/>
<_MonoRuntimeComponentDontLink Include="libmono-component-diagnostics_tracing-stub-static.a" />
<_MonoRuntimeComponentDontLink Include="libmono-component-hot_reload-stub-static.a" />
<_MonoRuntimeComponentDontLink Include="libmono-component-marshal-ilgen-stub-static.a" />
</ItemGroup>
Expand Down
1 change: 1 addition & 0 deletions src/mono/browser/runtime/corebindings.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ extern void mono_wasm_invoke_js_function (int function_js_handle, void *args);
extern int mono_runtime_run_module_cctor (MonoImage *image, MonoError *error);

typedef void (*background_job_cb)(void);
typedef int (*ds_job_cb)(void* data);

void mono_wasm_bind_assembly_exports (char *assembly_name);
void mono_wasm_assembly_get_entry_point (char *assembly_name, int auto_insert_breakpoint, MonoMethod **method_out);
Expand Down
2 changes: 2 additions & 0 deletions src/mono/browser/runtime/cwraps.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ const fn_signatures: SigLine[] = [
[true, "mono_wasm_parse_runtime_options", null, ["number", "number"]],
[true, "mono_wasm_strdup", "number", ["string"]],
[true, "mono_background_exec", null, []],
[true, "mono_wasm_ds_exec", null, []],
[true, "mono_wasm_execute_timer", null, []],
[true, "mono_wasm_load_icu_data", "number", ["number"]],
[false, "mono_wasm_add_assembly", "number", ["string", "number", "number"]],
Expand Down Expand Up @@ -166,6 +167,7 @@ export interface t_Cwraps {
mono_wasm_strdup(value: string): number;
mono_wasm_parse_runtime_options(length: number, argv: VoidPtr): void;
mono_background_exec(): void;
mono_wasm_ds_exec(): void;
mono_wasm_execute_timer(): void;
mono_wasm_load_icu_data(offset: VoidPtr): number;
mono_wasm_add_assembly(name: string, data: VoidPtr, size: number): number;
Expand Down
2 changes: 2 additions & 0 deletions src/mono/browser/runtime/es6/dotnet.es6.lib.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

// because we can't pass custom define symbols to acorn optimizer, we use environment variables to pass other build options
const WASM_ENABLE_SIMD = process.env.WASM_ENABLE_SIMD === "1";
const FEATURE_PERFTRACING = process.env.FEATURE_PERFTRACING === "1";
const WASM_ENABLE_EH = process.env.WASM_ENABLE_EH === "1";
const ENABLE_BROWSER_PROFILER = process.env.ENABLE_BROWSER_PROFILER === "1";
const ENABLE_AOT_PROFILER = process.env.ENABLE_AOT_PROFILER === "1";
Expand Down Expand Up @@ -86,6 +87,7 @@ function injectDependencies() {
DotnetSupportLib["$DOTNET__postset"] = `DOTNET.setup({ ` +
`wasmEnableSIMD: ${WASM_ENABLE_SIMD ? "true" : "false"},` +
`wasmEnableEH: ${WASM_ENABLE_EH ? "true" : "false"},` +
`enablePerfTracing: ${FEATURE_PERFTRACING ? "true" : "false"}, ` +
`enableAotProfiler: ${ENABLE_AOT_PROFILER ? "true" : "false"}, ` +
`enableBrowserProfiler: ${ENABLE_BROWSER_PROFILER ? "true" : "false"}, ` +
`enableLogProfiler: ${ENABLE_LOG_PROFILER ? "true" : "false"}, ` +
Expand Down
9 changes: 8 additions & 1 deletion src/mono/browser/runtime/exports-binding.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { mono_wasm_resolve_or_reject_promise } from "./marshal-to-js";
import { mono_wasm_schedule_timer, schedule_background_exec } from "./scheduling";
import { mono_wasm_asm_loaded } from "./startup";
import { mono_log_warn, mono_wasm_console_clear, mono_wasm_trace_logger } from "./logging";
import { mono_wasm_profiler_leave, mono_wasm_profiler_enter } from "./profiler";
import { mono_wasm_profiler_leave, mono_wasm_profiler_enter, ds_rt_websocket_close, ds_rt_websocket_create, ds_rt_websocket_poll, ds_rt_websocket_recv, ds_rt_websocket_send } from "./profiler";
import { mono_wasm_browser_entropy } from "./crypto";
import { mono_wasm_cancel_promise } from "./cancelable-promise";

Expand Down Expand Up @@ -88,6 +88,13 @@ export const mono_wasm_imports = [
mono_wasm_invoke_jsimport_ST,
mono_wasm_resolve_or_reject_promise,
mono_wasm_cancel_promise,

//event pipe
ds_rt_websocket_create,
ds_rt_websocket_send,
ds_rt_websocket_poll,
ds_rt_websocket_recv,
ds_rt_websocket_close,
];


Expand Down
23 changes: 23 additions & 0 deletions src/mono/browser/runtime/profiler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

import type { CharPtr, VoidPtr } from "./types/emscripten";

import { ENVIRONMENT_IS_WEB, mono_assert, runtimeHelpers } from "./globals";
import { MonoMethod, AOTProfilerOptions, BrowserProfilerOptions, LogProfilerOptions } from "./types/internal";
import { profiler_c_functions as cwraps } from "./cwraps";
Expand Down Expand Up @@ -105,3 +107,24 @@ export function mono_wasm_profiler_leave (method: MonoMethod): void {
globalThis.performance.measure(methodName, options);
}
}

/* eslint-disable @typescript-eslint/no-unused-vars */
export function ds_rt_websocket_create (urlPtr :CharPtr):number {
throw new Error("TODO");
}

export function ds_rt_websocket_send (client_socket :number, buffer:VoidPtr, bytes_to_write:number):number {
throw new Error("TODO");
}

export function ds_rt_websocket_poll (client_socket :number):number {
throw new Error("TODO");
}

export function ds_rt_websocket_recv (client_socket :number, buffer:VoidPtr, bytes_to_read:number):number {
throw new Error("TODO");
}

export function ds_rt_websocket_close (client_socket :number):number {
throw new Error("TODO");
}
2 changes: 2 additions & 0 deletions src/mono/browser/runtime/rollup.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ const nativeBinDir = process.env.NativeBinDir ? process.env.NativeBinDir.replace
const wasmObjDir = process.env.WasmObjDir ? process.env.WasmObjDir.replace(/"/g, "") : "obj";
const wasmEnableThreads = process.env.WasmEnableThreads === "true" ? true : false;
const wasmEnableSIMD = process.env.WASM_ENABLE_SIMD === "1" ? true : false;
const wasmEnablePerfTracing = process.env.FEATURE_PERFTRACING === "1" ? true : false;
const wasmEnableExceptionHandling = process.env.WASM_ENABLE_EH === "1" ? true : false;
const wasmEnableJsInteropByValue = process.env.ENABLE_JS_INTEROP_BY_VALUE == "1" ? true : false;
// because of stack walk at src/mono/browser/debugger/BrowserDebugProxy/MonoProxy.cs
Expand Down Expand Up @@ -108,6 +109,7 @@ const envConstants = {
configuration,
wasmEnableThreads,
wasmEnableSIMD,
wasmEnablePerfTracing,
wasmEnableExceptionHandling,
gitHash,
wasmEnableJsInteropByValue,
Expand Down
1 change: 1 addition & 0 deletions src/mono/browser/runtime/types/internal.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,7 @@ export type EmscriptenBuildOptions = {
enableAotProfiler: boolean,
enableBrowserProfiler: boolean,
enableLogProfiler: boolean,
enablePerfTracing: boolean,
runAOTCompilation: boolean,
wasmEnableThreads: boolean,
gitHash: string,
Expand Down
8 changes: 7 additions & 1 deletion src/mono/mono.proj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -698,12 +698,18 @@ JS_ENGINES = [NODE_JS]
<_MonoCMakeArgs Include="-DFEATURE_PERFTRACING_DISABLE_DEFAULT_LISTEN_PORT=1"/>
</ItemGroup>

<ItemGroup Condition="'$(TargetsBrowser)' == 'true' or '$(TargetsWasi)' == 'true'">
<ItemGroup Condition="'$(TargetsWasi)' == 'true'">
<_MonoCMakeArgs Include="-DFEATURE_PERFTRACING_DISABLE_PERFTRACING_LISTEN_PORTS=1"/>
<_MonoCMakeArgs Include="-DFEATURE_PERFTRACING_DISABLE_DEFAULT_LISTEN_PORT=1"/>
<_MonoCMakeArgs Include="-DFEATURE_PERFTRACING_DISABLE_CONNECT_PORTS=1" />
</ItemGroup>

<ItemGroup Condition="'$(TargetsBrowser)' == 'true'">
<_MonoCMakeArgs Include="-DFEATURE_PERFTRACING_PAL_WS=1"/>
<_MonoCMakeArgs Include="-DFEATURE_PERFTRACING_DISABLE_PERFTRACING_LISTEN_PORTS=1"/>
<_MonoCMakeArgs Include="-DFEATURE_PERFTRACING_DISABLE_DEFAULT_LISTEN_PORT=1"/>
</ItemGroup>

<!-- Components -->
<ItemGroup Condition="'$(MonoComponentsStatic)' == 'true'">
<_MonoCMakeArgs Include="-DSTATIC_COMPONENTS=1" />
Expand Down
9 changes: 9 additions & 0 deletions src/mono/mono/component/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,15 @@ endif()
if (ENABLE_PERFTRACING AND "${MONO_DIAGNOSTICS_TRACING_COMPONENT_NAME}" IN_LIST components_to_build)
# Build EventPipe and DiagnosticServer with the Mono runtime implementation as unity-builds.
add_library(eventpipe-mono-objects OBJECT)

if (DISABLE_THREADS)
set_target_properties(
eventpipe-mono-objects
Copy link
Member

Choose a reason for hiding this comment

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

Do we need the wider Mono specific DISABLE_THREADS here or is this just needed since we have some checks in diagnostics_server.c that currently uses it? If so, then maybe we should change that code to use the eventpipe define for single threaded/disabled threads. Alternative could be to define this based on the more generic event pipe as part of mono specific ds or ep config's.

Copy link
Member Author

Choose a reason for hiding this comment

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

Here we are passing it to src\native\eventpipe\CMakeLists.txt so that it could set FEATURE_PERFTRACING_SINGLE_THREADED

PROPERTIES
DISABLE_THREADS ON
)
endif()

set_target_properties(
eventpipe-mono-objects
PROPERTIES
Expand Down
95 changes: 95 additions & 0 deletions src/mono/mono/eventpipe/ep-rt-mono-runtime-provider.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,14 @@ extern EVENTPIPE_TRACE_CONTEXT MICROSOFT_WINDOWS_DOTNETRUNTIME_RUNDOWN_PROVIDER_
#define NUM_NANOSECONDS_IN_1_MS 1000000

// Sample profiler.
#if defined(PERFTRACING_MULTI_THREADED)
Copy link
Member

Choose a reason for hiding this comment

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

I will just make this comment once, but it applies through all changes and is basically an effect of the last comment in this review. Multithreaded is the default, so we shouldn't need an explicit define for multithreaded support, instead we should use one define disabling threading.

Copy link
Member Author

Choose a reason for hiding this comment

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

you prefer #if !defined(PERFTRACING_SINGLE_THREADED) everywhere, right ?

static GArray * _sampled_thread_callstacks = NULL;
static uint32_t _max_sampled_thread_count = 32;
#else
MonoProfilerHandle _ep_rt_mono_sampling_profiler_provider;
static EventPipeEvent *current_sampling_event = NULL;
static ep_rt_thread_handle_t current_sampling_thread = NULL;
#endif

// Mono profilers.
extern MonoProfilerHandle _ep_rt_mono_default_profiler_provider;
Expand Down Expand Up @@ -1207,6 +1213,7 @@ ep_rt_mono_walk_managed_stack_for_thread (
return true;
}

#if defined(PERFTRACING_MULTI_THREADED)
bool
ep_rt_mono_sample_profiler_write_sampling_event_for_threads (
ep_rt_thread_handle_t sampling_thread,
Expand Down Expand Up @@ -1306,6 +1313,94 @@ ep_rt_mono_sample_profiler_write_sampling_event_for_threads (
return true;
}

#else // !PERFTRACING_MULTI_THREADED

bool
ep_rt_mono_sample_profiler_write_sampling_event_for_threads (
Copy link
Member

Choose a reason for hiding this comment

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

How frequent will this be called on WASM? When we run regular sample profiler this is called on the timer interval defined in __sampling_rate_in_ns. Looking further down it appears that this method will only be called once, is that correct?

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct, I just want to capture the sampling_event and sampling_thread to be able to use it later.

ep_rt_thread_handle_t sampling_thread,
EventPipeEvent *sampling_event)
{
// here we just store the context for later
current_sampling_event = sampling_event;
current_sampling_thread = sampling_thread;

return true;
}

static void
method_enter (MonoProfiler *prof, MonoMethod *method, MonoProfilerCallContext *ctx)
Copy link
Member

Choose a reason for hiding this comment

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

Should we sample on every call? Sample profiler only samples at a specific time interval, like very ms, setup in __sampling_rate_in_ns. Maybe we should only sample based on the sample rate instead on every call to be more inline with how sampling works on all other platforms.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's right, it needs to consider sample frequency. But also I will bring another instrumentation for like safepoint.
I will make those improvements in next PR

{
MonoThreadInfo *thread_info = mono_thread_info_current ();
SampleProfileStackWalkData stack_walk_data;
SampleProfileStackWalkData *data= &stack_walk_data;
THREAD_INFO_TYPE adapter = { { 0 } };

data->thread_id = ep_rt_thread_id_t_to_uint64_t (mono_thread_info_get_tid (thread_info));
data->thread_ip = (uintptr_t)MONO_CONTEXT_GET_IP (&ctx->context);
data->payload_data = EP_SAMPLE_PROFILER_SAMPLE_TYPE_ERROR;
data->stack_walk_data.stack_contents = &data->stack_contents;
data->stack_walk_data.top_frame = true;
data->stack_walk_data.async_frame = false;
data->stack_walk_data.safe_point_frame = false;
data->stack_walk_data.runtime_invoke_frame = false;
ep_stack_contents_reset (&data->stack_contents);

mono_get_eh_callbacks ()->mono_walk_stack_with_ctx (sample_profiler_walk_managed_stack_for_thread_callback, &ctx->context, MONO_UNWIND_SIGNAL_SAFE, &stack_walk_data);
if (data->payload_data == EP_SAMPLE_PROFILER_SAMPLE_TYPE_EXTERNAL && (data->stack_walk_data.safe_point_frame || data->stack_walk_data.runtime_invoke_frame)) {
data->payload_data = EP_SAMPLE_PROFILER_SAMPLE_TYPE_MANAGED;
}
if (data->stack_walk_data.top_frame && ep_stack_contents_get_length (&data->stack_contents) == 0) {
data->payload_data = EP_SAMPLE_PROFILER_SAMPLE_TYPE_EXTERNAL;
}

if ((data->stack_walk_data.top_frame && data->payload_data == EP_SAMPLE_PROFILER_SAMPLE_TYPE_EXTERNAL) || (data->payload_data != EP_SAMPLE_PROFILER_SAMPLE_TYPE_ERROR && ep_stack_contents_get_length (&data->stack_contents) > 0)) {
if (data->stack_walk_data.async_frame) {
for (uint32_t frame_count = 0; frame_count < data->stack_contents.next_available_frame; ++frame_count)
mono_jit_info_table_find_internal ((gpointer)data->stack_contents.stack_frames [frame_count], TRUE, FALSE);
}
mono_thread_info_set_tid (&adapter, ep_rt_uint64_t_to_thread_id_t (data->thread_id));
uint32_t payload_data = ep_rt_val_uint32_t (data->payload_data);
ep_write_sample_profile_event (current_sampling_thread, current_sampling_event, &adapter, &data->stack_contents, (uint8_t *)&payload_data, sizeof (payload_data));
}
}

static MonoProfilerCallInstrumentationFlags
method_filter (MonoProfiler *prof, MonoMethod *method)
{
// TODO add more instrumentation, something like MINT_SDB_SEQ_POINT
return MONO_PROFILER_CALL_INSTRUMENTATION_ENTER;
Copy link
Member

Choose a reason for hiding this comment

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

This will cause instrumentation on all method calls and it looks like you can't opt out of it meaning you will always take the performance hit even if you won't use sample profiler.

Another problem is that you will only be able to sample on function enter, meaning that your samples will be clustered on start of functions. If you have long loops without calls, those will also be blind spots and you won't get any sampling events during that time.

Mono has the concept of safepoints when runtime runs in hybrid or coop suspend mode. Using that mechanism would behave similar to how Android/iOS works where we could sample on calls, loop back edges and some other scenarios. Problem is that WASM probably runs in preemptive suspend mode since it only have one thread, meaning that it won't include safepoints in the generated code and we probably don't want to enable cooperate suspend just for this, so in that case we would need to separate the safepoint handling and suspend model, but something would still need to signal the thread to stop on safepoint and right now thread will only suspend its execution and another "thread" will need to do the work (like running GC or sampling) and I assume that won't work on WASM either, meaning piggy back on safepoints for this is probably not an alternative.

Is the intention that we should support sampling only when running under interpreter or should this support AOT:ed code as well?

When do we leave managed code getting back to the browser event loop? Does it happen when we yield, doing wait calls, other sys calls or do we have other instrumentation that could run browser event loop in the middle of executing managed code? Just trying to understand the execution model to see what alternatives we might have based on WASM execution model.

Copy link
Member Author

Choose a reason for hiding this comment

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

Safepoint like instrumentation of interp sampling will be next PR.
AOT should do something similar too but I have not investigated yet.
There is also WASM jiterp, which needs to make this work.

We can't run browser event loop in the middle of executing managed code.

}

#endif // PERFTRACING_MULTI_THREADED

void
ep_rt_mono_sampling_provider_component_init (void)
{
#if defined(PERFTRACING_SINGLE_THREADED)
// in single-threaded mode, we install instrumentation callbacks on the mono profiler, instead of stop-the-world
_ep_rt_mono_sampling_profiler_provider = mono_profiler_create (NULL);
// this has negative performance impact even when the EP client is not connected!
// but it has to be enabled before managed code starts running, because the instrumentation needs to be in place
mono_profiler_set_call_instrumentation_filter_callback (_ep_rt_mono_sampling_profiler_provider, method_filter);
Copy link
Member

Choose a reason for hiding this comment

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

Maybe set this callback to NULL when shutting down, similar to how we do with other profilers in Mono eventpipe.

Copy link
Member Author

Choose a reason for hiding this comment

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

This would mean that interp could do IL->IR without instrumentation in the meantime, right ?
And you would not see those ever after in the sample.
This is why I set it up during startup, not later.

#endif
}

void
ep_rt_mono_sample_profiler_enabled (void)
{
#if defined(PERFTRACING_SINGLE_THREADED)
mono_profiler_set_method_enter_callback (_ep_rt_mono_sampling_profiler_provider, method_enter);
#endif
}

void
ep_rt_mono_sample_profiler_disabled (void)
{
#if defined(PERFTRACING_SINGLE_THREADED)
mono_profiler_set_method_enter_callback (_ep_rt_mono_sampling_profiler_provider, NULL);
#endif
}

void
ep_rt_mono_execute_rundown (dn_vector_ptr_t *execution_checkpoints)
{
Expand Down
1 change: 1 addition & 0 deletions src/mono/mono/eventpipe/ep-rt-mono.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -789,6 +789,7 @@ ep_rt_mono_component_init (void)
g_free (diag_env);

ep_rt_mono_runtime_provider_component_init ();
ep_rt_mono_sampling_provider_component_init ();
ep_rt_mono_profiler_provider_component_init ();
}

Expand Down
Loading
Loading