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

Jump to bottom

EventPipe: Stacks for sampling events are being discarded #12837

Closed
noahfalk opened this issue Jun 10, 2019 · 1 comment
Closed

EventPipe: Stacks for sampling events are being discarded #12837

noahfalk opened this issue Jun 10, 2019 · 1 comment
Assignees
@noahfalk
Labels
area-Tracing-coreclr EventPipe
Milestone
3.0

Comments

@noahfalk
Copy link
Member

Running the stack sampling provider produces events that have no stacks. This appears to be a recent regression between Preview5 and Preview6 and I tracked it to this line:

https://github.com/dotnet/coreclr/blob/master/src/vm/eventpipesession.cpp#L311

The line should read:

m_pBufferManager->WriteEvent(pThread, *this, event, payload, pActivityId, pRelatedActivityId, pEventThread, pStack)

cc @dotnet/dotnet-diag
@noahfalk noahfalk self-assigned this Jun 10, 2019
noahfalk referenced this issue in noahfalk/coreclr Jun 10, 2019
@noahfalk
Fix Linux build issues and misc bugs
0cf5b4e 
Bug fix list:
1) Pre-existing bug #25046
2) A number of places I had broken the old format when implementing the new one:
  a) Didn't write the event payload into the file due to unintended hiding of the dataLength variable
  b) NetTrace header was being applied regardless of format version
  c) Stack interning was being applied regardless of format version
  d) Final sequence point emitted regardless of format version
  e) Metadata blocks being used regardless of format version
3) AdvanceToNonEmptyBuffer wasn't converting the first buffer to read-only
4) CreateThread wasn't handling when the out pointer for ID was null
noahfalk referenced this issue in noahfalk/coreclr Jun 10, 2019
@noahfalk
Fix Linux build issues and misc bugs
f9c6a7f 
Bug fix list:
1) Pre-existing bug #25046
2) A number of places I had broken the old format when implementing the new one:
  a) Didn't write the event payload into the file due to unintended hiding of the dataLength variable
  b) NetTrace header was being applied regardless of format version
  c) Stack interning was being applied regardless of format version
  d) Final sequence point emitted regardless of format version
  e) Metadata blocks being used regardless of format version
3) AdvanceToNonEmptyBuffer wasn't converting the first buffer to read-only
4) CreateThread wasn't handling when the out pointer for ID was null
5) A missing IncrementSequenceNumber() was causing us not to catch dropped events
noahfalk referenced this issue in noahfalk/coreclr Jun 10, 2019
@noahfalk
Add support for the NetTrace EventPipe file format
932418d 
Right now the new format is not on by default, but it can be enabled using COMPlus_EventPipeNetTraceFormat = 1 for testing purposes. The plan to have a follow up PR that will add shipping configuration mechanisms and change the default setting.

See the documentation in the PerfView repo for more details about the format. At a glance the goal is to create a format that is more efficient to produce, has a smaller on disk size, and offers enhanced functionality in a few areas:
a) 64 bit thread id support
b) Detection of dropped events via sequence numbers
c) Better support for extracting subsets of the file

Together with the change there was also some refactoring of the EventPipeBufferManager and EventPipeThread.

This change addresses (at least in part) the following issues:
#19688, #23414, #24188, #20751, #20555, #21827, #24852, #25046
noahfalk referenced this issue in dotnet/coreclr Jun 10, 2019
@noahfalk
Add support for the NetTrace EventPipe file format
835836c 
Right now the new format is not on by default, but it can be enabled using COMPlus_EventPipeNetTraceFormat = 1 for testing purposes. The plan to have a follow up PR that will add shipping configuration mechanisms and change the default setting.

See the documentation in the PerfView repo for more details about the format. At a glance the goal is to create a format that is more efficient to produce, has a smaller on disk size, and offers enhanced functionality in a few areas:
a) 64 bit thread id support
b) Detection of dropped events via sequence numbers
c) Better support for extracting subsets of the file

Together with the change there was also some refactoring of the EventPipeBufferManager and EventPipeThread.

This change addresses (at least in part) the following issues:
#19688, #23414, #24188, #20751, #20555, #21827, #24852, #25046
@noahfalk
Copy link
Member Author

I tested the fix in my change (#24853), but looking at the code deltas I think @jorive's change fixed this independently in his earlier PR dotnet/coreclr#24896.

@msftgits msftgits transferred this issue from dotnet/coreclr Jan 31, 2020
@msftgits msftgits added this to the 3.0 milestone Jan 31, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@noahfalk noahfalk

Labels
area-Tracing-coreclr EventPipe
Projects
None yet
Milestone
3.0
Development

No branches or pull requests
2 participants
@noahfalk @msftgits