diff --git a/devtools/etdump/etdump_flatcc.cpp b/devtools/etdump/etdump_flatcc.cpp index ccca2beb257..90310d85719 100644 --- a/devtools/etdump/etdump_flatcc.cpp +++ b/devtools/etdump/etdump_flatcc.cpp @@ -230,11 +230,12 @@ EventTracerEntry ETDumpGen::start_profiling( // TODO: Update all occurrences of the ProfileEvent calls once the // EventTracerEntry struct is updated. -EventTracerEntry ETDumpGen::start_profiling_delegate( +Result ETDumpGen::start_profiling_delegate( const char* name, DelegateDebugIntId delegate_debug_index) { - ET_CHECK_MSG( + ET_CHECK_OR_RETURN_ERROR( (name == nullptr) ^ (delegate_debug_index == kUnsetDelegateDebugIntId), + InvalidArgument, "Only name or delegate_debug_index can be valid. Check DelegateMappingBuilder documentation for more details."); check_ready_to_add_events(); EventTracerEntry prof_entry; @@ -247,10 +248,10 @@ EventTracerEntry ETDumpGen::start_profiling_delegate( ? create_string_entry(name) : delegate_debug_index; prof_entry.start_time = et_pal_current_ticks(); - return prof_entry; + return Result(prof_entry); } -void ETDumpGen::end_profiling_delegate( +Result ETDumpGen::end_profiling_delegate( EventTracerEntry event_tracer_entry, const void* metadata, size_t metadata_len) { @@ -281,9 +282,10 @@ void ETDumpGen::end_profiling_delegate( etdump_RunData_events_push_start(builder_); etdump_Event_profile_event_add(builder_, id); etdump_RunData_events_push_end(builder_); + return Result(true); } -void ETDumpGen::log_profiling_delegate( +Result ETDumpGen::log_profiling_delegate( const char* name, DelegateDebugIntId delegate_debug_index, et_timestamp_t start_time, @@ -313,6 +315,7 @@ void ETDumpGen::log_profiling_delegate( etdump_RunData_events_push_start(builder_); etdump_Event_profile_event_add(builder_, id); etdump_RunData_events_push_end(builder_); + return Result(true); } Result ETDumpGen::log_intermediate_output_delegate( diff --git a/devtools/etdump/etdump_flatcc.h b/devtools/etdump/etdump_flatcc.h index 8b39b243165..7119ce356aa 100644 --- a/devtools/etdump/etdump_flatcc.h +++ b/devtools/etdump/etdump_flatcc.h @@ -83,14 +83,15 @@ class ETDumpGen : public ::executorch::runtime::EventTracer { ::executorch::runtime::DebugHandle debug_handle = 0) override; virtual void end_profiling( ::executorch::runtime::EventTracerEntry prof_entry) override; - virtual ::executorch::runtime::EventTracerEntry start_profiling_delegate( + virtual Result<::executorch::runtime::EventTracerEntry> + start_profiling_delegate( const char* name, DelegateDebugIntId delegate_debug_index) override; - virtual void end_profiling_delegate( + virtual Result end_profiling_delegate( ::executorch::runtime::EventTracerEntry prof_entry, const void* metadata, size_t metadata_len) override; - virtual void log_profiling_delegate( + virtual Result log_profiling_delegate( const char* name, DelegateDebugIntId delegate_debug_index, et_timestamp_t start_time, diff --git a/devtools/etdump/tests/etdump_test.cpp b/devtools/etdump/tests/etdump_test.cpp index d095844986f..2374e9f54a9 100644 --- a/devtools/etdump/tests/etdump_test.cpp +++ b/devtools/etdump/tests/etdump_test.cpp @@ -820,8 +820,10 @@ TEST_F(ProfilerETDumpTest, LogDelegateEvents) { const char* metadata = "test_metadata"; etdump_gen[i]->log_profiling_delegate( nullptr, 278, 1, 2, metadata, strlen(metadata) + 1); - EventTracerEntry entry = etdump_gen[i]->start_profiling_delegate( + Result res = etdump_gen[i]->start_profiling_delegate( "test_event", kUnsetDelegateDebugIntId); + ASSERT_TRUE(res.ok()); + EventTracerEntry entry = res.get(); EXPECT_NE(entry.delegate_event_id_type, DelegateDebugIdType::kNone); // Event 2 etdump_gen[i]->end_profiling_delegate( diff --git a/docs/source/delegate-debugging.md b/docs/source/delegate-debugging.md index f14b640da79..1e546c94acc 100644 --- a/docs/source/delegate-debugging.md +++ b/docs/source/delegate-debugging.md @@ -89,7 +89,7 @@ To log events in real-time (for example, explicitly denoting the profiling start To start an `EventTracerEntry` using `event_tracer_start_profiling_delegate`, the **Delegate Debug Identifier** (provided AOT to the `debug_handle_map`) is passed as either the name or `delegate_debug_id` argument depending on the **Delegate Debug Identifier** type (str and int respectively) ```c++ -EventTracerEntry event_tracer_start_profiling_delegate( +Result event_tracer_start_profiling_delegate( EventTracer* event_tracer, const char* name, DebugHandle delegate_debug_id) @@ -100,7 +100,7 @@ To conclude an `EventTracerEntry`, `event_tracer_end_profiling_delegate` is simp Optionally, additional runtime `metadata` can also be logged at this point. ```c++ -void event_tracer_end_profiling_delegate( +Result event_tracer_end_profiling_delegate( EventTracer* event_tracer, EventTracerEntry event_tracer_entry, const void* metadata = nullptr, @@ -113,7 +113,7 @@ ExecuTorch also allows you to log in post time. Some runtime settings don't have To log events in post (for example, logging start and end time simultaneously) `event_tracer_log_profiling_delegate` is called with a combination of the arguments used in the real-time logging API’s and timestamps. ```c++ -void event_tracer_log_profiling_delegate( +Result event_tracer_log_profiling_delegate( EventTracer* event_tracer, const char* name, DebugHandle delegate_debug_id, diff --git a/runtime/core/event_tracer.h b/runtime/core/event_tracer.h index 5bcdd0cfb1f..a21add1a70e 100644 --- a/runtime/core/event_tracer.h +++ b/runtime/core/event_tracer.h @@ -216,8 +216,15 @@ class EventTracer { * @param[in] delegate_debug_index The id of the delegate event. If string * based names are used by this delegate to identify ops executed in the * backend then kUnsetDebugHandle should be passed in here. + * @return Returns an Result instance which may contatain + * a EventTracerEntry instance. The EventTracerEntry instance should be + * passed back into the end_profiling_delegate() call. + * - Result::ok() is true if the event tracer entry + * was successfully created. + * - Result::error() is true if an error occurs + * during logging. */ - virtual EventTracerEntry start_profiling_delegate( + virtual Result start_profiling_delegate( const char* name, DelegateDebugIntId delegate_debug_index) = 0; @@ -234,8 +241,11 @@ class EventTracer { * are transparent to the event tracer. It will just pipe along the data and * make it available for the user again in the post-processing stage. * @param[in] metadata_len Length of the metadata buffer. + * @return A Result indicating the status of the logging operation. + * - True if the event tracer delegate event was successfully logged. + * - An error code if an error occurs during logging. */ - virtual void end_profiling_delegate( + virtual Result end_profiling_delegate( EventTracerEntry event_tracer_entry, const void* metadata = nullptr, size_t metadata_len = 0) = 0; @@ -264,8 +274,11 @@ class EventTracer { * are transparent to the event tracer. It will just pipe along the data and * make it available for the user again in the post-processing stage. * @param[in] metadata_len Length of the metadata buffer. + * @return A Result indicating the status of the logging operation. + * - True if the event tracer delegate event was successfully logged. + * - An error code if an error occurs during logging. */ - virtual void log_profiling_delegate( + virtual Result log_profiling_delegate( const char* name, DelegateDebugIntId delegate_debug_index, et_timestamp_t start_time, diff --git a/runtime/core/event_tracer_hooks_delegate.h b/runtime/core/event_tracer_hooks_delegate.h index b2369fc216d..cb0566dd45d 100644 --- a/runtime/core/event_tracer_hooks_delegate.h +++ b/runtime/core/event_tracer_hooks_delegate.h @@ -53,7 +53,10 @@ inline EventTracerEntry event_tracer_start_profiling_delegate( DebugHandle delegate_debug_id) { #ifdef ET_EVENT_TRACER_ENABLED if (event_tracer) { - return event_tracer->start_profiling_delegate(name, delegate_debug_id); + Result res = + event_tracer->start_profiling_delegate(name, delegate_debug_id); + ET_CHECK_MSG(res.ok(), "Failed to start profiling delegate event"); + return res.get(); } #else //! ET_EVENT_TRACER_ENABLED (void)name; @@ -179,8 +182,9 @@ inline void event_tracer_log_output_delegate( std::is_same::value || std::is_same>::value, "Unsupported type for intermediate output"); - event_tracer->log_intermediate_output_delegate( + Result res = event_tracer->log_intermediate_output_delegate( name, delegate_debug_id, output); + ET_CHECK_MSG(res.ok(), "Failed to log intermediate output delegate event"); } #else //! ET_EVENT_TRACER_ENABLED (void)name; diff --git a/runtime/core/test/event_tracer_test.cpp b/runtime/core/test/event_tracer_test.cpp index 1c9e1a446b9..24fd48989eb 100644 --- a/runtime/core/test/event_tracer_test.cpp +++ b/runtime/core/test/event_tracer_test.cpp @@ -74,21 +74,22 @@ class DummyEventTracer : public EventTracer { return 0; } - EventTracerEntry start_profiling_delegate( + Result start_profiling_delegate( const char* name, DelegateDebugIntId delegate_debug_id) override { (void)name; (void)delegate_debug_id; - return EventTracerEntry(); + return Result(EventTracerEntry()); } - void end_profiling_delegate( + Result end_profiling_delegate( ET_UNUSED EventTracerEntry event_tracer_entry, ET_UNUSED const void* metadata, ET_UNUSED size_t metadata_len) override { (void)event_tracer_entry; (void)metadata; (void)metadata_len; + return true; } void set_delegation_intermediate_output_filter( @@ -96,7 +97,7 @@ class DummyEventTracer : public EventTracer { (void)event_tracer_filter; } - void log_profiling_delegate( + Result log_profiling_delegate( const char* name, DelegateDebugIntId delegate_debug_id, et_timestamp_t start_time, @@ -109,6 +110,7 @@ class DummyEventTracer : public EventTracer { (void)end_time; (void)metadata; (void)metadata_len; + return true; } virtual Result log_intermediate_output_delegate(