From a4da5da42e5ae4d96d1dcd93dc909c5320bd914d Mon Sep 17 00:00:00 2001 From: Vincent Viego Date: Tue, 15 Aug 2023 18:31:31 -0700 Subject: [PATCH] Only execute expired tasks once per batched event group (#39013) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/39013 Changelog: [internal] Reviewed By: sammy-SC Differential Revision: D48210131 fbshipit-source-id: 1410aab2f70b75e71fd8a6ace8f4653682daaf90 --- .../react/renderer/core/EventPipe.h | 2 ++ .../renderer/core/EventQueueProcessor.cpp | 15 ++++++++++++++- .../react/renderer/core/EventQueueProcessor.h | 6 +++++- .../core/tests/EventQueueProcessorTest.cpp | 5 +++-- .../react/renderer/scheduler/Scheduler.cpp | 18 +++++++++++++++--- 5 files changed, 39 insertions(+), 7 deletions(-) diff --git a/packages/react-native/ReactCommon/react/renderer/core/EventPipe.h b/packages/react-native/ReactCommon/react/renderer/core/EventPipe.h index a31bc54d5ad5c5..28dec4068dc01a 100644 --- a/packages/react-native/ReactCommon/react/renderer/core/EventPipe.h +++ b/packages/react-native/ReactCommon/react/renderer/core/EventPipe.h @@ -25,4 +25,6 @@ using EventPipe = std::function; +using EventPipeConclusion = std::function; + } // namespace facebook::react diff --git a/packages/react-native/ReactCommon/react/renderer/core/EventQueueProcessor.cpp b/packages/react-native/ReactCommon/react/renderer/core/EventQueueProcessor.cpp index 776f649a295807..f3d4a95bc56b41 100644 --- a/packages/react-native/ReactCommon/react/renderer/core/EventQueueProcessor.cpp +++ b/packages/react-native/ReactCommon/react/renderer/core/EventQueueProcessor.cpp @@ -7,6 +7,7 @@ #include #include +#include #include "EventEmitter.h" #include "EventLogger.h" #include "EventQueue.h" @@ -16,8 +17,11 @@ namespace facebook::react { EventQueueProcessor::EventQueueProcessor( EventPipe eventPipe, + EventPipeConclusion eventPipeConclusion, StatePipe statePipe) - : eventPipe_(std::move(eventPipe)), statePipe_(std::move(statePipe)) {} + : eventPipe_(std::move(eventPipe)), + eventPipeConclusion_(std::move(eventPipeConclusion)), + statePipe_(std::move(statePipe)) {} void EventQueueProcessor::flushEvents( jsi::Runtime &runtime, @@ -67,6 +71,11 @@ void EventQueueProcessor::flushEvents( reactPriority, *event.eventPayload); + // We run the "Conclusion" per-event when unbatched + if (!CoreFeatures::enableDefaultAsyncBatchedPriority) { + eventPipeConclusion_(runtime); + } + if (eventLogger != nullptr) { eventLogger->onEventEnd(event.loggingTag); } @@ -75,6 +84,10 @@ void EventQueueProcessor::flushEvents( hasContinuousEventStarted_ = true; } } + // We only run the "Conclusion" once per event group when batched. + if (CoreFeatures::enableDefaultAsyncBatchedPriority) { + eventPipeConclusion_(runtime); + } // No need to lock `EventEmitter::DispatchMutex()` here. // The mutex protects from a situation when the `instanceHandle` can be diff --git a/packages/react-native/ReactCommon/react/renderer/core/EventQueueProcessor.h b/packages/react-native/ReactCommon/react/renderer/core/EventQueueProcessor.h index e2e81c14d2bf02..63aeee12d34760 100644 --- a/packages/react-native/ReactCommon/react/renderer/core/EventQueueProcessor.h +++ b/packages/react-native/ReactCommon/react/renderer/core/EventQueueProcessor.h @@ -19,13 +19,17 @@ namespace facebook::react { class EventQueueProcessor { public: - EventQueueProcessor(EventPipe eventPipe, StatePipe statePipe); + EventQueueProcessor( + EventPipe eventPipe, + EventPipeConclusion eventPipeConclusion, + StatePipe statePipe); void flushEvents(jsi::Runtime &runtime, std::vector &&events) const; void flushStateUpdates(std::vector &&states) const; private: EventPipe const eventPipe_; + EventPipeConclusion const eventPipeConclusion_; StatePipe const statePipe_; mutable bool hasContinuousEventStarted_{false}; diff --git a/packages/react-native/ReactCommon/react/renderer/core/tests/EventQueueProcessorTest.cpp b/packages/react-native/ReactCommon/react/renderer/core/tests/EventQueueProcessorTest.cpp index 3bc03bf1a39974..b6ebaffc3a052d 100644 --- a/packages/react-native/ReactCommon/react/renderer/core/tests/EventQueueProcessorTest.cpp +++ b/packages/react-native/ReactCommon/react/renderer/core/tests/EventQueueProcessorTest.cpp @@ -32,10 +32,11 @@ class EventQueueProcessorTest : public testing::Test { eventPriorities_.push_back(priority); }; + auto dummyEventPipeConclusion = [](jsi::Runtime &runtime) {}; auto dummyStatePipe = [](StateUpdate const &stateUpdate) {}; - eventProcessor_ = - std::make_unique(eventPipe, dummyStatePipe); + eventProcessor_ = std::make_unique( + eventPipe, dummyEventPipeConclusion, dummyStatePipe); } std::unique_ptr runtime_; diff --git a/packages/react-native/ReactCommon/react/renderer/scheduler/Scheduler.cpp b/packages/react-native/ReactCommon/react/renderer/scheduler/Scheduler.cpp index dc09ecb78b6fd2..a689389628844e 100644 --- a/packages/react-native/ReactCommon/react/renderer/scheduler/Scheduler.cpp +++ b/packages/react-native/ReactCommon/react/renderer/scheduler/Scheduler.cpp @@ -67,11 +67,23 @@ Scheduler::Scheduler( runtime, eventTarget, type, priority, payload); }, runtime); - if (runtimeScheduler != nullptr) { - runtimeScheduler->callExpiredTasks(runtime); + + // We only want to run this per-event if we are not batching by default + if (!CoreFeatures::enableDefaultAsyncBatchedPriority) { + if (runtimeScheduler != nullptr) { + runtimeScheduler->callExpiredTasks(runtime); + } } }; + auto eventPipeConclusion = + [runtimeScheduler = runtimeScheduler.get()](jsi::Runtime &runtime) { + if (CoreFeatures::enableDefaultAsyncBatchedPriority && + runtimeScheduler != nullptr) { + runtimeScheduler->callExpiredTasks(runtime); + } + }; + auto statePipe = [uiManager](StateUpdate const &stateUpdate) { uiManager->updateState(stateUpdate); }; @@ -79,7 +91,7 @@ Scheduler::Scheduler( // Creating an `EventDispatcher` instance inside the already allocated // container (inside the optional). eventDispatcher_->emplace( - EventQueueProcessor(eventPipe, statePipe), + EventQueueProcessor(eventPipe, eventPipeConclusion, statePipe), schedulerToolbox.synchronousEventBeatFactory, schedulerToolbox.asynchronousEventBeatFactory, eventOwnerBox);