Skip to content

Conversation

@lateralusX
Copy link
Member

If no managed frames are located on stack, sample was dropped and not emitted into EventPipe by sample profiler. This cause issues in tooling that try to do thread time calculations based on sample, especially in cases where embedded threads returned to native code during several samples before calling back into runtime. In such scenarios the last sampled event would be prolonged, giving false information that that stack frame lasted much longer than it really did.

Always writing samples into EventPipe will also visualize time an attached thread spend outside of runtime, not running managed code.

Fix should open up for better visualization as described in dotnet/android#6243.

We can do similar fix to CoreCLR as well:

if (ep_rt_coreclr_walk_managed_stack_for_thread (target_thread, current_stack_contents) && !ep_stack_contents_is_empty (current_stack_contents)) {
)

but I'm splitting fixes into different PR's since MonoVM's embedding scenario makes this scenario much more likely to occur (already observed in different embedding scenarios) than CoreCLR.

If no managed frames (including helper frames) are located on stack,
sample was dropped and not emitted into EventPipe. This cause issues
in tooling that try to do thread time calculations based on sample,
especially in cases where embedded threads returned to native
code during several samples before calling back into runtime. In such
scenarios the last sampled event would be prolonged, giving false
information that that stackframe lasted much longer than it really did.

Always writing samples into EventPipe will also visualize time an attached
thread spend outside of runtime, not running managed code.
@ghost ghost added the area-Tracing-mono label Sep 6, 2021
Only include external samples still on top fram (no callstack) or
managed/external samples includig a callstack.
@lateralusX
Copy link
Member Author

//CC @josalem

Copy link
Contributor

@josalem josalem left a comment

Choose a reason for hiding this comment

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

This is a good step while we rethink some of the semantics of our stackwalker. I wish there was a way to indicate this state without sending sample events, but the Sample Profiler provider doesn't have that capability and I'm not sure it's worth adding at this moment.

CC @brianrob since this will impact PerfView's parsing of stack samples (for the better)

@lateralusX
Copy link
Member Author

Perfview will collect these as unmanaged code right below the thread since there is no callstack associated with sample event, so it already handles the case, but It would be good if we could have option in Perfview to exclude events without available callstacks in for example calltree, right now you can exclude UNAMANGED_CODE_TIME, but that will exclude all occurrences, would be good to be able to exclude these occurrences happening when UNAMANGED_CODE_TIME represent a sample event in external code without any callstack.

@lateralusX
Copy link
Member Author

/backport to release/6.0

@github-actions
Copy link
Contributor

github-actions bot commented Sep 7, 2021

Started backporting to release/6.0: https://github.com/dotnet/runtime/actions/runs/1210361775

@lateralusX lateralusX merged commit b0aa6f4 into dotnet:main Sep 9, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Oct 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants