From ad7c1e85fa95a92d5cc58b93b6ca9e2b77ec86b6 Mon Sep 17 00:00:00 2001 From: Brian Robbins Date: Mon, 29 Jan 2018 16:57:01 -0800 Subject: [PATCH] EventPipe: Don't Generate and Emit Stacks on Rundown (#16083) --- src/vm/eventpipe.cpp | 8 +++++--- src/vm/eventpipebuffer.cpp | 3 ++- src/vm/eventpipebuffer.h | 3 ++- src/vm/eventpipebuffermanager.cpp | 6 +++--- src/vm/eventpipebuffermanager.h | 3 ++- src/vm/eventpipeconfiguration.cpp | 1 + src/vm/eventpipeconfiguration.h | 1 + src/vm/eventpipeeventinstance.cpp | 8 +++++--- src/vm/eventpipeeventinstance.h | 5 +++-- 9 files changed, 24 insertions(+), 14 deletions(-) diff --git a/src/vm/eventpipe.cpp b/src/vm/eventpipe.cpp index 2eeb64f..41af3ef 100644 --- a/src/vm/eventpipe.cpp +++ b/src/vm/eventpipe.cpp @@ -521,6 +521,7 @@ void EventPipe::WriteEventInternal(EventPipeEvent &event, EventPipeEventPayload NOTHROW; GC_NOTRIGGER; MODE_ANY; + PRECONDITION(s_pSession != NULL); } CONTRACTL_END; @@ -546,7 +547,7 @@ void EventPipe::WriteEventInternal(EventPipeEvent &event, EventPipeEventPayload if(!s_pConfig->RundownEnabled() && s_pBufferManager != NULL) { - if(!s_pBufferManager->WriteEvent(pThread, event, payload, pActivityId, pRelatedActivityId)) + if(!s_pBufferManager->WriteEvent(pThread, *s_pSession, event, payload, pActivityId, pRelatedActivityId)) { // This is used in DEBUG to make sure that we don't log an event synchronously that we didn't log to the buffer. return; @@ -564,6 +565,7 @@ void EventPipe::WriteEventInternal(EventPipeEvent &event, EventPipeEventPayload // B) It is unclear there is a benefit to multiple file write calls // as opposed a a buffer copy here EventPipeEventInstance instance( + *s_pSession, event, pThread->GetOSThreadId(), pData, @@ -637,7 +639,7 @@ void EventPipe::WriteSampleProfileEvent(Thread *pSamplingThread, EventPipeEvent { // Specify the sampling thread as the "current thread", so that we select the right buffer. // Specify the target thread so that the event gets properly attributed. - if(!s_pBufferManager->WriteEvent(pSamplingThread, *pEvent, payload, NULL /* pActivityId */, NULL /* pRelatedActivityId */, pTargetThread, &stackContents)) + if(!s_pBufferManager->WriteEvent(pSamplingThread, *s_pSession, *pEvent, payload, NULL /* pActivityId */, NULL /* pRelatedActivityId */, pTargetThread, &stackContents)) { // This is used in DEBUG to make sure that we don't log an event synchronously that we didn't log to the buffer. return; @@ -649,7 +651,7 @@ void EventPipe::WriteSampleProfileEvent(Thread *pSamplingThread, EventPipeEvent GCX_PREEMP(); // Create an instance for the synchronous path. - SampleProfilerEventInstance instance(*pEvent, pTargetThread, pData, length); + SampleProfilerEventInstance instance(*s_pSession, *pEvent, pTargetThread, pData, length); stackContents.CopyTo(instance.GetStack()); // Write to the EventPipeFile. diff --git a/src/vm/eventpipebuffer.cpp b/src/vm/eventpipebuffer.cpp index 407c875..3ac2122 100644 --- a/src/vm/eventpipebuffer.cpp +++ b/src/vm/eventpipebuffer.cpp @@ -47,7 +47,7 @@ EventPipeBuffer::~EventPipeBuffer() } } -bool EventPipeBuffer::WriteEvent(Thread *pThread, EventPipeEvent &event, EventPipeEventPayload &payload, LPCGUID pActivityId, LPCGUID pRelatedActivityId, StackContents *pStack) +bool EventPipeBuffer::WriteEvent(Thread *pThread, EventPipeSession &session, EventPipeEvent &event, EventPipeEventPayload &payload, LPCGUID pActivityId, LPCGUID pRelatedActivityId, StackContents *pStack) { CONTRACTL { @@ -75,6 +75,7 @@ bool EventPipeBuffer::WriteEvent(Thread *pThread, EventPipeEvent &event, EventPi { // Placement-new the EventPipeEventInstance. EventPipeEventInstance *pInstance = new (m_pCurrent) EventPipeEventInstance( + session, event, pThread->GetOSThreadId(), pDataDest, diff --git a/src/vm/eventpipebuffer.h b/src/vm/eventpipebuffer.h index c96ad26..7d6287d 100644 --- a/src/vm/eventpipebuffer.h +++ b/src/vm/eventpipebuffer.h @@ -10,6 +10,7 @@ #include "eventpipe.h" #include "eventpipeevent.h" #include "eventpipeeventinstance.h" +#include "eventpipesession.h" class EventPipeBuffer { @@ -82,7 +83,7 @@ public: // Returns: // - true: The write succeeded. // - false: The write failed. In this case, the buffer should be considered full. - bool WriteEvent(Thread *pThread, EventPipeEvent &event, EventPipeEventPayload &payload, LPCGUID pActivityId, LPCGUID pRelatedActivityId, StackContents *pStack = NULL); + bool WriteEvent(Thread *pThread, EventPipeSession &session, EventPipeEvent &event, EventPipeEventPayload &payload, LPCGUID pActivityId, LPCGUID pRelatedActivityId, StackContents *pStack = NULL); // Get the timestamp of the most recent event in the buffer. LARGE_INTEGER GetMostRecentTimeStamp() const; diff --git a/src/vm/eventpipebuffermanager.cpp b/src/vm/eventpipebuffermanager.cpp index 045e1d9..e93c439 100644 --- a/src/vm/eventpipebuffermanager.cpp +++ b/src/vm/eventpipebuffermanager.cpp @@ -289,7 +289,7 @@ void EventPipeBufferManager::DeAllocateBuffer(EventPipeBuffer *pBuffer) } } -bool EventPipeBufferManager::WriteEvent(Thread *pThread, EventPipeEvent &event, EventPipeEventPayload &payload, LPCGUID pActivityId, LPCGUID pRelatedActivityId, Thread *pEventThread, StackContents *pStack) +bool EventPipeBufferManager::WriteEvent(Thread *pThread, EventPipeSession &session, EventPipeEvent &event, EventPipeEventPayload &payload, LPCGUID pActivityId, LPCGUID pRelatedActivityId, Thread *pEventThread, StackContents *pStack) { CONTRACTL { @@ -348,7 +348,7 @@ bool EventPipeBufferManager::WriteEvent(Thread *pThread, EventPipeEvent &event, else { // Attempt to write the event to the buffer. If this fails, we should allocate a new buffer. - allocNewBuffer = !pBuffer->WriteEvent(pEventThread, event, payload, pActivityId, pRelatedActivityId, pStack); + allocNewBuffer = !pBuffer->WriteEvent(pEventThread, session, event, payload, pActivityId, pRelatedActivityId, pStack); } } @@ -371,7 +371,7 @@ bool EventPipeBufferManager::WriteEvent(Thread *pThread, EventPipeEvent &event, // This is the second time if this thread did have one or more buffers, but they were full. if(allocNewBuffer && pBuffer != NULL) { - allocNewBuffer = !pBuffer->WriteEvent(pEventThread, event, payload, pActivityId, pRelatedActivityId, pStack); + allocNewBuffer = !pBuffer->WriteEvent(pEventThread, session, event, payload, pActivityId, pRelatedActivityId, pStack); } // Mark that the thread is no longer writing an event. diff --git a/src/vm/eventpipebuffermanager.h b/src/vm/eventpipebuffermanager.h index 942d4e2..87502ed 100644 --- a/src/vm/eventpipebuffermanager.h +++ b/src/vm/eventpipebuffermanager.h @@ -10,6 +10,7 @@ #include "eventpipe.h" #include "eventpipefile.h" #include "eventpipebuffer.h" +#include "eventpipesession.h" #include "spinlock.h" class EventPipeBufferList; @@ -69,7 +70,7 @@ public: // This is because the thread that writes the events is not the same as the "event thread". // An optional stack trace can be provided for sample profiler events. // Otherwise, if a stack trace is needed, one will be automatically collected. - bool WriteEvent(Thread *pThread, EventPipeEvent &event, EventPipeEventPayload &payload, LPCGUID pActivityId, LPCGUID pRelatedActivityId, Thread *pEventThread = NULL, StackContents *pStack = NULL); + bool WriteEvent(Thread *pThread, EventPipeSession &session, EventPipeEvent &event, EventPipeEventPayload &payload, LPCGUID pActivityId, LPCGUID pRelatedActivityId, Thread *pEventThread = NULL, StackContents *pStack = NULL); // Write the contents of the managed buffers to the specified file. // The stopTimeStamp is used to determine when tracing was stopped to ensure that we diff --git a/src/vm/eventpipeconfiguration.cpp b/src/vm/eventpipeconfiguration.cpp index 4678fc3..c8833dc 100644 --- a/src/vm/eventpipeconfiguration.cpp +++ b/src/vm/eventpipeconfiguration.cpp @@ -501,6 +501,7 @@ EventPipeEventInstance* EventPipeConfiguration::BuildEventMetadataEvent(EventPip // Construct the event instance. EventPipeEventInstance *pInstance = new EventPipeEventInstance( + *EventPipe::s_pSession, *m_pMetadataEvent, GetCurrentThreadId(), pInstancePayload, diff --git a/src/vm/eventpipeconfiguration.h b/src/vm/eventpipeconfiguration.h index d1a8a90..7e587cd 100644 --- a/src/vm/eventpipeconfiguration.h +++ b/src/vm/eventpipeconfiguration.h @@ -6,6 +6,7 @@ #ifdef FEATURE_PERFTRACING +#include "eventpipe.h" #include "slist.h" class EventPipeSessionProvider; diff --git a/src/vm/eventpipeeventinstance.cpp b/src/vm/eventpipeeventinstance.cpp index 6cb7438..51bd4ce 100644 --- a/src/vm/eventpipeeventinstance.cpp +++ b/src/vm/eventpipeeventinstance.cpp @@ -3,6 +3,7 @@ // See the LICENSE file in the project root for more information. #include "common.h" +#include "eventpipeconfiguration.h" #include "eventpipeeventinstance.h" #include "eventpipejsonfile.h" #include "fastserializer.h" @@ -11,6 +12,7 @@ #ifdef FEATURE_PERFTRACING EventPipeEventInstance::EventPipeEventInstance( + EventPipeSession &session, EventPipeEvent &event, DWORD threadID, BYTE *pData, @@ -53,7 +55,7 @@ EventPipeEventInstance::EventPipeEventInstance( m_dataLength = length; QueryPerformanceCounter(&m_timeStamp); - if(event.NeedStack()) + if(event.NeedStack() && !session.RundownEnabled()) { EventPipe::WalkManagedStackForCurrentThread(m_stackContents); } @@ -221,8 +223,8 @@ bool EventPipeEventInstance::EnsureConsistency() } #endif // _DEBUG -SampleProfilerEventInstance::SampleProfilerEventInstance(EventPipeEvent &event, Thread *pThread, BYTE *pData, unsigned int length) - :EventPipeEventInstance(event, pThread->GetOSThreadId(), pData, length, NULL /* pActivityId */, NULL /* pRelatedActivityId */) +SampleProfilerEventInstance::SampleProfilerEventInstance(EventPipeSession &session, EventPipeEvent &event, Thread *pThread, BYTE *pData, unsigned int length) + :EventPipeEventInstance(session, event, pThread->GetOSThreadId(), pData, length, NULL /* pActivityId */, NULL /* pRelatedActivityId */) { LIMITED_METHOD_CONTRACT; } diff --git a/src/vm/eventpipeeventinstance.h b/src/vm/eventpipeeventinstance.h index 4fcf95c..5298122 100644 --- a/src/vm/eventpipeeventinstance.h +++ b/src/vm/eventpipeeventinstance.h @@ -9,6 +9,7 @@ #include "eventpipe.h" #include "eventpipeevent.h" +#include "eventpipesession.h" #include "fastserializableobject.h" #include "fastserializer.h" @@ -19,7 +20,7 @@ class EventPipeEventInstance public: - EventPipeEventInstance(EventPipeEvent &event, DWORD threadID, BYTE *pData, unsigned int length, LPCGUID pActivityId, LPCGUID pRelatedActivityId); + EventPipeEventInstance(EventPipeSession &session, EventPipeEvent &event, DWORD threadID, BYTE *pData, unsigned int length, LPCGUID pActivityId, LPCGUID pRelatedActivityId); // Get the event associated with this instance. EventPipeEvent* GetEvent() const; @@ -82,7 +83,7 @@ class SampleProfilerEventInstance : public EventPipeEventInstance public: - SampleProfilerEventInstance(EventPipeEvent &event, Thread *pThread, BYTE *pData, unsigned int length); + SampleProfilerEventInstance(EventPipeSession &session, EventPipeEvent &event, Thread *pThread, BYTE *pData, unsigned int length); }; #endif // FEATURE_PERFTRACING -- 2.7.4