From efcc798de90cb5e0f1d918fd3c3daefc7413be1c Mon Sep 17 00:00:00 2001 From: Ruslan Lesiutin Date: Mon, 19 May 2025 08:34:29 -0700 Subject: [PATCH] Make processingStartTime and processingEndTime optional (#51390) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/51390 # Changelog: [Internal] Ideally, these should not be optional, but these values are set at runtime. Assertions added to validate that these values are set before reporting to `PerformanceEntryReporter`. Reviewed By: rshest Differential Revision: D74811439 fbshipit-source-id: 5e95931848263a58d977c07bac0eb18e53b91140 --- .../renderer/observers/events/CMakeLists.txt | 1 + .../events/EventPerformanceLogger.cpp | 27 +++++++++++++++---- .../observers/events/EventPerformanceLogger.h | 9 ++++--- 3 files changed, 28 insertions(+), 9 deletions(-) diff --git a/packages/react-native/ReactCommon/react/renderer/observers/events/CMakeLists.txt b/packages/react-native/ReactCommon/react/renderer/observers/events/CMakeLists.txt index 7c5b9510937..b50aaa28357 100644 --- a/packages/react-native/ReactCommon/react/renderer/observers/events/CMakeLists.txt +++ b/packages/react-native/ReactCommon/react/renderer/observers/events/CMakeLists.txt @@ -13,6 +13,7 @@ add_library(react_renderer_observers_events OBJECT ${react_renderer_observers_ev target_include_directories(react_renderer_observers_events PUBLIC ${REACT_COMMON_DIR}) target_link_libraries(react_renderer_observers_events + react_debug react_performance_timeline react_timing react_renderer_core diff --git a/packages/react-native/ReactCommon/react/renderer/observers/events/EventPerformanceLogger.cpp b/packages/react-native/ReactCommon/react/renderer/observers/events/EventPerformanceLogger.cpp index ba9efde297d..6dc9680b232 100644 --- a/packages/react-native/ReactCommon/react/renderer/observers/events/EventPerformanceLogger.cpp +++ b/packages/react-native/ReactCommon/react/renderer/observers/events/EventPerformanceLogger.cpp @@ -7,8 +7,10 @@ #include "EventPerformanceLogger.h" +#include #include #include + #include namespace facebook::react { @@ -127,7 +129,7 @@ EventTag EventPerformanceLogger::onEventStart( { std::lock_guard lock(eventsInFlightMutex_); eventsInFlight_.emplace( - eventTag, EventEntry{reportedName, target, timeStamp, 0.0}); + eventTag, EventEntry{reportedName, target, timeStamp}); } return eventTag; } @@ -163,6 +165,9 @@ void EventPerformanceLogger::onEventProcessingEnd(EventTag tag) { } auto& entry = it->second; + react_native_assert( + entry.processingStartTime.has_value() && + "Attempting to set processingEndTime while processingStartTime is not set."); entry.processingEndTime = timeStamp; } } @@ -188,12 +193,18 @@ void EventPerformanceLogger::dispatchPendingEventTimingEntries( entry.isWaitingForMount = true; ++it; } else { + react_native_assert( + entry.processingStartTime.has_value() && + "Attempted to report PerformanceEventTiming, which did not have processingStartTime defined."); + react_native_assert( + entry.processingEndTime.has_value() && + "Attempted to report PerformanceEventTiming, which did not have processingEndTime defined."); performanceEntryReporter->reportEvent( std::string(entry.name), entry.startTime, performanceEntryReporter->getCurrentTimeStamp() - entry.startTime, - entry.processingStartTime, - entry.processingEndTime, + entry.processingStartTime.value(), + entry.processingEndTime.value(), entry.interactionId); it = eventsInFlight_.erase(it); } @@ -214,12 +225,18 @@ void EventPerformanceLogger::shadowTreeDidMount( const auto& entry = it->second; if (entry.isWaitingForMount && isTargetInRootShadowNode(entry.target, rootShadowNode)) { + react_native_assert( + entry.processingStartTime.has_value() && + "Attempted to report PerformanceEventTiming, which did not have processingStartTime defined."); + react_native_assert( + entry.processingEndTime.has_value() && + "Attempted to report PerformanceEventTiming, which did not have processingEndTime defined."); performanceEntryReporter->reportEvent( std::string(entry.name), entry.startTime, mountTime - entry.startTime, - entry.processingStartTime, - entry.processingEndTime, + entry.processingStartTime.value(), + entry.processingEndTime.value(), entry.interactionId); it = eventsInFlight_.erase(it); } else { diff --git a/packages/react-native/ReactCommon/react/renderer/observers/events/EventPerformanceLogger.h b/packages/react-native/ReactCommon/react/renderer/observers/events/EventPerformanceLogger.h index 4e1ed875ea5..ffa1d01fb79 100644 --- a/packages/react-native/ReactCommon/react/renderer/observers/events/EventPerformanceLogger.h +++ b/packages/react-native/ReactCommon/react/renderer/observers/events/EventPerformanceLogger.h @@ -11,6 +11,7 @@ #include #include #include + #include #include #include @@ -52,9 +53,9 @@ class EventPerformanceLogger : public EventLogger, struct EventEntry { std::string_view name; SharedEventTarget target{nullptr}; - DOMHighResTimeStamp startTime{0.0}; - DOMHighResTimeStamp processingStartTime{0.0}; - DOMHighResTimeStamp processingEndTime{0.0}; + DOMHighResTimeStamp startTime; + std::optional processingStartTime; + std::optional processingEndTime; bool isWaitingForMount{false}; @@ -63,7 +64,7 @@ class EventPerformanceLogger : public EventLogger, PerformanceEntryInteractionId interactionId{0}; bool isWaitingForDispatch() { - return processingEndTime == 0.0; + return !processingEndTime.has_value(); } };