Skip to content

Commit 0784e0a

Browse files
tianleiwuyuslepukhin
tianleiwu
and
yuslepukhin
authored
Revert "Improve Windows ETW callback registration and fix issues" (#25055)
### Description Revert [Improve Windows ETW callback registration and fix issues](#24877) to unblock python packaging pipeline. ### Motivation and Context Python packaging pipeline is failing due to the changes in the PR. --------- Co-authored-by: Dmitri Smirnov <dmitrism@microsoft.com>
1 parent 82fddd7 commit 0784e0a

File tree

15 files changed

+295
-375
lines changed

15 files changed

+295
-375
lines changed

‎onnxruntime/core/framework/execution_provider.cc‎

Lines changed: 0 additions & 104 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
// Copyright (c) Microsoft Corporation. All rights reserved.
22
// Licensed under the MIT License.
33
#include "core/framework/execution_provider.h"
4-
#include "core/framework/execution_providers.h"
54

65
#include "core/graph/graph_viewer.h"
76
#include "core/framework/compute_capability.h"
@@ -10,8 +9,6 @@
109
#include "core/framework/murmurhash3.h"
1110
#include "core/framework/op_kernel.h"
1211

13-
#include <stdint.h>
14-
1512
namespace onnxruntime {
1613

1714
std::vector<std::unique_ptr<ComputeCapability>>
@@ -40,105 +37,4 @@ common::Status IExecutionProvider::Compile(const std::vector<FusedNodeAndGraph>&
4037
}
4138

4239
#endif
43-
44-
ExecutionProviders::ExecutionProviders() {
45-
#ifdef _WIN32
46-
// Register callback for ETW capture state (rundown)
47-
etw_callback_key_ = "ExecutionProviders_rundown_";
48-
etw_callback_key_.append(std::to_string(reinterpret_cast<uintptr_t>(this)));
49-
WindowsTelemetry::RegisterInternalCallback(
50-
etw_callback_key_,
51-
[this](LPCGUID SourceId,
52-
ULONG IsEnabled,
53-
UCHAR Level,
54-
ULONGLONG MatchAnyKeyword,
55-
ULONGLONG MatchAllKeyword,
56-
PEVENT_FILTER_DESCRIPTOR FilterData,
57-
PVOID CallbackContext) { this->EtwProvidersCallback(SourceId, IsEnabled, Level,
58-
MatchAnyKeyword, MatchAllKeyword,
59-
FilterData, CallbackContext); });
60-
#endif
61-
}
62-
63-
ExecutionProviders::~ExecutionProviders() {
64-
#ifdef _WIN32
65-
WindowsTelemetry::UnregisterInternalCallback(etw_callback_key_);
66-
#endif
67-
}
68-
69-
common::Status ExecutionProviders::Add(const std::string& provider_id,
70-
const std::shared_ptr<IExecutionProvider>& p_exec_provider) {
71-
// make sure there are no issues before we change any internal data structures
72-
if (provider_idx_map_.find(provider_id) != provider_idx_map_.end()) {
73-
auto status = ORT_MAKE_STATUS(ONNXRUNTIME, FAIL, "Provider ", provider_id, " has already been registered.");
74-
LOGS_DEFAULT(ERROR) << status.ErrorMessage();
75-
return status;
76-
}
77-
78-
// index that provider will have after insertion
79-
auto new_provider_idx = exec_providers_.size();
80-
81-
ORT_IGNORE_RETURN_VALUE(provider_idx_map_.insert({provider_id, new_provider_idx}));
82-
83-
// update execution provider options
84-
auto providerOptions = p_exec_provider->GetProviderOptions();
85-
exec_provider_options_[provider_id] = providerOptions;
86-
87-
#ifdef _WIN32
88-
LogProviderOptions(provider_id, providerOptions, false);
89-
#endif
90-
91-
exec_provider_ids_.push_back(provider_id);
92-
exec_providers_.push_back(p_exec_provider);
93-
return Status::OK();
94-
}
95-
96-
#ifdef _WIN32
97-
void ExecutionProviders::EtwProvidersCallback(LPCGUID /* SourceId */,
98-
ULONG IsEnabled,
99-
UCHAR /* Level */,
100-
ULONGLONG MatchAnyKeyword,
101-
ULONGLONG /* MatchAllKeyword */,
102-
PEVENT_FILTER_DESCRIPTOR /* FilterData */,
103-
PVOID /* CallbackContext */) {
104-
// Check if this callback is for capturing state
105-
if ((IsEnabled == EVENT_CONTROL_CODE_CAPTURE_STATE) &&
106-
((MatchAnyKeyword & static_cast<ULONGLONG>(onnxruntime::logging::ORTTraceLoggingKeyword::Session)) != 0)) {
107-
for (size_t i = 0; i < exec_providers_.size(); ++i) {
108-
const auto& provider_id = exec_provider_ids_[i];
109-
110-
auto it = exec_provider_options_.find(provider_id);
111-
if (it != exec_provider_options_.end()) {
112-
const auto& options = it->second;
113-
114-
LogProviderOptions(provider_id, options, true);
115-
}
116-
}
117-
}
118-
}
119-
120-
void ExecutionProviders::LogProviderOptions(const std::string& provider_id,
121-
const ProviderOptions& providerOptions,
122-
bool captureState) {
123-
#ifdef ONNXRUNTIME_ENABLE_INSTRUMENT
124-
for (const auto& config_pair : providerOptions) {
125-
TraceLoggingWrite(
126-
telemetry_provider_handle,
127-
"ProviderOptions",
128-
TraceLoggingKeyword(static_cast<uint64_t>(onnxruntime::logging::ORTTraceLoggingKeyword::Session)),
129-
TraceLoggingLevel(WINEVENT_LEVEL_INFO),
130-
TraceLoggingString(provider_id.c_str(), "ProviderId"),
131-
TraceLoggingString(config_pair.first.c_str(), "Key"),
132-
TraceLoggingString(config_pair.second.c_str(), "Value"),
133-
TraceLoggingBool(captureState, "isCaptureState"));
134-
}
135-
#else
136-
ORT_UNUSED_PARAMETER(provider_id);
137-
ORT_UNUSED_PARAMETER(providerOptions);
138-
ORT_UNUSED_PARAMETER(captureState);
139-
#endif
140-
}
141-
142-
#endif
143-
14440
} // namespace onnxruntime

‎onnxruntime/core/framework/execution_providers.h‎

Lines changed: 80 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -26,24 +26,91 @@ Class for managing lookup of the execution providers in a session.
2626
*/
2727
class ExecutionProviders {
2828
public:
29-
ExecutionProviders();
29+
ExecutionProviders() {
30+
#ifdef _WIN32
31+
// Register callback for ETW capture state (rundown)
32+
etw_callback_ = onnxruntime::WindowsTelemetry::EtwInternalCallback(
33+
[this](
34+
LPCGUID SourceId,
35+
ULONG IsEnabled,
36+
UCHAR Level,
37+
ULONGLONG MatchAnyKeyword,
38+
ULONGLONG MatchAllKeyword,
39+
PEVENT_FILTER_DESCRIPTOR FilterData,
40+
PVOID CallbackContext) {
41+
(void)SourceId;
42+
(void)Level;
43+
(void)MatchAnyKeyword;
44+
(void)MatchAllKeyword;
45+
(void)FilterData;
46+
(void)CallbackContext;
47+
48+
// Check if this callback is for capturing state
49+
if ((IsEnabled == EVENT_CONTROL_CODE_CAPTURE_STATE) &&
50+
((MatchAnyKeyword & static_cast<ULONGLONG>(onnxruntime::logging::ORTTraceLoggingKeyword::Session)) != 0)) {
51+
for (size_t i = 0; i < exec_providers_.size(); ++i) {
52+
const auto& provider_id = exec_provider_ids_[i];
53+
54+
auto it = exec_provider_options_.find(provider_id);
55+
if (it != exec_provider_options_.end()) {
56+
const auto& options = it->second;
57+
58+
LogProviderOptions(provider_id, options, true);
59+
}
60+
}
61+
}
62+
});
63+
WindowsTelemetry::RegisterInternalCallback(etw_callback_);
64+
#endif
65+
}
66+
67+
~ExecutionProviders() {
68+
#ifdef _WIN32
69+
WindowsTelemetry ::UnregisterInternalCallback(etw_callback_);
70+
#endif
71+
}
3072

31-
~ExecutionProviders();
73+
common::Status
74+
Add(const std::string& provider_id, const std::shared_ptr<IExecutionProvider>& p_exec_provider) {
75+
// make sure there are no issues before we change any internal data structures
76+
if (provider_idx_map_.find(provider_id) != provider_idx_map_.end()) {
77+
auto status = ORT_MAKE_STATUS(ONNXRUNTIME, FAIL, "Provider ", provider_id, " has already been registered.");
78+
LOGS_DEFAULT(ERROR) << status.ErrorMessage();
79+
return status;
80+
}
3281

33-
common::Status Add(const std::string& provider_id, const std::shared_ptr<IExecutionProvider>& p_exec_provider);
82+
// index that provider will have after insertion
83+
auto new_provider_idx = exec_providers_.size();
84+
85+
ORT_IGNORE_RETURN_VALUE(provider_idx_map_.insert({provider_id, new_provider_idx}));
86+
87+
// update execution provider options
88+
auto providerOptions = p_exec_provider->GetProviderOptions();
89+
exec_provider_options_[provider_id] = providerOptions;
3490

3591
#ifdef _WIN32
92+
LogProviderOptions(provider_id, providerOptions, false);
93+
#endif
3694

37-
void EtwProvidersCallback(LPCGUID SourceId,
38-
ULONG IsEnabled,
39-
UCHAR Level,
40-
ULONGLONG MatchAnyKeyword,
41-
ULONGLONG MatchAllKeyword,
42-
PEVENT_FILTER_DESCRIPTOR FilterData,
43-
PVOID CallbackContext);
95+
exec_provider_ids_.push_back(provider_id);
96+
exec_providers_.push_back(p_exec_provider);
97+
return Status::OK();
98+
}
4499

45-
void LogProviderOptions(const std::string& provider_id, const ProviderOptions& providerOptions,
46-
bool captureState);
100+
#ifdef _WIN32
101+
void LogProviderOptions(const std::string& provider_id, const ProviderOptions& providerOptions, bool captureState) {
102+
for (const auto& config_pair : providerOptions) {
103+
TraceLoggingWrite(
104+
telemetry_provider_handle,
105+
"ProviderOptions",
106+
TraceLoggingKeyword(static_cast<uint64_t>(onnxruntime::logging::ORTTraceLoggingKeyword::Session)),
107+
TraceLoggingLevel(WINEVENT_LEVEL_INFO),
108+
TraceLoggingString(provider_id.c_str(), "ProviderId"),
109+
TraceLoggingString(config_pair.first.c_str(), "Key"),
110+
TraceLoggingString(config_pair.second.c_str(), "Value"),
111+
TraceLoggingBool(captureState, "isCaptureState"));
112+
}
113+
}
47114
#endif
48115

49116
const IExecutionProvider* Get(const onnxruntime::Node& node) const {
@@ -102,7 +169,7 @@ class ExecutionProviders {
102169
bool cpu_execution_provider_was_implicitly_added_ = false;
103170

104171
#ifdef _WIN32
105-
std::string etw_callback_key_;
172+
WindowsTelemetry::EtwInternalCallback etw_callback_;
106173
#endif
107174
};
108175
} // namespace onnxruntime

‎onnxruntime/core/platform/windows/logging/etw_sink.cc‎

Lines changed: 27 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -106,15 +106,18 @@ HRESULT EtwRegistrationManager::Status() const {
106106
return etw_status_;
107107
}
108108

109-
void EtwRegistrationManager::RegisterInternalCallback(const std::string& cb_key, EtwInternalCallback callback) {
109+
void EtwRegistrationManager::RegisterInternalCallback(const EtwInternalCallback& callback) {
110110
std::lock_guard<std::mutex> lock(callbacks_mutex_);
111-
[[maybe_unused]] auto result = callbacks_.emplace(cb_key, std::move(callback));
112-
assert(result.second);
111+
callbacks_.push_back(&callback);
113112
}
114113

115-
void EtwRegistrationManager::UnregisterInternalCallback(const std::string& cb_key) {
114+
void EtwRegistrationManager::UnregisterInternalCallback(const EtwInternalCallback& callback) {
116115
std::lock_guard<std::mutex> lock(callbacks_mutex_);
117-
callbacks_.erase(cb_key);
116+
auto new_end = std::remove_if(callbacks_.begin(), callbacks_.end(),
117+
[&callback](const EtwInternalCallback* ptr) {
118+
return ptr == &callback;
119+
});
120+
callbacks_.erase(new_end, callbacks_.end());
118121
}
119122

120123
void NTAPI EtwRegistrationManager::ORT_TL_EtwEnableCallback(
@@ -135,12 +138,21 @@ void NTAPI EtwRegistrationManager::ORT_TL_EtwEnableCallback(
135138
manager.InvokeCallbacks(SourceId, IsEnabled, Level, MatchAnyKeyword, MatchAllKeyword, FilterData, CallbackContext);
136139
}
137140

138-
EtwRegistrationManager::EtwRegistrationManager()
139-
: initialization_status_(InitializationStatus::NotInitialized),
140-
is_enabled_(false),
141-
level_(),
142-
keyword_(0),
143-
etw_status_(S_OK) {
141+
EtwRegistrationManager::~EtwRegistrationManager() {
142+
std::lock_guard<std::mutex> lock(callbacks_mutex_);
143+
callbacks_.clear();
144+
if (initialization_status_ == InitializationStatus::Initialized ||
145+
initialization_status_ == InitializationStatus::Initializing) {
146+
std::lock_guard<std::mutex> init_lock(init_mutex_);
147+
assert(initialization_status_ != InitializationStatus::Initializing);
148+
if (initialization_status_ == InitializationStatus::Initialized) {
149+
::TraceLoggingUnregister(etw_provider_handle);
150+
initialization_status_ = InitializationStatus::NotInitialized;
151+
}
152+
}
153+
}
154+
155+
EtwRegistrationManager::EtwRegistrationManager() {
144156
}
145157

146158
void EtwRegistrationManager::LazyInitialize() {
@@ -161,13 +173,6 @@ void EtwRegistrationManager::LazyInitialize() {
161173
}
162174
}
163175

164-
EtwRegistrationManager::~EtwRegistrationManager() {
165-
if (initialization_status_ == InitializationStatus::Initialized) {
166-
::TraceLoggingUnregister(etw_provider_handle);
167-
initialization_status_ = InitializationStatus::NotInitialized;
168-
}
169-
}
170-
171176
void EtwRegistrationManager::InvokeCallbacks(LPCGUID SourceId, ULONG IsEnabled, UCHAR Level, ULONGLONG MatchAnyKeyword,
172177
ULONGLONG MatchAllKeyword, PEVENT_FILTER_DESCRIPTOR FilterData,
173178
PVOID CallbackContext) {
@@ -177,9 +182,10 @@ void EtwRegistrationManager::InvokeCallbacks(LPCGUID SourceId, ULONG IsEnabled,
177182
}
178183

179184
std::lock_guard<std::mutex> lock(callbacks_mutex_);
180-
for (const auto& entry : callbacks_) {
181-
const auto& cb = entry.second;
182-
cb(SourceId, IsEnabled, Level, MatchAnyKeyword, MatchAllKeyword, FilterData, CallbackContext);
185+
for (const auto& callback : callbacks_) {
186+
if (callback != nullptr) {
187+
(*callback)(SourceId, IsEnabled, Level, MatchAnyKeyword, MatchAllKeyword, FilterData, CallbackContext);
188+
}
183189
}
184190
}
185191

‎onnxruntime/core/platform/windows/logging/etw_sink.h‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
#include <atomic>
2121
#include <iostream>
2222
#include <string>
23-
#include <unordered_map>
2423
#include <vector>
2524

2625
#include "core/common/logging/capture.h"
@@ -78,9 +77,9 @@ class EtwRegistrationManager {
7877
// Get the ETW registration status
7978
HRESULT Status() const;
8079

81-
void RegisterInternalCallback(const std::string& cb_key, EtwInternalCallback callback);
80+
void RegisterInternalCallback(const EtwInternalCallback& callback);
8281

83-
void UnregisterInternalCallback(const std::string& cb_key);
82+
void UnregisterInternalCallback(const EtwInternalCallback& callback);
8483

8584
private:
8685
EtwRegistrationManager();
@@ -101,11 +100,11 @@ class EtwRegistrationManager {
101100
_In_opt_ PEVENT_FILTER_DESCRIPTOR FilterData,
102101
_In_opt_ PVOID CallbackContext);
103102

104-
std::mutex init_mutex_;
105-
std::atomic<InitializationStatus> initialization_status_ = InitializationStatus::NotInitialized;
106-
std::unordered_map<std::string, EtwInternalCallback> callbacks_;
103+
std::vector<const EtwInternalCallback*> callbacks_;
107104
std::mutex callbacks_mutex_;
108105
mutable std::mutex provider_change_mutex_;
106+
std::mutex init_mutex_;
107+
InitializationStatus initialization_status_ = InitializationStatus::NotInitialized;
109108
bool is_enabled_;
110109
UCHAR level_;
111110
ULONGLONG keyword_;
@@ -134,8 +133,8 @@ class EtwRegistrationManager {
134133
Severity MapLevelToSeverity() { return Severity::kFATAL; }
135134
uint64_t Keyword() const { return 0; }
136135
HRESULT Status() const { return 0; }
137-
void RegisterInternalCallback(const std::string& cb_key, EtwInternalCallback callback) {}
138-
void UnregisterInternalCallback(const std::string& cb_key) {}
136+
void RegisterInternalCallback(const EtwInternalCallback& callback) {}
137+
void UnregisterInternalCallback(const EtwInternalCallback& callback) {}
139138

140139
private:
141140
EtwRegistrationManager() = default;

0 commit comments

Comments
 (0)