Skip to content

Commit 3c3f17f

Browse files
Gasoonjiafacebook-github-bot
Gasoonjia
authored and
facebook-github-bot
committed
introduce DelegateDebugIntId
Summary: EventTracer misused `DebugHandle` type (uint32_t) for debug id of both op-level and delegation level. In fact for delegate_debug_id we should use int32_t instead. This diff creates a new type called `DelegateDebugIntId` specific for delegate_debug_id, and a new flag kUnsetDelegateDebugIntId for unset delegate debug id. Updates corresponding tests as well. Differential Revision: D72297495
1 parent 4987f0b commit 3c3f17f

File tree

5 files changed

+69
-65
lines changed

5 files changed

+69
-65
lines changed

‎devtools/etdump/etdump_flatcc.cpp

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,10 @@ using ::executorch::runtime::ArrayRef;
2727
using ::executorch::runtime::ChainID;
2828
using ::executorch::runtime::DebugHandle;
2929
using ::executorch::runtime::DelegateDebugIdType;
30+
using ::executorch::runtime::DelegateDebugIntId;
3031
using ::executorch::runtime::EValue;
3132
using ::executorch::runtime::EventTracerEntry;
33+
using ::executorch::runtime::kUnsetDelegateDebugIntId;
3234
using ::executorch::runtime::LoggedEValueType;
3335
using ::executorch::runtime::Result;
3436
using ::executorch::runtime::Span;
@@ -223,9 +225,9 @@ EventTracerEntry ETDumpGen::start_profiling(
223225
// EventTracerEntry struct is updated.
224226
EventTracerEntry ETDumpGen::start_profiling_delegate(
225227
const char* name,
226-
DebugHandle delegate_debug_index) {
228+
DelegateDebugIntId delegate_debug_index) {
227229
ET_CHECK_MSG(
228-
(name == nullptr) ^ (delegate_debug_index == -1),
230+
(name == nullptr) ^ (delegate_debug_index == kUnsetDelegateDebugIntId),
229231
"Only name or delegate_debug_index can be valid. Check DelegateMappingBuilder documentation for more details.");
230232
check_ready_to_add_events();
231233
EventTracerEntry prof_entry;
@@ -234,7 +236,7 @@ EventTracerEntry ETDumpGen::start_profiling_delegate(
234236
prof_entry.delegate_event_id_type = delegate_event_id_type;
235237
prof_entry.chain_id = chain_id_;
236238
prof_entry.debug_handle = debug_handle_;
237-
prof_entry.event_id = delegate_debug_index == static_cast<unsigned int>(-1)
239+
prof_entry.event_id = delegate_debug_index == kUnsetDelegateDebugIntId
238240
? create_string_entry(name)
239241
: delegate_debug_index;
240242
prof_entry.start_time = et_pal_current_ticks();
@@ -276,13 +278,13 @@ void ETDumpGen::end_profiling_delegate(
276278

277279
void ETDumpGen::log_profiling_delegate(
278280
const char* name,
279-
DebugHandle delegate_debug_index,
281+
DelegateDebugIntId delegate_debug_index,
280282
et_timestamp_t start_time,
281283
et_timestamp_t end_time,
282284
const void* metadata,
283285
size_t metadata_len) {
284286
ET_CHECK_MSG(
285-
(name == nullptr) ^ (delegate_debug_index == -1),
287+
(name == nullptr) ^ (delegate_debug_index == kUnsetDelegateDebugIntId),
286288
"Only name or delegate_debug_index can be valid. Check DelegateMappingBuilder documentation for more details.");
287289
check_ready_to_add_events();
288290
int64_t string_id = name != nullptr ? create_string_entry(name) : -1;
@@ -308,7 +310,7 @@ void ETDumpGen::log_profiling_delegate(
308310

309311
Result<bool> ETDumpGen::log_intermediate_output_delegate(
310312
const char* name,
311-
DebugHandle delegate_debug_index,
313+
DelegateDebugIntId delegate_debug_index,
312314
const Tensor& output) {
313315
Result<bool> result = log_intermediate_output_delegate_helper(
314316
name, delegate_debug_index, output);
@@ -317,7 +319,7 @@ Result<bool> ETDumpGen::log_intermediate_output_delegate(
317319

318320
Result<bool> ETDumpGen::log_intermediate_output_delegate(
319321
const char* name,
320-
DebugHandle delegate_debug_index,
322+
DelegateDebugIntId delegate_debug_index,
321323
const ArrayRef<Tensor> output) {
322324
log_intermediate_output_delegate_helper(name, delegate_debug_index, output);
323325
Result<bool> result = log_intermediate_output_delegate_helper(
@@ -327,7 +329,7 @@ Result<bool> ETDumpGen::log_intermediate_output_delegate(
327329

328330
Result<bool> ETDumpGen::log_intermediate_output_delegate(
329331
const char* name,
330-
DebugHandle delegate_debug_index,
332+
DelegateDebugIntId delegate_debug_index,
331333
const int& output) {
332334
log_intermediate_output_delegate_helper(name, delegate_debug_index, output);
333335
Result<bool> result = log_intermediate_output_delegate_helper(
@@ -337,7 +339,7 @@ Result<bool> ETDumpGen::log_intermediate_output_delegate(
337339

338340
Result<bool> ETDumpGen::log_intermediate_output_delegate(
339341
const char* name,
340-
DebugHandle delegate_debug_index,
342+
DelegateDebugIntId delegate_debug_index,
341343
const bool& output) {
342344
log_intermediate_output_delegate_helper(name, delegate_debug_index, output);
343345
Result<bool> result = log_intermediate_output_delegate_helper(
@@ -347,7 +349,7 @@ Result<bool> ETDumpGen::log_intermediate_output_delegate(
347349

348350
Result<bool> ETDumpGen::log_intermediate_output_delegate(
349351
const char* name,
350-
DebugHandle delegate_debug_index,
352+
DelegateDebugIntId delegate_debug_index,
351353
const double& output) {
352354
log_intermediate_output_delegate_helper(name, delegate_debug_index, output);
353355
Result<bool> result = log_intermediate_output_delegate_helper(
@@ -358,10 +360,10 @@ Result<bool> ETDumpGen::log_intermediate_output_delegate(
358360
template <typename T>
359361
Result<bool> ETDumpGen::log_intermediate_output_delegate_helper(
360362
const char* name,
361-
DebugHandle delegate_debug_index,
363+
DelegateDebugIntId delegate_debug_index,
362364
const T& output) {
363365
ET_CHECK_OR_RETURN_ERROR(
364-
(name == nullptr) ^ (delegate_debug_index == -1),
366+
(name == nullptr) ^ (delegate_debug_index == kUnsetDelegateDebugIntId),
365367
InvalidArgument,
366368
"Only name or delegate_debug_index can be valid. Check DelegateMappingBuilder documentation for more details.");
367369

‎devtools/etdump/etdump_flatcc.h

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ struct flatcc_builder;
2525
namespace executorch {
2626
namespace etdump {
2727

28+
using ::executorch::runtime::DelegateDebugIntId;
2829
using ::executorch::runtime::Result;
2930

3031
namespace internal {
@@ -84,14 +85,14 @@ class ETDumpGen : public ::executorch::runtime::EventTracer {
8485
::executorch::runtime::EventTracerEntry prof_entry) override;
8586
virtual ::executorch::runtime::EventTracerEntry start_profiling_delegate(
8687
const char* name,
87-
::executorch::runtime::DebugHandle delegate_debug_index) override;
88+
DelegateDebugIntId delegate_debug_index) override;
8889
virtual void end_profiling_delegate(
8990
::executorch::runtime::EventTracerEntry prof_entry,
9091
const void* metadata,
9192
size_t metadata_len) override;
9293
virtual void log_profiling_delegate(
9394
const char* name,
94-
::executorch::runtime::DebugHandle delegate_debug_index,
95+
DelegateDebugIntId delegate_debug_index,
9596
et_timestamp_t start_time,
9697
et_timestamp_t end_time,
9798
const void* metadata,
@@ -111,15 +112,15 @@ class ETDumpGen : public ::executorch::runtime::EventTracer {
111112
*/
112113
virtual Result<bool> log_intermediate_output_delegate(
113114
const char* name,
114-
::executorch::runtime::DebugHandle delegate_debug_index,
115+
DelegateDebugIntId delegate_debug_index,
115116
const executorch::aten::Tensor& output) override;
116117

117118
/**
118119
* Log an intermediate tensor array output from a delegate.
119120
*/
120121
virtual Result<bool> log_intermediate_output_delegate(
121122
const char* name,
122-
::executorch::runtime::DebugHandle delegate_debug_index,
123+
DelegateDebugIntId delegate_debug_index,
123124
const ::executorch::runtime::ArrayRef<executorch::aten::Tensor> output)
124125
override;
125126

@@ -128,23 +129,23 @@ class ETDumpGen : public ::executorch::runtime::EventTracer {
128129
*/
129130
virtual Result<bool> log_intermediate_output_delegate(
130131
const char* name,
131-
::executorch::runtime::DebugHandle delegate_debug_index,
132+
DelegateDebugIntId delegate_debug_index,
132133
const int& output) override;
133134

134135
/**
135136
* Log an intermediate bool output from a delegate.
136137
*/
137138
virtual Result<bool> log_intermediate_output_delegate(
138139
const char* name,
139-
::executorch::runtime::DebugHandle delegate_debug_index,
140+
DelegateDebugIntId delegate_debug_index,
140141
const bool& output) override;
141142

142143
/**
143144
* Log an intermediate double output from a delegate.
144145
*/
145146
virtual Result<bool> log_intermediate_output_delegate(
146147
const char* name,
147-
::executorch::runtime::DebugHandle delegate_debug_index,
148+
DelegateDebugIntId delegate_debug_index,
148149
const double& output) override;
149150
void set_debug_buffer(::executorch::runtime::Span<uint8_t> buffer);
150151
void set_data_sink(DataSinkBase* data_sink);
@@ -173,7 +174,7 @@ class ETDumpGen : public ::executorch::runtime::EventTracer {
173174
template <typename T>
174175
Result<bool> log_intermediate_output_delegate_helper(
175176
const char* name,
176-
::executorch::runtime::DebugHandle delegate_debug_index,
177+
DelegateDebugIntId delegate_debug_index,
177178
const T& output);
178179

179180
long write_tensor_or_raise_error(executorch::aten::Tensor tensor);

‎devtools/etdump/tests/etdump_test.cpp

Lines changed: 14 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ using ::executorch::runtime::DelegateDebugIdType;
3636
using ::executorch::runtime::Error;
3737
using ::executorch::runtime::EValue;
3838
using ::executorch::runtime::EventTracerEntry;
39+
using ::executorch::runtime::kUnsetDelegateDebugIntId;
3940
using ::executorch::runtime::LoggedEValueType;
4041
using ::executorch::runtime::Span;
4142
using ::executorch::runtime::Tag;
@@ -70,9 +71,7 @@ class ProfilerETDumpTest : public ::testing::Test {
7071
TensorFactory<ScalarType::Float>& tf) {
7172
ET_EXPECT_DEATH(
7273
gen->log_intermediate_output_delegate(
73-
"test_event_tensor",
74-
static_cast<torch::executor::DebugHandle>(-1),
75-
tf.ones({3, 2})),
74+
"test_event_tensor", kUnsetDelegateDebugIntId, tf.ones({3, 2})),
7675
"Must set data sink before writing tensor-like data");
7776
}
7877

@@ -582,7 +581,7 @@ TEST_F(ProfilerETDumpTest, LogDelegateIntermediateOutput) {
582581
Result<bool> log_tensor_list_result =
583582
etdump_gen[i]->log_intermediate_output_delegate(
584583
nullptr,
585-
static_cast<torch::executor::DebugHandle>(-1),
584+
kUnsetDelegateDebugIntId,
586585
ArrayRef<Tensor>(tensors.data(), tensors.size()));
587586

588587
Result<bool> log_int_result =
@@ -599,7 +598,7 @@ TEST_F(ProfilerETDumpTest, LogDelegateIntermediateOutput) {
599598

600599
Result<bool> log_bool_result =
601600
etdump_gen[i]->log_intermediate_output_delegate(
602-
nullptr, static_cast<torch::executor::DebugHandle>(-1), 29.82);
601+
nullptr, kUnsetDelegateDebugIntId, 29.82);
603602

604603
ASSERT_EQ(log_tensor_result.error(), Error::InvalidArgument);
605604
ASSERT_EQ(log_tensor_list_result.error(), Error::InvalidArgument);
@@ -611,33 +610,25 @@ TEST_F(ProfilerETDumpTest, LogDelegateIntermediateOutput) {
611610

612611
// Log a tensor
613612
etdump_gen[i]->log_intermediate_output_delegate(
614-
"test_event_tensor",
615-
static_cast<torch::executor::DebugHandle>(-1),
616-
tf.ones({3, 2}));
613+
"test_event_tensor", kUnsetDelegateDebugIntId, tf.ones({3, 2}));
617614

618615
// Log a tensor list
619616
etdump_gen[i]->log_intermediate_output_delegate(
620617
"test_event_tensorlist",
621-
static_cast<torch::executor::DebugHandle>(-1),
618+
kUnsetDelegateDebugIntId,
622619
ArrayRef<Tensor>(tensors.data(), tensors.size()));
623620

624621
// Log an int
625622
etdump_gen[i]->log_intermediate_output_delegate(
626-
"test_event_tensorlist",
627-
static_cast<torch::executor::DebugHandle>(-1),
628-
10);
623+
"test_event_tensorlist", kUnsetDelegateDebugIntId, 10);
629624

630625
// Log a double
631626
etdump_gen[i]->log_intermediate_output_delegate(
632-
"test_event_tensorlist",
633-
static_cast<torch::executor::DebugHandle>(-1),
634-
20.75);
627+
"test_event_tensorlist", kUnsetDelegateDebugIntId, 20.75);
635628

636629
// Log a bool
637630
etdump_gen[i]->log_intermediate_output_delegate(
638-
"test_event_tensorlist",
639-
static_cast<torch::executor::DebugHandle>(-1),
640-
true);
631+
"test_event_tensorlist", kUnsetDelegateDebugIntId, true);
641632

642633
ETDumpResult result = etdump_gen[i]->get_etdump_data();
643634
ASSERT_TRUE(result.buf != nullptr);
@@ -762,23 +753,18 @@ TEST_F(ProfilerETDumpTest, LogDelegateEvents) {
762753
etdump_gen[i]->log_profiling_delegate(
763754
nullptr, 278, 1, 2, metadata, strlen(metadata) + 1);
764755
EventTracerEntry entry = etdump_gen[i]->start_profiling_delegate(
765-
"test_event", static_cast<torch::executor::DebugHandle>(-1));
756+
"test_event", kUnsetDelegateDebugIntId);
766757
EXPECT_NE(entry.delegate_event_id_type, DelegateDebugIdType::kNone);
767758
// Event 2
768759
etdump_gen[i]->end_profiling_delegate(
769760
entry, metadata, strlen(metadata) + 1);
770761
// Event 3
771762
etdump_gen[i]->log_profiling_delegate(
772-
"test_event",
773-
static_cast<torch::executor::DebugHandle>(-1),
774-
1,
775-
2,
776-
nullptr,
777-
0);
763+
"test_event", kUnsetDelegateDebugIntId, 1, 2, nullptr, 0);
778764
// Event 4
779765
etdump_gen[i]->log_profiling_delegate(
780766
"test_event",
781-
static_cast<torch::executor::DebugHandle>(-1),
767+
kUnsetDelegateDebugIntId,
782768
1,
783769
2,
784770
metadata,
@@ -856,11 +842,11 @@ TEST_F(ProfilerETDumpTest, LogDelegateEvents) {
856842
std::string(delegate_debug_id_name, strlen(delegate_debug_id_name)),
857843
"test_event");
858844
// Event 2 used a string delegate debug identifier, so delegate_debug_id_int
859-
// should be -1.
845+
// should be kUnsetDelegateDebugIntId.
860846
EXPECT_EQ(
861847
etdump_ProfileEvent_delegate_debug_id_int(
862848
etdump_Event_profile_event(event)),
863-
-1);
849+
kUnsetDelegateDebugIntId);
864850
if (!etdump_gen[i]->is_static_etdump()) {
865851
free(result.buf);
866852
}

0 commit comments

Comments
 (0)