From 57e49b584d60a7238b113b255898250a7d03a70d Mon Sep 17 00:00:00 2001 From: Samuel Susla Date: Mon, 5 Feb 2024 07:43:17 -0800 Subject: [PATCH] remove gating around improved onLayout batching (#42631) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/42631 changelog: [General][Breaking] onLayout event batching changes Reviewed By: mdvacca, veviego Differential Revision: D52955970 fbshipit-source-id: 9e8a340c47bd0e39a55cc1d16a6361959342c55f --- .../React/Fabric/RCTSurfacePresenter.mm | 4 -- .../ReactAndroid/api/ReactAndroid.api | 1 - .../react/config/ReactFeatureFlags.java | 3 - .../src/main/jni/react/fabric/Binding.cpp | 2 - .../components/view/BaseViewEventEmitter.cpp | 68 +++++++++---------- .../react/renderer/core/ConcreteState.h | 7 +- .../renderer/core/EventQueueProcessor.cpp | 11 +-- .../react/renderer/scheduler/Scheduler.cpp | 10 +-- .../ReactCommon/react/utils/CoreFeatures.cpp | 1 - .../ReactCommon/react/utils/CoreFeatures.h | 3 - 10 files changed, 35 insertions(+), 75 deletions(-) diff --git a/packages/react-native/React/Fabric/RCTSurfacePresenter.mm b/packages/react-native/React/Fabric/RCTSurfacePresenter.mm index 1668444eeb949f..16076436dd915f 100644 --- a/packages/react-native/React/Fabric/RCTSurfacePresenter.mm +++ b/packages/react-native/React/Fabric/RCTSurfacePresenter.mm @@ -268,10 +268,6 @@ - (RCTScheduler *)_createScheduler CoreFeatures::enableMountHooks = true; } - if (reactNativeConfig && reactNativeConfig->getBool("react_fabric:enable_default_async_batched_priority")) { - CoreFeatures::enableDefaultAsyncBatchedPriority = true; - } - if (reactNativeConfig && reactNativeConfig->getBool("react_fabric:enable_cloneless_state_progression")) { CoreFeatures::enableClonelessStateProgression = true; } diff --git a/packages/react-native/ReactAndroid/api/ReactAndroid.api b/packages/react-native/ReactAndroid/api/ReactAndroid.api index 2a5f753722afab..cb4107736741ce 100644 --- a/packages/react-native/ReactAndroid/api/ReactAndroid.api +++ b/packages/react-native/ReactAndroid/api/ReactAndroid.api @@ -1896,7 +1896,6 @@ public class com/facebook/react/config/ReactFeatureFlags { public static field enableBridgelessArchitectureSoftExceptions Z public static field enableClonelessStateProgression Z public static field enableCppPropsIteratorSetter Z - public static field enableDefaultAsyncBatchedPriority Z public static field enableEagerRootViewAttachment Z public static field enableFabricLogs Z public static field enableFabricPendingEventQueue Z diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java index 7d06c1b2cd593c..f7c66a649ee936 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java @@ -118,9 +118,6 @@ public class ReactFeatureFlags { /** Use native view configs in bridgeless mode. */ public static boolean useNativeViewConfigsInBridgelessMode = false; - /** Default state updates and events to async batched priority. */ - public static boolean enableDefaultAsyncBatchedPriority = false; - /** Utilize shared Event C++ pipeline with fabric's renderer */ public static boolean enableFabricSharedEventPipeline = true; diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/fabric/Binding.cpp b/packages/react-native/ReactAndroid/src/main/jni/react/fabric/Binding.cpp index 3331911f0f17ec..70f01ae35e4d74 100644 --- a/packages/react-native/ReactAndroid/src/main/jni/react/fabric/Binding.cpp +++ b/packages/react-native/ReactAndroid/src/main/jni/react/fabric/Binding.cpp @@ -414,8 +414,6 @@ void Binding::installFabricUIManager( CoreFeatures::enablePropIteratorSetter = getFeatureFlagValue("enableCppPropsIteratorSetter"); - CoreFeatures::enableDefaultAsyncBatchedPriority = - getFeatureFlagValue("enableDefaultAsyncBatchedPriority"); CoreFeatures::enableClonelessStateProgression = getFeatureFlagValue("enableClonelessStateProgression"); CoreFeatures::excludeYogaFromRawProps = diff --git a/packages/react-native/ReactCommon/react/renderer/components/view/BaseViewEventEmitter.cpp b/packages/react-native/ReactCommon/react/renderer/components/view/BaseViewEventEmitter.cpp index f349f2503f6514..506041daf320fc 100644 --- a/packages/react-native/ReactCommon/react/renderer/components/view/BaseViewEventEmitter.cpp +++ b/packages/react-native/ReactCommon/react/renderer/components/view/BaseViewEventEmitter.cpp @@ -6,7 +6,6 @@ */ #include "BaseViewEventEmitter.h" -#include namespace facebook::react { @@ -80,42 +79,37 @@ void BaseViewEventEmitter::onLayout(const LayoutMetrics& layoutMetrics) const { layoutEventState->isDispatching = true; } - dispatchEvent( - "layout", - [layoutEventState](jsi::Runtime& runtime) { - auto frame = Rect{}; - - { - std::scoped_lock guard(layoutEventState->mutex); - - layoutEventState->isDispatching = false; - - // If some *particular* `frame` was already dispatched before, - // and since then there were no other new values of the `frame` - // observed, do nothing. - if (layoutEventState->wasDispatched) { - return jsi::Value::null(); - } - - frame = layoutEventState->frame; - - // If some *particular* `frame` was *not* already dispatched before, - // it's time to dispatch it and mark as dispatched. - layoutEventState->wasDispatched = true; - } - - auto layout = jsi::Object(runtime); - layout.setProperty(runtime, "x", frame.origin.x); - layout.setProperty(runtime, "y", frame.origin.y); - layout.setProperty(runtime, "width", frame.size.width); - layout.setProperty(runtime, "height", frame.size.height); - auto payload = jsi::Object(runtime); - payload.setProperty(runtime, "layout", std::move(layout)); - return jsi::Value(std::move(payload)); - }, - CoreFeatures::enableDefaultAsyncBatchedPriority - ? EventPriority::AsynchronousBatched - : EventPriority::AsynchronousUnbatched); + dispatchEvent("layout", [layoutEventState](jsi::Runtime& runtime) { + auto frame = Rect{}; + + { + std::scoped_lock guard(layoutEventState->mutex); + + layoutEventState->isDispatching = false; + + // If some *particular* `frame` was already dispatched before, + // and since then there were no other new values of the `frame` + // observed, do nothing. + if (layoutEventState->wasDispatched) { + return jsi::Value::null(); + } + + frame = layoutEventState->frame; + + // If some *particular* `frame` was *not* already dispatched before, + // it's time to dispatch it and mark as dispatched. + layoutEventState->wasDispatched = true; + } + + auto layout = jsi::Object(runtime); + layout.setProperty(runtime, "x", frame.origin.x); + layout.setProperty(runtime, "y", frame.origin.y); + layout.setProperty(runtime, "width", frame.size.width); + layout.setProperty(runtime, "height", frame.size.height); + auto payload = jsi::Object(runtime); + payload.setProperty(runtime, "layout", std::move(layout)); + return jsi::Value(std::move(payload)); + }); } } // namespace facebook::react diff --git a/packages/react-native/ReactCommon/react/renderer/core/ConcreteState.h b/packages/react-native/ReactCommon/react/renderer/core/ConcreteState.h index cfc3a1a4f954fb..6a0ab8daf2f0b2 100644 --- a/packages/react-native/ReactCommon/react/renderer/core/ConcreteState.h +++ b/packages/react-native/ReactCommon/react/renderer/core/ConcreteState.h @@ -12,7 +12,6 @@ #include #include -#include namespace facebook::react { @@ -68,11 +67,7 @@ class ConcreteState : public State { } void updateState(Data&& newData) const { - updateState( - std::move(newData), - CoreFeatures::enableDefaultAsyncBatchedPriority - ? EventPriority::AsynchronousBatched - : EventPriority::AsynchronousUnbatched); + updateState(std::move(newData), EventPriority::AsynchronousBatched); } /* diff --git a/packages/react-native/ReactCommon/react/renderer/core/EventQueueProcessor.cpp b/packages/react-native/ReactCommon/react/renderer/core/EventQueueProcessor.cpp index b595cc31895c87..a49882f438bcd6 100644 --- a/packages/react-native/ReactCommon/react/renderer/core/EventQueueProcessor.cpp +++ b/packages/react-native/ReactCommon/react/renderer/core/EventQueueProcessor.cpp @@ -7,7 +7,6 @@ #include #include -#include #include "EventEmitter.h" #include "EventLogger.h" #include "EventQueue.h" @@ -71,11 +70,6 @@ void EventQueueProcessor::flushEvents( reactPriority, *event.eventPayload); - // We run the "Conclusion" per-event when unbatched - if (!CoreFeatures::enableDefaultAsyncBatchedPriority) { - eventPipeConclusion_(runtime); - } - if (eventLogger != nullptr) { eventLogger->onEventProcessingEnd(event.loggingTag); } @@ -84,10 +78,9 @@ void EventQueueProcessor::flushEvents( hasContinuousEventStarted_ = true; } } + // We only run the "Conclusion" once per event group when batched. - if (CoreFeatures::enableDefaultAsyncBatchedPriority) { - eventPipeConclusion_(runtime); - } + 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/scheduler/Scheduler.cpp b/packages/react-native/ReactCommon/react/renderer/scheduler/Scheduler.cpp index b5493677cd007e..ee4ed0a1f40e01 100644 --- a/packages/react-native/ReactCommon/react/renderer/scheduler/Scheduler.cpp +++ b/packages/react-native/ReactCommon/react/renderer/scheduler/Scheduler.cpp @@ -66,19 +66,11 @@ Scheduler::Scheduler( runtime, eventTarget, type, priority, payload); }, 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) { + if (runtimeScheduler != nullptr) { runtimeScheduler->callExpiredTasks(runtime); } }; diff --git a/packages/react-native/ReactCommon/react/utils/CoreFeatures.cpp b/packages/react-native/ReactCommon/react/utils/CoreFeatures.cpp index cfa5c7319cf6c7..1ae6897bfbc1cc 100644 --- a/packages/react-native/ReactCommon/react/utils/CoreFeatures.cpp +++ b/packages/react-native/ReactCommon/react/utils/CoreFeatures.cpp @@ -14,7 +14,6 @@ bool CoreFeatures::cacheLastTextMeasurement = false; bool CoreFeatures::enableGranularScrollViewStateUpdatesIOS = false; bool CoreFeatures::enableMountHooks = false; bool CoreFeatures::enableGranularShadowTreeStateReconciliation = false; -bool CoreFeatures::enableDefaultAsyncBatchedPriority = false; bool CoreFeatures::enableClonelessStateProgression = false; bool CoreFeatures::excludeYogaFromRawProps = false; bool CoreFeatures::enableReportEventPaintTime = false; diff --git a/packages/react-native/ReactCommon/react/utils/CoreFeatures.h b/packages/react-native/ReactCommon/react/utils/CoreFeatures.h index 9f0a3d630c3df3..1cf5dc68d1e2df 100644 --- a/packages/react-native/ReactCommon/react/utils/CoreFeatures.h +++ b/packages/react-native/ReactCommon/react/utils/CoreFeatures.h @@ -36,9 +36,6 @@ class CoreFeatures { // state and the last commit that updated state changed before committing. static bool enableGranularShadowTreeStateReconciliation; - // Default state updates and events to async batched priority. - static bool enableDefaultAsyncBatchedPriority; - // When enabled, Fabric will avoid cloning notes to perform state progression. static bool enableClonelessStateProgression;