From c8f77deb64c8bf582a7a9a090dea6a8ad6c9be6b Mon Sep 17 00:00:00 2001 From: Brian Robbins Date: Fri, 12 May 2017 14:14:28 -0700 Subject: [PATCH] Don't allow specification of needStack to EventPipeProvider when creating an event. (#11571) --- src/scripts/genEventPipe.py | 11 +++-------- src/vm/eventpipeprovider.cpp | 13 +++++++++++++ src/vm/eventpipeprovider.h | 10 +++++++++- 3 files changed, 25 insertions(+), 9 deletions(-) diff --git a/src/scripts/genEventPipe.py b/src/scripts/genEventPipe.py index 3a818ce..02634ad 100644 --- a/src/scripts/genEventPipe.py +++ b/src/scripts/genEventPipe.py @@ -136,14 +136,9 @@ def generateClrEventPipeWriteEventsImpl( eventLevel = eventLevel.replace("win:", "EventPipeEventLevel::") exclusionInfo = parseExclusionList(exclusionListFile) taskName = eventNode.getAttribute('task') - noStack = getStackWalkBit( - providerName, - taskName, - eventName, - exclusionInfo.nostack) - - initEvent = """ EventPipeEvent%s = EventPipeProvider%s->AddEvent(%s,%s,%s,%s,%d); -""" % (eventName, providerPrettyName, eventKeywordsMask, eventValue, eventVersion, eventLevel, int(noStack)) + + initEvent = """ EventPipeEvent%s = EventPipeProvider%s->AddEvent(%s,%s,%s,%s); +""" % (eventName, providerPrettyName, eventKeywordsMask, eventValue, eventVersion, eventLevel) WriteEventImpl.append(initEvent) WriteEventImpl.append("}") diff --git a/src/vm/eventpipeprovider.cpp b/src/vm/eventpipeprovider.cpp index be87fb4..beee6fc 100644 --- a/src/vm/eventpipeprovider.cpp +++ b/src/vm/eventpipeprovider.cpp @@ -128,6 +128,19 @@ void EventPipeProvider::SetConfiguration(bool providerEnabled, INT64 keywords, E InvokeCallback(); } +EventPipeEvent* EventPipeProvider::AddEvent(INT64 keywords, unsigned int eventID, unsigned int eventVersion, EventPipeEventLevel level) +{ + CONTRACTL + { + THROWS; + GC_NOTRIGGER; + MODE_ANY; + } + CONTRACTL_END; + + return AddEvent(keywords, eventID, eventVersion, level, true /* needStack */); +} + EventPipeEvent* EventPipeProvider::AddEvent(INT64 keywords, unsigned int eventID, unsigned int eventVersion, EventPipeEventLevel level, bool needStack) { CONTRACTL diff --git a/src/vm/eventpipeprovider.h b/src/vm/eventpipeprovider.h index 7aaa8e4..464d011 100644 --- a/src/vm/eventpipeprovider.h +++ b/src/vm/eventpipeprovider.h @@ -26,6 +26,7 @@ class EventPipeProvider { // Declare friends. friend class EventPipeConfiguration; + friend class SampleProfiler; private: // The GUID of the provider. @@ -71,10 +72,17 @@ public: bool EventEnabled(INT64 keywords, EventPipeEventLevel eventLevel) const; // Create a new event. - EventPipeEvent* AddEvent(INT64 keywords, unsigned int eventID, unsigned int eventVersion, EventPipeEventLevel level, bool needStack); + EventPipeEvent* AddEvent(INT64 keywords, unsigned int eventID, unsigned int eventVersion, EventPipeEventLevel level); private: + // Create a new event, but allow needStack to be specified. + // In general, we want stack walking to be controlled by the consumer and not the producer of events. + // However, there are a couple of cases that we know we don't want to do a stackwalk that would affect performance significantly: + // 1. Sample profiler events: The sample profiler already does a stack walk of the target thread. Doing one of the sampler thread is a waste. + // 2. Metadata events: These aren't as painful but because we have to keep this functionality around, might as well use it. + EventPipeEvent* AddEvent(INT64 keywords, unsigned int eventID, unsigned int eventVersion, EventPipeEventLevel level, bool needStack); + // Add an event to the provider. void AddEvent(EventPipeEvent &event); -- 2.7.4