From 18168f63efda22f7a9e6f1d0e49c3a5604fe8f6c Mon Sep 17 00:00:00 2001 From: Andrew Au Date: Wed, 26 Jun 2019 15:21:39 -0700 Subject: EventPipe events should only be sent to sessions that are listening to the event (#25401) --- src/vm/eventpipeconfiguration.cpp | 33 +++++++++++++++++++++++++++++++++ src/vm/eventpipeconfiguration.h | 4 +++- src/vm/eventpipeevent.cpp | 13 ++++++------- src/vm/eventpipeevent.h | 4 ++-- src/vm/eventpipeprovider.cpp | 38 +++++++++++++++----------------------- src/vm/eventpipeprovider.h | 7 ++----- 6 files changed, 61 insertions(+), 38 deletions(-) diff --git a/src/vm/eventpipeconfiguration.cpp b/src/vm/eventpipeconfiguration.cpp index a1c91e8133..412c447b26 100644 --- a/src/vm/eventpipeconfiguration.cpp +++ b/src/vm/eventpipeconfiguration.cpp @@ -309,6 +309,39 @@ void EventPipeConfiguration::ComputeKeywordAndLevel(const EventPipeProvider& pro }); } +INT64 EventPipeConfiguration::ComputeEventEnabledMask(const EventPipeProvider& provider, INT64 keywords, EventPipeEventLevel eventLevel) const +{ + CONTRACTL + { + THROWS; + GC_TRIGGERS; + MODE_PREEMPTIVE; + PRECONDITION(EventPipe::IsLockOwnedByCurrentThread()); + } + CONTRACTL_END; + INT64 result = 0; + EventPipe::ForEachSession([&](EventPipeSession &session) { + EventPipeSessionProvider *pSessionProvider = GetSessionProvider(session, &provider); + if (pSessionProvider != nullptr) + { + INT64 sessionKeyword = pSessionProvider->GetKeywords(); + EventPipeEventLevel sessionLevel = pSessionProvider->GetLevel(); + // The event is enabled if: + // - The provider is enabled. + // - The event keywords are unspecified in the manifest (== 0) or when masked with the enabled config are != 0. + // - The event level is LogAlways or the provider's verbosity level is set to greater than the event's verbosity level in the manifest. + bool providerEnabled = provider.Enabled(); + bool keywordEnabled = (keywords == 0) || ((sessionKeyword & keywords) != 0); + bool levelEnabled = ((eventLevel == EventPipeEventLevel::LogAlways) || (sessionLevel >= eventLevel)); + if (providerEnabled && keywordEnabled && levelEnabled) + { + result = result | session.GetMask(); + } + } + }); + return result; +} + void EventPipeConfiguration::Enable(EventPipeSession &session, EventPipeProviderCallbackDataQueue *pEventPipeProviderCallbackDataQueue) { CONTRACTL diff --git a/src/vm/eventpipeconfiguration.h b/src/vm/eventpipeconfiguration.h index 5f751ef428..bdc9c5260b 100644 --- a/src/vm/eventpipeconfiguration.h +++ b/src/vm/eventpipeconfiguration.h @@ -54,7 +54,9 @@ public: // Delete deferred providers. void DeleteDeferredProviders(); - + + // Compute the enabled bit mask, the ith bit is 1 iff an event with the given (provider, keywords, eventLevel) is enabled for the ith session. + INT64 ComputeEventEnabledMask(const EventPipeProvider& provider, INT64 keywords, EventPipeEventLevel eventLevel) const; private: // Get the provider without taking the lock. EventPipeProvider *GetProviderNoLock(const SString &providerID); diff --git a/src/vm/eventpipeevent.cpp b/src/vm/eventpipeevent.cpp index 40ef588754..e352afb456 100644 --- a/src/vm/eventpipeevent.cpp +++ b/src/vm/eventpipeevent.cpp @@ -22,7 +22,7 @@ EventPipeEvent::EventPipeEvent( m_eventVersion(eventVersion), m_level(level), m_needStack(needStack), - m_enabled(false), + m_enabledMask(0), m_pMetadata(nullptr) { CONTRACTL @@ -138,7 +138,7 @@ bool EventPipeEvent::NeedStack() const bool EventPipeEvent::IsEnabled() const { LIMITED_METHOD_CONTRACT; - return m_enabled; + return m_enabledMask != 0; } BYTE *EventPipeEvent::GetMetadata() const @@ -156,16 +156,15 @@ unsigned int EventPipeEvent::GetMetadataLength() const void EventPipeEvent::RefreshState() { LIMITED_METHOD_CONTRACT; - m_enabled = m_pProvider->EventEnabled(m_keywords, m_level); + _ASSERTE(EventPipe::IsLockOwnedByCurrentThread()); + m_enabledMask = m_pProvider->ComputeEventEnabledMask(m_keywords, m_level); } bool EventPipeEvent::IsEnabled(uint64_t sessionMask) const { LIMITED_METHOD_CONTRACT; - _ASSERT(m_pProvider != nullptr); - const bool IsProviderEnabled = m_pProvider->IsEnabled(sessionMask); - const bool IsEventEnabled = m_pProvider->EventEnabled(m_keywords, m_level); - return (IsProviderEnabled && IsEventEnabled); + _ASSERTE(m_pProvider != nullptr); + return (m_pProvider->IsEnabled(sessionMask) && (m_enabledMask & sessionMask) != 0); } #endif // FEATURE_PERFTRACING diff --git a/src/vm/eventpipeevent.h b/src/vm/eventpipeevent.h index 122c3bef81..f2dfe4ec23 100644 --- a/src/vm/eventpipeevent.h +++ b/src/vm/eventpipeevent.h @@ -34,8 +34,8 @@ private: // True if a call stack should be captured when writing the event. const bool m_needStack; - // True if the event is current enabled. - Volatile m_enabled; + // The ith bit is 1 iff the event is enabled for the ith session + Volatile m_enabledMask; // Metadata BYTE *m_pMetadata; diff --git a/src/vm/eventpipeprovider.cpp b/src/vm/eventpipeprovider.cpp index e1681f7ccf..4148aa131a 100644 --- a/src/vm/eventpipeprovider.cpp +++ b/src/vm/eventpipeprovider.cpp @@ -84,26 +84,18 @@ const SString &EventPipeProvider::GetProviderName() const return m_providerName; } -bool EventPipeProvider::EventEnabled(INT64 keywords) const +INT64 EventPipeProvider::ComputeEventEnabledMask(INT64 keywords, EventPipeEventLevel eventLevel) const { - LIMITED_METHOD_CONTRACT; - - // The event is enabled if: - // - The provider is enabled. - // - The event keywords are unspecified in the manifest (== 0) or when masked with the enabled config are != 0. - return (Enabled() && ((keywords == 0) || ((m_keywords & keywords) != 0))); -} - -bool EventPipeProvider::EventEnabled(INT64 keywords, EventPipeEventLevel eventLevel) const -{ - LIMITED_METHOD_CONTRACT; + CONTRACTL + { + THROWS; + GC_TRIGGERS; + MODE_PREEMPTIVE; + PRECONDITION(EventPipe::IsLockOwnedByCurrentThread()); + } + CONTRACTL_END; - // The event is enabled if: - // - The provider is enabled. - // - The event keywords are unspecified in the manifest (== 0) or when masked with the enabled config are != 0. - // - The event level is LogAlways or the provider's verbosity level is set to greater than the event's verbosity level in the manifest. - return (EventEnabled(keywords) && - ((eventLevel == EventPipeEventLevel::LogAlways) || (m_providerLevel >= eventLevel))); + return m_pConfig->ComputeEventEnabledMask((*this), keywords, eventLevel); } EventPipeProviderCallbackData EventPipeProvider::SetConfiguration( @@ -130,7 +122,7 @@ EventPipeProviderCallbackData EventPipeProvider::SetConfiguration( m_providerLevel = providerLevelForAllSessions; RefreshAllEvents(); - return PrepareCallbackData(keywords, providerLevel, pFilterData); + return PrepareCallbackData(m_keywords, m_providerLevel, pFilterData); } EventPipeProviderCallbackData EventPipeProvider::UnsetConfiguration( @@ -158,7 +150,7 @@ EventPipeProviderCallbackData EventPipeProvider::UnsetConfiguration( m_providerLevel = providerLevelForAllSessions; RefreshAllEvents(); - return PrepareCallbackData(keywords, providerLevel, pFilterData); + return PrepareCallbackData(m_keywords, m_providerLevel, pFilterData); } EventPipeEvent *EventPipeProvider::AddEvent(unsigned int eventID, INT64 keywords, unsigned int eventVersion, EventPipeEventLevel level, bool needStack, BYTE *pMetadata, unsigned int metadataLength) @@ -166,7 +158,7 @@ EventPipeEvent *EventPipeProvider::AddEvent(unsigned int eventID, INT64 keywords CONTRACTL { THROWS; - GC_NOTRIGGER; + GC_TRIGGERS; MODE_ANY; } CONTRACTL_END; @@ -192,7 +184,7 @@ void EventPipeProvider::AddEvent(EventPipeEvent &event) CONTRACTL { THROWS; - GC_NOTRIGGER; + GC_TRIGGERS; MODE_ANY; } CONTRACTL_END; @@ -304,7 +296,7 @@ void EventPipeProvider::RefreshAllEvents() CONTRACTL { THROWS; - GC_NOTRIGGER; + GC_TRIGGERS; MODE_ANY; PRECONDITION(EventPipe::IsLockOwnedByCurrentThread()); } diff --git a/src/vm/eventpipeprovider.h b/src/vm/eventpipeprovider.h index ad7663d91c..d842f18044 100644 --- a/src/vm/eventpipeprovider.h +++ b/src/vm/eventpipeprovider.h @@ -72,11 +72,8 @@ public: return ((m_sessions & sessionMask) != 0); } - // Determine if the specified keywords are enabled. - bool EventEnabled(INT64 keywords) const; - - // Determine if the specified keywords and level match the configuration. - bool EventEnabled(INT64 keywords, EventPipeEventLevel eventLevel) const; + // Compute the enabled bit mask, the ith bit is 1 iff an event with the given (provider, keywords, eventLevel) is enabled for the ith session. + INT64 ComputeEventEnabledMask(INT64 keywords, EventPipeEventLevel eventLevel) const; // Create a new event. EventPipeEvent* AddEvent(unsigned int eventID, INT64 keywords, unsigned int eventVersion, EventPipeEventLevel level, bool needStack, BYTE *pMetadata = NULL, unsigned int metadataLength = 0); -- cgit v1.2.3