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-ep] Implement EventSource counters support; add an EventPipeSessionOptions builder to TS #69567

Merged
merged 11 commits into from
Jun 1, 2022

Conversation

lambdageek
Copy link
Member

@lambdageek lambdageek commented May 19, 2022

  1. Implements support for creating event counters in WebAssembly apps.
    Important change adds Thread.IsInternalThreadStartSupported and Thread.InternalUnsafeStart() to System.Threading.Thread that can be used by System.Private.CoreLib to start threads on a runtime where Thread.IsThreadStartSupported is false.
    This is used to create the event counters polling thread.
    This is in addition to the native runtime being able to create threads using mono_thread_create_internal
  2. Coop thread suspend: STW calls mono_threads_platform_stw_defer_initial_suspend which postpones suspending a thread until the next GC suspend phase. Doesn't do anything on most platforms. On WASM, suspend the main browser thread after all the other threads are suspended. This helps prevent deadlocks where the main thread is suspended while another thread is doing a call that is proxied to the main thread and cannot complete. By suspending the main thread last, we give it a chance to service the other threads' requests.
  3. Adds a MONO.diagnostics.SessionOptionsBuilder class that can be used to create an EventPipeSessionOptions instance and populate the provider string for an event pipe session.
  4. Updated the browser-eventpipe sample to create an EventSource and some counters. The sample is now interactive so you have to click "Start Work" in a browser for things to happen.

There's an outstanding issue #69568 that will be investigated in a follow-up PR

@ghost ghost assigned lambdageek May 19, 2022
@lambdageek lambdageek added arch-wasm WebAssembly architecture and removed area-VM-meta-mono labels May 19, 2022
@ghost
Copy link

ghost commented May 19, 2022

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

Issue Details
  1. Depends on [wasm-mt] Add coop GC transitions to various EMSCRIPTEN_KEEPALIVE functions; define to noop if no threads #69515
  2. Implements support for creating event counters in WebAssembly apps.
    Important change adds Thread.IsInternalThreadStartSupported and Thread.InternalUnsafeStart() to System.Threading.Thread that can be used by System.Private.CoreLib to start threads on a runtime where Thread.IsThreadStartSupported is false.
    This is used to create the event counters polling thread.
    This is in addition to the native runtime being able to create threads using mono_thread_create_internal
  3. Adds a MONO.diagnostics.SessionOptionsBuilder class that can be used to create an EventPipeSessionOptions instance and populate the provider string for an event pipe session.
  4. Updated the browser-eventpipe sample to create an EventSource and some counters. The sample is now interactive so you have to click "Start Work" in a browser for things to happen.
Author: lambdageek
Assignees: lambdageek
Labels:

arch-wasm

Milestone: -

@lambdageek lambdageek requested review from a team and removed request for a team May 19, 2022 17:27
@lambdageek
Copy link
Member Author

@dotnet/area-system-runtime FYI, I'm making some changes to some internal methods on System.Thread.

lambdageek and others added 8 commits May 22, 2022 13:33
- Had to set the coop GC timeout to 50 sec (might just be the
recursive fib sample)

- Added a method for coop GC to suspend the main browser thread in the
second phase (so that other threads are less likely to deadlock if they delegate
work to it)

- Added an event provider builder. (TODO: make it into a builder for
EventPipeSessionOptions)

- Added a Thread.InternalUnsafeStart() method and a
IsInternalThreadStartSupported property.  This allows EventSource to
start a thread to poll the counter values.

- The sample with counters can now record counter values.  But not at
the same time as the default configuration (runtime, runtime private,
sample profiler).  Not sure if it's a wasm issue or a limitation of
EventPipe.
it creates an entire EventPipeSessionOptions object, not just the
providers config
@lambdageek lambdageek marked this pull request as ready for review May 22, 2022 17:38
@lambdageek
Copy link
Member Author

@lateralusX @pavelsavara Can I get a review for this PR, please

internal static bool IsThreadStartSupported => true;
internal static bool IsInternalThreadStartSupported => true;
#elif FEATURE_WASM_PERFTRACING
Copy link
Member

Choose a reason for hiding this comment

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

Does this have any relationship with the wider, FEATURE_PERFTRACING used to guard all perftracing support?

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 think FEATURE_PERFTRACING can be true or false on the wasm threaded runtime, ie it's the general perftracing support. Whereas FEATURE_WASM_PERFTRACING is the specific configuration where user threading is disabled.

Copy link
Member

@lateralusX lateralusX Jun 1, 2022

Choose a reason for hiding this comment

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

Yes, FEATURE_PERFTRACING will turn on/off all diagnostics supports, both in BCL and runtime. So can you build EventPipe/Diagnostic support on WASM without user threading enabled and get something to work and would you enabled FEATURE_WASM_PERFTRACING without enabling FEATURE_PERFTRACING?

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 I should rename FEATURE_WASM_PERFTRACING to FEATURE_WASM_INTERNAL_THREADS_ONLY or something. The point is that it enables starting utility threads, but doesn't enable general user threading.

Copy link
Member

@lateralusX lateralusX left a comment

Choose a reason for hiding this comment

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

Runtime changes LGTM.

@lambdageek
Copy link
Member Author

/azp run runtime-wasm

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@lambdageek
Copy link
Member Author

/azp run runtime-wasm

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@lambdageek lambdageek merged commit 6726fae into dotnet:main Jun 1, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Jul 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants