Skip to content

Add try-before-set tests for DataSink in DebugEvent and LogDelegateIntermediateOutput #9762

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Gasoonjia merged 8 commits into from
Apr 1, 2025
Merged

Add try-before-set tests for DataSink in DebugEvent and LogDelegateIntermediateOutput #9762

Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
4174f48
Refactor and add ET_EXPECT_DEATH tests to LogDelegateIntermediateOutput
keyprocedure Mar 31, 2025
9c6097a
Reuse class-level buf in LogDelegateIntermediateOutput test instead o…
keyprocedure Mar 31, 2025
5901797
Merge branch 'pytorch:main' into add-datasink-try-before-set-tests
keyprocedure Apr 1, 2025
8dec5b7
Add ET_EXPECT_DEATH tests to DebugEvent test
keyprocedure Apr 1, 2025
40ccf72
Merge branch 'main' into add-datasink-try-before-set-tests
Gasoonjia Apr 1, 2025
d49509e
Move create_event_block call under j == 0 condition
keyprocedure Apr 1, 2025
86024d8
Make expect_log_intermediate_delegate_death a protected member function
keyprocedure Apr 1, 2025
f27d896
Merge branch 'main' into add-datasink-try-before-set-tests
keyprocedure Apr 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
101 changes: 70 additions & 31 deletions devtools/etdump/tests/etdump_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,11 +186,16 @@ TEST_F(ProfilerETDumpTest, AllocationEvents) {
}

