[browser] EventPipe - CPU sampling - single thread #113758
Merged
Conversation
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
pavelsavara
added
arch-wasm
38 tasks
pavelsavara
force-pushed
the
browser_ep_sampling
branch
from
c1b4ef2 to
556f3b9
Compare
pavelsavara
commented
Mar 21, 2025
pavelsavara
commented
Mar 21, 2025
This was referenced Mar 21, 2025
pavelsavara
force-pushed
the
browser_ep_sampling
branch
from
be32979 to
d9161ed
Compare
pavelsavara
requested review from
BrzVlad,
akoeplinger,
kotlarmilos,
lateralusX,
lewing,
maraf,
steveisok and
vitek-karas
as code owners
Open
3 tasks
- add LMF to profiler in interp and jiterp
This was referenced Mar 24, 2025
maraf
reviewed
Mar 25, 2025
lateralusX
reviewed
Mar 26, 2025
There was a problem hiding this comment.
Mainly looked at the native code. Overall, it looks good, some comments are not actionable in this PR and I might convert them into issues later, but wanted to include them since this PR surface the underlying issues, other comments are more questions or checking assumptions and some are actually actionable :)
- session->refcount, ep_session_dec_ref, ep_session_inc_ref - conditional default_profiler_sample_rate_in_nanoseconds and use of ep_sample_profiler_get_sampling_rate() - don't register for MONO_PROFILER_CALL_INSTRUMENTATION_LEAVE and MONO_PROFILER_CALL_INSTRUMENTATION_TAIL_CALL - inlining feedback for method_enter, method_samplepoint, method_exc_leave - LMF feedback for mono_jiterp_prof_enter, mono_jiterp_prof_samplepoint, mono_jiterp_prof_leave
lateralusX
reviewed
Mar 27, 2025
lateralusX
reviewed
Mar 27, 2025
lateralusX
reviewed
Mar 27, 2025
lateralusX
approved these changes
Mar 27, 2025
|
/ba-g CI timeouts |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
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.
single-threaded sampling in EP -
ep-rt-mono-runtime-provider.cshould_record_sample()calculation about when to take next samplesample_current_thread_stack_tracewithoutMonoContext *because in interp we don't have itmono_wasm_instrument_methodto disable the instrumentation. Could be some callspec in the future.Fixes
mono_wasm_profiler_free_methodconditionep_session_freeuse-after-free in STINTERP_PROFILER_RAISEto not always create contextep_rt_wait_event_freeIPon jiterpMonoProfilerCallContextallocation feedbackconstfeedbackFlags
WASM_PERFTRACINGin rollup envDOTNET_WasmPerfInstrumentation, opt in viaWasmPerfInstrumentationMSBuild propertyMetricsSupportby default for browserTesting: I'm doing manual testing with next PR which contains the JavaScript client. That's lot of TypeScript which would take attention away from C details in this PR. And so I split this PR as separate step.
Contributes to #76316
On top of #112352