Skip to content

Commit 357ed9d

Browse files
Error handling log intermediate output delegate (#9761)
### Summary This PR contains a fix for all `log_intermediate_output_delegate` methods which can now propagate any error caught inside of `log_intermediate_output_delegate_helper`. Fixes #9549 release notes: `log_intermediate_output_delegate` no longer aborts runtime if parameters delegate debug index or name are invalid. Instead, an error is returned. ### Test plan Tests for this PR can be verified by building `sdk_etdump_tests` and using `ctest` --------- Co-authored-by: Gasoonjia <[email protected]>
1 parent 7cc25b1 commit 357ed9d

File tree

3 files changed

+66
-12
lines changed

3 files changed

+66
-12
lines changed

devtools/etdump/etdump_flatcc.cpp

+24-10
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@ using ::executorch::runtime::Tag;
3636

3737
namespace executorch {
3838
namespace etdump {
39-
4039
namespace {
4140

4241
executorch_flatbuffer_ScalarType_enum_t get_flatbuffer_scalar_type(
@@ -311,49 +310,59 @@ Result<bool> ETDumpGen::log_intermediate_output_delegate(
311310
const char* name,
312311
DebugHandle delegate_debug_index,
313312
const Tensor& output) {
314-
log_intermediate_output_delegate_helper(name, delegate_debug_index, output);
315-
return true;
313+
Result<bool> result = log_intermediate_output_delegate_helper(
314+
name, delegate_debug_index, output);
315+
return result;
316316
}
317317

318318
Result<bool> ETDumpGen::log_intermediate_output_delegate(
319319
const char* name,
320320
DebugHandle delegate_debug_index,
321321
const ArrayRef<Tensor> output) {
322322
log_intermediate_output_delegate_helper(name, delegate_debug_index, output);
323-
return true;
323+
Result<bool> result = log_intermediate_output_delegate_helper(
324+
name, delegate_debug_index, output);
325+
return result;
324326
}
325327

326328
Result<bool> ETDumpGen::log_intermediate_output_delegate(
327329
const char* name,
328330
DebugHandle delegate_debug_index,
329331
const int& output) {
330332
log_intermediate_output_delegate_helper(name, delegate_debug_index, output);
331-
return true;
333+
Result<bool> result = log_intermediate_output_delegate_helper(
334+
name, delegate_debug_index, output);
335+
return result;
332336
}
333337

334338
Result<bool> ETDumpGen::log_intermediate_output_delegate(
335339
const char* name,
336340
DebugHandle delegate_debug_index,
337341
const bool& output) {
338342
log_intermediate_output_delegate_helper(name, delegate_debug_index, output);
339-
return true;
343+
Result<bool> result = log_intermediate_output_delegate_helper(
344+
name, delegate_debug_index, output);
345+
return result;
340346
}
341347

342348
Result<bool> ETDumpGen::log_intermediate_output_delegate(
343349
const char* name,
344350
DebugHandle delegate_debug_index,
345351
const double& output) {
346352
log_intermediate_output_delegate_helper(name, delegate_debug_index, output);
347-
return true;
353+
Result<bool> result = log_intermediate_output_delegate_helper(
354+
name, delegate_debug_index, output);
355+
return result;
348356
}
349357

350358
template <typename T>
351-
void ETDumpGen::log_intermediate_output_delegate_helper(
359+
Result<bool> ETDumpGen::log_intermediate_output_delegate_helper(
352360
const char* name,
353361
DebugHandle delegate_debug_index,
354362
const T& output) {
355-
ET_CHECK_MSG(
363+
ET_CHECK_OR_RETURN_ERROR(
356364
(name == nullptr) ^ (delegate_debug_index == -1),
365+
InvalidArgument,
357366
"Only name or delegate_debug_index can be valid. Check DelegateMappingBuilder documentation for more details.");
358367

359368
check_ready_to_add_events();
@@ -413,7 +422,10 @@ void ETDumpGen::log_intermediate_output_delegate_helper(
413422
etdump_Value_bool_value_add(builder_, bool_ref);
414423
etdump_Value_val_add(builder_, etdump_ValueType_Bool);
415424
} else {
416-
ET_CHECK_MSG(0, "Unsupported output type for intermediate logging\n");
425+
ET_CHECK_OR_RETURN_ERROR(
426+
0,
427+
InvalidArgument,
428+
"Unsupported output type for intermediate logging\n");
417429
}
418430

419431
auto value_ref = etdump_Value_end(builder_);
@@ -424,6 +436,8 @@ void ETDumpGen::log_intermediate_output_delegate_helper(
424436
etdump_RunData_events_push_start(builder_);
425437
etdump_Event_debug_event_add(builder_, debug_event);
426438
etdump_RunData_events_push_end(builder_);
439+
440+
return true;
427441
}
428442

429443
void ETDumpGen::end_profiling(EventTracerEntry prof_entry) {

devtools/etdump/etdump_flatcc.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ class ETDumpGen : public ::executorch::runtime::EventTracer {
171171
* Supported types include tensor, tensor array, int, bool and double.
172172
*/
173173
template <typename T>
174-
void log_intermediate_output_delegate_helper(
174+
Result<bool> log_intermediate_output_delegate_helper(
175175
const char* name,
176176
::executorch::runtime::DebugHandle delegate_debug_index,
177177
const T& output);

devtools/etdump/tests/etdump_test.cpp

+41-1
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,13 @@ using ::executorch::aten::ScalarType;
2727
using ::executorch::aten::Tensor;
2828
using ::executorch::etdump::ETDumpGen;
2929
using ::executorch::etdump::ETDumpResult;
30+
using ::executorch::etdump::Result;
3031
using ::executorch::extension::testing::TempFile;
3132
using ::executorch::runtime::AllocatorID;
3233
using ::executorch::runtime::ArrayRef;
3334
using ::executorch::runtime::BoxedEvalueList;
3435
using ::executorch::runtime::DelegateDebugIdType;
36+
using ::executorch::runtime::Error;
3537
using ::executorch::runtime::EValue;
3638
using ::executorch::runtime::EventTracerEntry;
3739
using ::executorch::runtime::LoggedEValueType;
@@ -529,14 +531,52 @@ TEST_F(ProfilerETDumpTest, LogDelegateIntermediateOutput) {
529531
etdump_gen[i]->set_data_sink(&file_data_sink.get());
530532
}
531533

534+
// Only a valid name or delegate debug index should be passed in. If valid
535+
// entries are passed in for both then the test should assert out.
536+
Result<bool> log_tensor_result =
537+
etdump_gen[i]->log_intermediate_output_delegate(
538+
"test_event_tensor",
539+
static_cast<torch::executor::DebugHandle>(2589),
540+
tf.ones({3, 2}));
541+
542+
std::vector<Tensor> tensors = {tf.ones({5, 4}), tf.ones({7, 6})};
543+
Result<bool> log_tensor_list_result =
544+
etdump_gen[i]->log_intermediate_output_delegate(
545+
nullptr,
546+
static_cast<torch::executor::DebugHandle>(-1),
547+
ArrayRef<Tensor>(tensors.data(), tensors.size()));
548+
549+
Result<bool> log_int_result =
550+
etdump_gen[i]->log_intermediate_output_delegate(
551+
"test_event_tensor",
552+
static_cast<torch::executor::DebugHandle>(2589),
553+
10);
554+
555+
Result<bool> log_double_result =
556+
etdump_gen[i]->log_intermediate_output_delegate(
557+
"test_event_tensor",
558+
static_cast<torch::executor::DebugHandle>(2589),
559+
29.82);
560+
561+
Result<bool> log_bool_result =
562+
etdump_gen[i]->log_intermediate_output_delegate(
563+
nullptr, static_cast<torch::executor::DebugHandle>(-1), 29.82);
564+
565+
ASSERT_EQ(log_tensor_result.error(), Error::InvalidArgument);
566+
ASSERT_EQ(log_tensor_list_result.error(), Error::InvalidArgument);
567+
ASSERT_EQ(log_int_result.error(), Error::InvalidArgument);
568+
ASSERT_EQ(log_double_result.error(), Error::InvalidArgument);
569+
ASSERT_EQ(log_bool_result.error(), Error::InvalidArgument);
570+
571+
// Now we check log intermediate output delegate with valid args
572+
532573
// Log a tensor
533574
etdump_gen[i]->log_intermediate_output_delegate(
534575
"test_event_tensor",
535576
static_cast<torch::executor::DebugHandle>(-1),
536577
tf.ones({3, 2}));
537578

538579
// Log a tensor list
539-
std::vector<Tensor> tensors = {tf.ones({5, 4}), tf.ones({7, 6})};
540580
etdump_gen[i]->log_intermediate_output_delegate(
541581
"test_event_tensorlist",
542582
static_cast<torch::executor::DebugHandle>(-1),

0 commit comments

Comments
 (0)