TEST_F(ProfilerETDumpTest, DebugEvent) {
const size_t debug_buf_size = 2048;
const size_t etdump_buf_size = 512 * 1024;
ASSERT_NE(this->buf, nullptr);
Span<uint8_t> span_buf = Span<uint8_t>(this->buf, etdump_buf_size);

for (size_t i = 0; i < 2; i++) {
for (size_t j = 0; j < 3; j++) {
etdump_gen[i]->create_event_block("test_block");

void* ptr = malloc(2048);
void* ptr = malloc(debug_buf_size);

EValue evalue_int((int64_t)5);
etdump_gen[i]->log_evalue(evalue_int);
Expand All @@ -206,24 +211,37 @@ TEST_F(ProfilerETDumpTest, DebugEvent) {
TensorFactory<ScalarType::Float> tf;
EValue evalue_tensor(tf.ones({3, 2}));

// using span to record debug data
Span<uint8_t> buffer((uint8_t*)ptr, 2048);
auto buffer_data_sink = BufferDataSink::create(ptr, 2048);
// Create span to record debug data
Span<uint8_t> buffer((uint8_t*)ptr, debug_buf_size);
auto buffer_data_sink = BufferDataSink::create(ptr, debug_buf_size);
auto file_data_sink = FileDataSink::create(dump_file_path.c_str());

if (j == 0) {
ET_EXPECT_DEATH(
etdump_gen[i]->log_evalue(evalue_tensor),
"Must set data sink before writing tensor-like data");

// Set debug buffer with span
etdump_gen[i]->set_debug_buffer(buffer);
}
// using buffer data sink to record debug data
else if (j == 1) {
etdump_gen[i]->set_data_sink(&buffer_data_sink.get());
}
// using file data sink to record debug data
else {
etdump_gen[i]->set_data_sink(&file_data_sink.get());
} else {
// Reset ETDumpGen to trigger ET_EXPECT_DEATH before setting data sink
delete etdump_gen[i];

// Recreate ETDumpGen; set span buffer only for etdump_gen[1]
etdump_gen[i] = (i == 0) ? new ETDumpGen() : new ETDumpGen(span_buf);
etdump_gen[i]->create_event_block("test_block");

ET_EXPECT_DEATH(
etdump_gen[i]->log_evalue(evalue_tensor),
"Must set data sink before writing tensor-like data");

if (j == 1) {
// Set buffer data sink
etdump_gen[i]->set_data_sink(&buffer_data_sink.get());
} else {
// Set file data sink
etdump_gen[i]->set_data_sink(&file_data_sink.get());
}
}

etdump_gen[i]->log_evalue(evalue_tensor);
Expand Down Expand Up @@ -497,36 +515,57 @@ TEST_F(ProfilerETDumpTest, VerifyData) {
}
}

// Triggers ET_EXPECT_DEATH if log_intermediate_output_delegate has no data sink
static void expect_log_intermediate_delegate_death(
Copy link
Contributor

@Gasoonjia Gasoonjia Apr 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love this helper function! Please make it as a protected member function of ProfilerETDumpTest to prohibit misuse. See https://github.com/pytorch/executorch/blob/main/kernels/test/op_split_copy_test.cpp#L94 for an example.

Copy link
Contributor Author

@keyprocedure keyprocedure Apr 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved the helper function, I appreciate the example!

ETDumpGen* gen,
TensorFactory<ScalarType::Float>& tf) {
ET_EXPECT_DEATH(
gen->log_intermediate_output_delegate(
"test_event_tensor",
static_cast<torch::executor::DebugHandle>(-1),
tf.ones({3, 2})),
"Must set data sink before writing tensor-like data");
}

TEST_F(ProfilerETDumpTest, LogDelegateIntermediateOutput) {
const size_t debug_buf_size = 2048;
const size_t etdump_buf_size = 512 * 1024;
ASSERT_NE(this->buf, nullptr);
Span<uint8_t> span_buf = Span<uint8_t>(this->buf, etdump_buf_size);

for (size_t i = 0; i < 2; i++) {
for (size_t j = 0; j < 3; j++) {
void* ptr = malloc(2048);
Span<uint8_t> buffer((uint8_t*)ptr, 2048);
void* ptr = malloc(debug_buf_size);
Span<uint8_t> buffer((uint8_t*)ptr, debug_buf_size);

auto buffer_data_sink = BufferDataSink::create(ptr, 2048);
auto buffer_data_sink = BufferDataSink::create(ptr, debug_buf_size);
auto file_data_sink = FileDataSink::create(dump_file_path.c_str());

etdump_gen[i]->create_event_block("test_block");
Copy link
Contributor

@Gasoonjia Gasoonjia Apr 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perhapes we can move this line under if (j == 0) to make more structural.

Copy link
Contributor

@Gasoonjia Gasoonjia Apr 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above!

Copy link
Contributor Author

@keyprocedure keyprocedure Apr 1, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! I moved line 544 under the if (j==0) condition

Does the second comment, "same as above", refer to line 209 in the most recently changed file?
It looks like that call creates a target block where values logged via log_evalue in the following lines are appended

TensorFactory<ScalarType::Float> tf;

// using span to record debug data
if (j == 0) {
// TODO(gasoonjia): add similar ET_EXPECT_DEATH on BufferDataSink branch
ET_EXPECT_DEATH(
etdump_gen[i]->log_intermediate_output_delegate(
"test_event_tensor",
static_cast<torch::executor::DebugHandle>(-1),
tf.ones({3, 2})),
"Must set data sink before writing tensor-like data");
expect_log_intermediate_delegate_death(etdump_gen[i], tf);

// Set debug buffer with span
etdump_gen[i]->set_debug_buffer(buffer);
}
// using buffer data sink to record debug data
else if (j == 1) {
etdump_gen[i]->set_data_sink(&buffer_data_sink.get());
}
// using file data sink to record debug data
else {
etdump_gen[i]->set_data_sink(&file_data_sink.get());
} else {
// Reset ETDumpGen to trigger ET_EXPECT_DEATH before setting data sink
delete etdump_gen[i];

// Recreate ETDumpGen; set span buffer only for etdump_gen[1]
etdump_gen[i] = (i == 0) ? new ETDumpGen() : new ETDumpGen(span_buf);
etdump_gen[i]->create_event_block("test_block");

expect_log_intermediate_delegate_death(etdump_gen[i], tf);

if (j == 1) {
// Set buffer data sink
etdump_gen[i]->set_data_sink(&buffer_data_sink.get());
} else {
// Set file data sink
etdump_gen[i]->set_data_sink(&file_data_sink.get());
}
}

// Log a tensor
Expand Down
Loading