[release/6.0] Always write sample events in EventPipe based on sample frequency. #58761
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
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.
Only include external samples still on top fram (no callstack) or managed/external samples includig a callstack.
ghost
added
the
marek-safar
approved these changes
Sep 7, 2021
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Backport of #58720 to release/6.0
/cc @lateralusX
Customer Impact
Observed in embedding scenarios using diagnostic tools like speedscope, dotnet/android#6243 on nettrace files generated on Android. These tools uses EventPipe sample profiler event to apply thread time. Since its a sample profiler, that is error prone, but since we won't write samples based on frequency in case where threads don't have any managed frames on stack (could happen on embedding thread), tools will incorrectly prolong thread time for last sample including managed frames. Fix will make sure samples are still written into EventPipe when we are in unmanaged code without managed frames on stack, assisting tooling's thread time heuristics, more accurately represent time for last sampled managed callstack. NOTE, this scenario is highly workload and implementation dependent, in cases where attached runtime threads stays in runtime (managed or unmanaged code), there will always be managed frames on the stack, this issue only happens if an attached thread returns back to native code.
Testing
Manual testing validating that tools better represent thread times in embedding scenarios.
Risk
Low, samples with 0 length callstack are handled by both Perfview, Speedscope and VS. Amount of samples in unmanaged code will increase in calltrees when this scenarios happens, mainly an issue in Perfview, but can be excluded from analysis if not needed and these events will only be included in embedding scenarios (and attached threads returns back out to native code).