From c678ffb3fd2b58a62bb5873467df51eecee45c30 Mon Sep 17 00:00:00 2001 From: Pieter De Baets Date: Thu, 11 Jul 2024 05:45:27 -0700 Subject: [PATCH 1/2] Do eager native module init on mqt_native thread Differential Revision: D59465977 --- .../featureflags/ReactNativeFeatureFlags.kt | 8 +++- .../ReactNativeFeatureFlagsCxxAccessor.kt | 12 +++++- .../ReactNativeFeatureFlagsCxxInterop.kt | 4 +- .../ReactNativeFeatureFlagsDefaults.kt | 4 +- .../ReactNativeFeatureFlagsLocalAccessor.kt | 13 ++++++- .../ReactNativeFeatureFlagsProvider.kt | 4 +- .../facebook/react/runtime/ReactHostImpl.java | 7 ++-- .../facebook/react/runtime/ReactInstance.java | 25 ++++++++---- .../JReactNativeFeatureFlagsCxxInterop.cpp | 16 +++++++- .../JReactNativeFeatureFlagsCxxInterop.h | 5 ++- .../featureflags/ReactNativeFeatureFlags.cpp | 6 ++- .../featureflags/ReactNativeFeatureFlags.h | 7 +++- .../ReactNativeFeatureFlagsAccessor.cpp | 38 ++++++++++++++----- .../ReactNativeFeatureFlagsAccessor.h | 6 ++- .../ReactNativeFeatureFlagsDefaults.h | 6 ++- .../ReactNativeFeatureFlagsProvider.h | 3 +- .../NativeReactNativeFeatureFlags.cpp | 7 +++- .../NativeReactNativeFeatureFlags.h | 4 +- .../ReactNativeFeatureFlags.config.js | 5 +++ .../featureflags/ReactNativeFeatureFlags.js | 7 +++- .../specs/NativeReactNativeFeatureFlags.js | 3 +- 21 files changed, 151 insertions(+), 39 deletions(-) diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlags.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlags.kt index 271245d09b67..6762d902fd63 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlags.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlags.kt @@ -4,7 +4,7 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * - * @generated SignedSource<<61e7f1b2647d1e9c9583079b5e7fb166>> + * @generated SignedSource<> */ /** @@ -142,6 +142,12 @@ public object ReactNativeFeatureFlags { @JvmStatic public fun fuseboxEnabledRelease(): Boolean = accessor.fuseboxEnabledRelease() + /** + * Construct modules that requires eager init on the dedicate native modules thread + */ + @JvmStatic + public fun initEagerTurboModulesOnNativeModulesQueueAndroid(): Boolean = accessor.initEagerTurboModulesOnNativeModulesQueueAndroid() + /** * Only enqueue Choreographer calls if there is an ongoing animation, instead of enqueueing every frame. */ diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsCxxAccessor.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsCxxAccessor.kt index 22633b10dbf7..6be47b1bef00 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsCxxAccessor.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsCxxAccessor.kt @@ -4,7 +4,7 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * - * @generated SignedSource<> + * @generated SignedSource<> */ /** @@ -39,6 +39,7 @@ public class ReactNativeFeatureFlagsCxxAccessor : ReactNativeFeatureFlagsAccesso private var forceBatchingMountItemsOnAndroidCache: Boolean? = null private var fuseboxEnabledDebugCache: Boolean? = null private var fuseboxEnabledReleaseCache: Boolean? = null + private var initEagerTurboModulesOnNativeModulesQueueAndroidCache: Boolean? = null private var lazyAnimationCallbacksCache: Boolean? = null private var setAndroidLayoutDirectionCache: Boolean? = null private var useImmediateExecutorInAndroidBridgelessCache: Boolean? = null @@ -220,6 +221,15 @@ public class ReactNativeFeatureFlagsCxxAccessor : ReactNativeFeatureFlagsAccesso return cached } + override fun initEagerTurboModulesOnNativeModulesQueueAndroid(): Boolean { + var cached = initEagerTurboModulesOnNativeModulesQueueAndroidCache + if (cached == null) { + cached = ReactNativeFeatureFlagsCxxInterop.initEagerTurboModulesOnNativeModulesQueueAndroid() + initEagerTurboModulesOnNativeModulesQueueAndroidCache = cached + } + return cached + } + override fun lazyAnimationCallbacks(): Boolean { var cached = lazyAnimationCallbacksCache if (cached == null) { diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsCxxInterop.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsCxxInterop.kt index 2427763c366c..0403a5cd011b 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsCxxInterop.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsCxxInterop.kt @@ -4,7 +4,7 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * - * @generated SignedSource<<97e3090e29645738028f11945d4b759e>> + * @generated SignedSource<<956a18f139bec734e985a48bf3265775>> */ /** @@ -66,6 +66,8 @@ public object ReactNativeFeatureFlagsCxxInterop { @DoNotStrip @JvmStatic public external fun fuseboxEnabledRelease(): Boolean + @DoNotStrip @JvmStatic public external fun initEagerTurboModulesOnNativeModulesQueueAndroid(): Boolean + @DoNotStrip @JvmStatic public external fun lazyAnimationCallbacks(): Boolean @DoNotStrip @JvmStatic public external fun setAndroidLayoutDirection(): Boolean diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsDefaults.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsDefaults.kt index dbce3161165b..116469104b5f 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsDefaults.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsDefaults.kt @@ -4,7 +4,7 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * - * @generated SignedSource<<556729c01ca69ca38b51b96e6b559667>> + * @generated SignedSource<<54256999bd721723c1e7a95669311490>> */ /** @@ -61,6 +61,8 @@ public open class ReactNativeFeatureFlagsDefaults : ReactNativeFeatureFlagsProvi override fun fuseboxEnabledRelease(): Boolean = false + override fun initEagerTurboModulesOnNativeModulesQueueAndroid(): Boolean = false + override fun lazyAnimationCallbacks(): Boolean = false override fun setAndroidLayoutDirection(): Boolean = true diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsLocalAccessor.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsLocalAccessor.kt index c88a19c40f01..510ba531553c 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsLocalAccessor.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsLocalAccessor.kt @@ -4,7 +4,7 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * - * @generated SignedSource<<0a5ce89c1476d6693eaf18adc020ebf4>> + * @generated SignedSource<<97de4303f4682dff2e159390db982010>> */ /** @@ -43,6 +43,7 @@ public class ReactNativeFeatureFlagsLocalAccessor : ReactNativeFeatureFlagsAcces private var forceBatchingMountItemsOnAndroidCache: Boolean? = null private var fuseboxEnabledDebugCache: Boolean? = null private var fuseboxEnabledReleaseCache: Boolean? = null + private var initEagerTurboModulesOnNativeModulesQueueAndroidCache: Boolean? = null private var lazyAnimationCallbacksCache: Boolean? = null private var setAndroidLayoutDirectionCache: Boolean? = null private var useImmediateExecutorInAndroidBridgelessCache: Boolean? = null @@ -243,6 +244,16 @@ public class ReactNativeFeatureFlagsLocalAccessor : ReactNativeFeatureFlagsAcces return cached } + override fun initEagerTurboModulesOnNativeModulesQueueAndroid(): Boolean { + var cached = initEagerTurboModulesOnNativeModulesQueueAndroidCache + if (cached == null) { + cached = currentProvider.initEagerTurboModulesOnNativeModulesQueueAndroid() + accessedFeatureFlags.add("initEagerTurboModulesOnNativeModulesQueueAndroid") + initEagerTurboModulesOnNativeModulesQueueAndroidCache = cached + } + return cached + } + override fun lazyAnimationCallbacks(): Boolean { var cached = lazyAnimationCallbacksCache if (cached == null) { diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsProvider.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsProvider.kt index dd53c27737f3..8e7d8757e1d1 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsProvider.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsProvider.kt @@ -4,7 +4,7 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * - * @generated SignedSource<<16ba68de9b22ddc66208f9ef60f017d9>> + * @generated SignedSource<<60d71718bb4be3be613ffa20ea953324>> */ /** @@ -61,6 +61,8 @@ public interface ReactNativeFeatureFlagsProvider { @DoNotStrip public fun fuseboxEnabledRelease(): Boolean + @DoNotStrip public fun initEagerTurboModulesOnNativeModulesQueueAndroid(): Boolean + @DoNotStrip public fun lazyAnimationCallbacks(): Boolean @DoNotStrip public fun setAndroidLayoutDirection(): Boolean diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/runtime/ReactHostImpl.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/runtime/ReactHostImpl.java index 80c9e0162b97..5f0432e24d47 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/runtime/ReactHostImpl.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/runtime/ReactHostImpl.java @@ -1090,14 +1090,15 @@ private Task getOrCreateReactInstanceTask() { getOrCreateReactHostInspectorTarget()); mReactInstance = instance; - // eagerly initailize turbo modules - instance.initializeEagerTurboModules(); - MemoryPressureListener memoryPressureListener = createMemoryPressureListener(instance); mMemoryPressureListener = memoryPressureListener; mMemoryPressureRouter.addMemoryPressureListener(memoryPressureListener); + // Eagerly initialize turbo modules in parallel with JS bundle execution + // as TurboModuleManager will handle any concurrent access + instance.initializeEagerTurboModules(); + log(method, "Loading JS Bundle"); instance.loadJSBundle(bundleLoader); diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/runtime/ReactInstance.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/runtime/ReactInstance.java index 9897cbb82a46..81e81c85022b 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/runtime/ReactInstance.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/runtime/ReactInstance.java @@ -289,12 +289,21 @@ final class ReactInstance { } void initializeEagerTurboModules() { - Systrace.beginSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE, "initializeEagerTurboModules"); - // Eagerly initialize TurboModules - for (String moduleName : mTurboModuleManager.getEagerInitModuleNames()) { - mTurboModuleManager.getModule(moduleName); + Runnable task = + () -> { + Systrace.beginSection( + Systrace.TRACE_TAG_REACT_JAVA_BRIDGE, "initializeEagerTurboModules"); + // Eagerly initialize TurboModules + for (String moduleName : mTurboModuleManager.getEagerInitModuleNames()) { + mTurboModuleManager.getModule(moduleName); + } + Systrace.endSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE); + }; + if (ReactNativeFeatureFlags.initEagerTurboModulesOnNativeModulesQueueAndroid()) { + mQueueConfiguration.getNativeModulesQueueThread().runOnQueue(task); + } else { + task.run(); } - Systrace.endSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE); } private static synchronized void loadLibraryIfNeeded() { @@ -309,10 +318,10 @@ public ReactQueueConfiguration getReactQueueConfiguration() { } private class ReactJsExceptionHandlerImpl implements ReactJsExceptionHandler { - private final MessageQueueThread mNativemodulesmessagequeuethread; + private final MessageQueueThread mMessageQueueThread; ReactJsExceptionHandlerImpl(MessageQueueThread nativeModulesMessageQueueThread) { - this.mNativemodulesmessagequeuethread = nativeModulesMessageQueueThread; + mMessageQueueThread = nativeModulesMessageQueueThread; } @Override @@ -320,7 +329,7 @@ public void reportJsException(ParsedError error) { JavaOnlyMap data = StackTraceHelper.convertParsedError(error); // Simulate async native module method call - mNativemodulesmessagequeuethread.runOnQueue( + mMessageQueueThread.runOnQueue( () -> { NativeExceptionsManagerSpec exceptionsManager = (NativeExceptionsManagerSpec) diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/featureflags/JReactNativeFeatureFlagsCxxInterop.cpp b/packages/react-native/ReactAndroid/src/main/jni/react/featureflags/JReactNativeFeatureFlagsCxxInterop.cpp index ce5c6ecd6eb4..cfd8a671cbbf 100644 --- a/packages/react-native/ReactAndroid/src/main/jni/react/featureflags/JReactNativeFeatureFlagsCxxInterop.cpp +++ b/packages/react-native/ReactAndroid/src/main/jni/react/featureflags/JReactNativeFeatureFlagsCxxInterop.cpp @@ -4,7 +4,7 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * - * @generated SignedSource<> + * @generated SignedSource<<0a287c69e07600efe13acfcf8bd29f0d>> */ /** @@ -153,6 +153,12 @@ class ReactNativeFeatureFlagsProviderHolder return method(javaProvider_); } + bool initEagerTurboModulesOnNativeModulesQueueAndroid() override { + static const auto method = + getReactNativeFeatureFlagsProviderJavaClass()->getMethod("initEagerTurboModulesOnNativeModulesQueueAndroid"); + return method(javaProvider_); + } + bool lazyAnimationCallbacks() override { static const auto method = getReactNativeFeatureFlagsProviderJavaClass()->getMethod("lazyAnimationCallbacks"); @@ -306,6 +312,11 @@ bool JReactNativeFeatureFlagsCxxInterop::fuseboxEnabledRelease( return ReactNativeFeatureFlags::fuseboxEnabledRelease(); } +bool JReactNativeFeatureFlagsCxxInterop::initEagerTurboModulesOnNativeModulesQueueAndroid( + facebook::jni::alias_ref /*unused*/) { + return ReactNativeFeatureFlags::initEagerTurboModulesOnNativeModulesQueueAndroid(); +} + bool JReactNativeFeatureFlagsCxxInterop::lazyAnimationCallbacks( facebook::jni::alias_ref /*unused*/) { return ReactNativeFeatureFlags::lazyAnimationCallbacks(); @@ -425,6 +436,9 @@ void JReactNativeFeatureFlagsCxxInterop::registerNatives() { makeNativeMethod( "fuseboxEnabledRelease", JReactNativeFeatureFlagsCxxInterop::fuseboxEnabledRelease), + makeNativeMethod( + "initEagerTurboModulesOnNativeModulesQueueAndroid", + JReactNativeFeatureFlagsCxxInterop::initEagerTurboModulesOnNativeModulesQueueAndroid), makeNativeMethod( "lazyAnimationCallbacks", JReactNativeFeatureFlagsCxxInterop::lazyAnimationCallbacks), diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/featureflags/JReactNativeFeatureFlagsCxxInterop.h b/packages/react-native/ReactAndroid/src/main/jni/react/featureflags/JReactNativeFeatureFlagsCxxInterop.h index 931bde586369..8390e2a4da75 100644 --- a/packages/react-native/ReactAndroid/src/main/jni/react/featureflags/JReactNativeFeatureFlagsCxxInterop.h +++ b/packages/react-native/ReactAndroid/src/main/jni/react/featureflags/JReactNativeFeatureFlagsCxxInterop.h @@ -4,7 +4,7 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * - * @generated SignedSource<<57ee61471fc9a9d3fbcf437ec7787b98>> + * @generated SignedSource<<8bc3813da5b0d8ad6a46064b30f48ea8>> */ /** @@ -87,6 +87,9 @@ class JReactNativeFeatureFlagsCxxInterop static bool fuseboxEnabledRelease( facebook::jni::alias_ref); + static bool initEagerTurboModulesOnNativeModulesQueueAndroid( + facebook::jni::alias_ref); + static bool lazyAnimationCallbacks( facebook::jni::alias_ref); diff --git a/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlags.cpp b/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlags.cpp index c673373affb9..2260f4273350 100644 --- a/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlags.cpp +++ b/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlags.cpp @@ -4,7 +4,7 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * - * @generated SignedSource<<1f6e54fe4620719b93b97f0be257bde6>> + * @generated SignedSource<<1bea72c406359550598787ea5da29b17>> */ /** @@ -97,6 +97,10 @@ bool ReactNativeFeatureFlags::fuseboxEnabledRelease() { return getAccessor().fuseboxEnabledRelease(); } +bool ReactNativeFeatureFlags::initEagerTurboModulesOnNativeModulesQueueAndroid() { + return getAccessor().initEagerTurboModulesOnNativeModulesQueueAndroid(); +} + bool ReactNativeFeatureFlags::lazyAnimationCallbacks() { return getAccessor().lazyAnimationCallbacks(); } diff --git a/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlags.h b/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlags.h index c8fafa996156..947ec66fb802 100644 --- a/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlags.h +++ b/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlags.h @@ -4,7 +4,7 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * - * @generated SignedSource<<52ba90db8208aec0eca2de2c9155246a>> + * @generated SignedSource<<627fb0dc656780d46131fad9bf87efa8>> */ /** @@ -132,6 +132,11 @@ class ReactNativeFeatureFlags { */ RN_EXPORT static bool fuseboxEnabledRelease(); + /** + * Construct modules that requires eager init on the dedicate native modules thread + */ + RN_EXPORT static bool initEagerTurboModulesOnNativeModulesQueueAndroid(); + /** * Only enqueue Choreographer calls if there is an ongoing animation, instead of enqueueing every frame. */ diff --git a/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlagsAccessor.cpp b/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlagsAccessor.cpp index 942b33ffe7f4..af1e85974499 100644 --- a/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlagsAccessor.cpp +++ b/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlagsAccessor.cpp @@ -4,7 +4,7 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * - * @generated SignedSource<<0bbd53feabe770d25788128960e3ddcf>> + * @generated SignedSource<<23169873ef7f5c07f5c07f59a7440d47>> */ /** @@ -371,6 +371,24 @@ bool ReactNativeFeatureFlagsAccessor::fuseboxEnabledRelease() { return flagValue.value(); } +bool ReactNativeFeatureFlagsAccessor::initEagerTurboModulesOnNativeModulesQueueAndroid() { + auto flagValue = initEagerTurboModulesOnNativeModulesQueueAndroid_.load(); + + if (!flagValue.has_value()) { + // This block is not exclusive but it is not necessary. + // If multiple threads try to initialize the feature flag, we would only + // be accessing the provider multiple times but the end state of this + // instance and the returned flag value would be the same. + + markFlagAsAccessed(19, "initEagerTurboModulesOnNativeModulesQueueAndroid"); + + flagValue = currentProvider_->initEagerTurboModulesOnNativeModulesQueueAndroid(); + initEagerTurboModulesOnNativeModulesQueueAndroid_ = flagValue; + } + + return flagValue.value(); +} + bool ReactNativeFeatureFlagsAccessor::lazyAnimationCallbacks() { auto flagValue = lazyAnimationCallbacks_.load(); @@ -380,7 +398,7 @@ bool ReactNativeFeatureFlagsAccessor::lazyAnimationCallbacks() { // be accessing the provider multiple times but the end state of this // instance and the returned flag value would be the same. - markFlagAsAccessed(19, "lazyAnimationCallbacks"); + markFlagAsAccessed(20, "lazyAnimationCallbacks"); flagValue = currentProvider_->lazyAnimationCallbacks(); lazyAnimationCallbacks_ = flagValue; @@ -398,7 +416,7 @@ bool ReactNativeFeatureFlagsAccessor::setAndroidLayoutDirection() { // be accessing the provider multiple times but the end state of this // instance and the returned flag value would be the same. - markFlagAsAccessed(20, "setAndroidLayoutDirection"); + markFlagAsAccessed(21, "setAndroidLayoutDirection"); flagValue = currentProvider_->setAndroidLayoutDirection(); setAndroidLayoutDirection_ = flagValue; @@ -416,7 +434,7 @@ bool ReactNativeFeatureFlagsAccessor::useImmediateExecutorInAndroidBridgeless() // be accessing the provider multiple times but the end state of this // instance and the returned flag value would be the same. - markFlagAsAccessed(21, "useImmediateExecutorInAndroidBridgeless"); + markFlagAsAccessed(22, "useImmediateExecutorInAndroidBridgeless"); flagValue = currentProvider_->useImmediateExecutorInAndroidBridgeless(); useImmediateExecutorInAndroidBridgeless_ = flagValue; @@ -434,7 +452,7 @@ bool ReactNativeFeatureFlagsAccessor::useModernRuntimeScheduler() { // be accessing the provider multiple times but the end state of this // instance and the returned flag value would be the same. - markFlagAsAccessed(22, "useModernRuntimeScheduler"); + markFlagAsAccessed(23, "useModernRuntimeScheduler"); flagValue = currentProvider_->useModernRuntimeScheduler(); useModernRuntimeScheduler_ = flagValue; @@ -452,7 +470,7 @@ bool ReactNativeFeatureFlagsAccessor::useNativeViewConfigsInBridgelessMode() { // be accessing the provider multiple times but the end state of this // instance and the returned flag value would be the same. - markFlagAsAccessed(23, "useNativeViewConfigsInBridgelessMode"); + markFlagAsAccessed(24, "useNativeViewConfigsInBridgelessMode"); flagValue = currentProvider_->useNativeViewConfigsInBridgelessMode(); useNativeViewConfigsInBridgelessMode_ = flagValue; @@ -470,7 +488,7 @@ bool ReactNativeFeatureFlagsAccessor::useNewReactImageViewBackgroundDrawing() { // be accessing the provider multiple times but the end state of this // instance and the returned flag value would be the same. - markFlagAsAccessed(24, "useNewReactImageViewBackgroundDrawing"); + markFlagAsAccessed(25, "useNewReactImageViewBackgroundDrawing"); flagValue = currentProvider_->useNewReactImageViewBackgroundDrawing(); useNewReactImageViewBackgroundDrawing_ = flagValue; @@ -488,7 +506,7 @@ bool ReactNativeFeatureFlagsAccessor::useRuntimeShadowNodeReferenceUpdate() { // be accessing the provider multiple times but the end state of this // instance and the returned flag value would be the same. - markFlagAsAccessed(25, "useRuntimeShadowNodeReferenceUpdate"); + markFlagAsAccessed(26, "useRuntimeShadowNodeReferenceUpdate"); flagValue = currentProvider_->useRuntimeShadowNodeReferenceUpdate(); useRuntimeShadowNodeReferenceUpdate_ = flagValue; @@ -506,7 +524,7 @@ bool ReactNativeFeatureFlagsAccessor::useRuntimeShadowNodeReferenceUpdateOnLayou // be accessing the provider multiple times but the end state of this // instance and the returned flag value would be the same. - markFlagAsAccessed(26, "useRuntimeShadowNodeReferenceUpdateOnLayout"); + markFlagAsAccessed(27, "useRuntimeShadowNodeReferenceUpdateOnLayout"); flagValue = currentProvider_->useRuntimeShadowNodeReferenceUpdateOnLayout(); useRuntimeShadowNodeReferenceUpdateOnLayout_ = flagValue; @@ -524,7 +542,7 @@ bool ReactNativeFeatureFlagsAccessor::useStateAlignmentMechanism() { // be accessing the provider multiple times but the end state of this // instance and the returned flag value would be the same. - markFlagAsAccessed(27, "useStateAlignmentMechanism"); + markFlagAsAccessed(28, "useStateAlignmentMechanism"); flagValue = currentProvider_->useStateAlignmentMechanism(); useStateAlignmentMechanism_ = flagValue; diff --git a/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlagsAccessor.h b/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlagsAccessor.h index 77b01371a791..d3ed1db3de2b 100644 --- a/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlagsAccessor.h +++ b/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlagsAccessor.h @@ -4,7 +4,7 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * - * @generated SignedSource<<21ccc4daa6d8c02f29e4bbd4c9a75e49>> + * @generated SignedSource<> */ /** @@ -50,6 +50,7 @@ class ReactNativeFeatureFlagsAccessor { bool forceBatchingMountItemsOnAndroid(); bool fuseboxEnabledDebug(); bool fuseboxEnabledRelease(); + bool initEagerTurboModulesOnNativeModulesQueueAndroid(); bool lazyAnimationCallbacks(); bool setAndroidLayoutDirection(); bool useImmediateExecutorInAndroidBridgeless(); @@ -69,7 +70,7 @@ class ReactNativeFeatureFlagsAccessor { std::unique_ptr currentProvider_; bool wasOverridden_; - std::array, 28> accessedFeatureFlags_; + std::array, 29> accessedFeatureFlags_; std::atomic> commonTestFlag_; std::atomic> allowCollapsableChildren_; @@ -90,6 +91,7 @@ class ReactNativeFeatureFlagsAccessor { std::atomic> forceBatchingMountItemsOnAndroid_; std::atomic> fuseboxEnabledDebug_; std::atomic> fuseboxEnabledRelease_; + std::atomic> initEagerTurboModulesOnNativeModulesQueueAndroid_; std::atomic> lazyAnimationCallbacks_; std::atomic> setAndroidLayoutDirection_; std::atomic> useImmediateExecutorInAndroidBridgeless_; diff --git a/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlagsDefaults.h b/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlagsDefaults.h index 93c8cc64a9b5..6b5e88cc0775 100644 --- a/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlagsDefaults.h +++ b/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlagsDefaults.h @@ -4,7 +4,7 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * - * @generated SignedSource<<1f576108717e5bd1b5ca8de0aaae5b0e>> + * @generated SignedSource<<031622096a93ca9da5e278dab84324b9>> */ /** @@ -103,6 +103,10 @@ class ReactNativeFeatureFlagsDefaults : public ReactNativeFeatureFlagsProvider { return false; } + bool initEagerTurboModulesOnNativeModulesQueueAndroid() override { + return false; + } + bool lazyAnimationCallbacks() override { return false; } diff --git a/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlagsProvider.h b/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlagsProvider.h index 4648b0b5c43e..6b6c1c2cffb9 100644 --- a/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlagsProvider.h +++ b/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlagsProvider.h @@ -4,7 +4,7 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * - * @generated SignedSource<<760ad06ff565102e3ba42d671f45a470>> + * @generated SignedSource<<55e4299def48d95ea40cd988e70fd608>> */ /** @@ -44,6 +44,7 @@ class ReactNativeFeatureFlagsProvider { virtual bool forceBatchingMountItemsOnAndroid() = 0; virtual bool fuseboxEnabledDebug() = 0; virtual bool fuseboxEnabledRelease() = 0; + virtual bool initEagerTurboModulesOnNativeModulesQueueAndroid() = 0; virtual bool lazyAnimationCallbacks() = 0; virtual bool setAndroidLayoutDirection() = 0; virtual bool useImmediateExecutorInAndroidBridgeless() = 0; diff --git a/packages/react-native/ReactCommon/react/nativemodule/featureflags/NativeReactNativeFeatureFlags.cpp b/packages/react-native/ReactCommon/react/nativemodule/featureflags/NativeReactNativeFeatureFlags.cpp index fcefaf0f4944..944e9ef00a1a 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/featureflags/NativeReactNativeFeatureFlags.cpp +++ b/packages/react-native/ReactCommon/react/nativemodule/featureflags/NativeReactNativeFeatureFlags.cpp @@ -4,7 +4,7 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * - * @generated SignedSource<<1c3b8179bf67dd9d44c8d2e29bf752b6>> + * @generated SignedSource<<345d27de837a248fbfd5e299754097f0>> */ /** @@ -132,6 +132,11 @@ bool NativeReactNativeFeatureFlags::fuseboxEnabledRelease( return ReactNativeFeatureFlags::fuseboxEnabledRelease(); } +bool NativeReactNativeFeatureFlags::initEagerTurboModulesOnNativeModulesQueueAndroid( + jsi::Runtime& /*runtime*/) { + return ReactNativeFeatureFlags::initEagerTurboModulesOnNativeModulesQueueAndroid(); +} + bool NativeReactNativeFeatureFlags::lazyAnimationCallbacks( jsi::Runtime& /*runtime*/) { return ReactNativeFeatureFlags::lazyAnimationCallbacks(); diff --git a/packages/react-native/ReactCommon/react/nativemodule/featureflags/NativeReactNativeFeatureFlags.h b/packages/react-native/ReactCommon/react/nativemodule/featureflags/NativeReactNativeFeatureFlags.h index b5df7b0e290d..ea92618e04ca 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/featureflags/NativeReactNativeFeatureFlags.h +++ b/packages/react-native/ReactCommon/react/nativemodule/featureflags/NativeReactNativeFeatureFlags.h @@ -4,7 +4,7 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * - * @generated SignedSource<> + * @generated SignedSource<<4121c9b419a41e84ec2c1066cb2707e2>> */ /** @@ -73,6 +73,8 @@ class NativeReactNativeFeatureFlags bool fuseboxEnabledRelease(jsi::Runtime& runtime); + bool initEagerTurboModulesOnNativeModulesQueueAndroid(jsi::Runtime& runtime); + bool lazyAnimationCallbacks(jsi::Runtime& runtime); bool setAndroidLayoutDirection(jsi::Runtime& runtime); diff --git a/packages/react-native/scripts/featureflags/ReactNativeFeatureFlags.config.js b/packages/react-native/scripts/featureflags/ReactNativeFeatureFlags.config.js index 662fea1331d7..da677636559f 100644 --- a/packages/react-native/scripts/featureflags/ReactNativeFeatureFlags.config.js +++ b/packages/react-native/scripts/featureflags/ReactNativeFeatureFlags.config.js @@ -128,6 +128,11 @@ const definitions: FeatureFlagDefinitions = { description: 'Flag determining if the React Native DevTools (Fusebox) CDP backend should be enabled in release builds. This flag is global and should not be changed across React Host lifetimes.', }, + initEagerTurboModulesOnNativeModulesQueueAndroid: { + defaultValue: false, + description: + 'Construct modules that requires eager init on the dedicate native modules thread', + }, lazyAnimationCallbacks: { defaultValue: false, description: diff --git a/packages/react-native/src/private/featureflags/ReactNativeFeatureFlags.js b/packages/react-native/src/private/featureflags/ReactNativeFeatureFlags.js index 606bbae4a933..3fd57ddfddb7 100644 --- a/packages/react-native/src/private/featureflags/ReactNativeFeatureFlags.js +++ b/packages/react-native/src/private/featureflags/ReactNativeFeatureFlags.js @@ -4,7 +4,7 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * - * @generated SignedSource<<3fce773b88616a376b8ab6dbd5649813>> + * @generated SignedSource<<3ee4e03eae552b81423262351ee2a494>> * @flow strict-local */ @@ -60,6 +60,7 @@ export type ReactNativeFeatureFlags = { forceBatchingMountItemsOnAndroid: Getter, fuseboxEnabledDebug: Getter, fuseboxEnabledRelease: Getter, + initEagerTurboModulesOnNativeModulesQueueAndroid: Getter, lazyAnimationCallbacks: Getter, setAndroidLayoutDirection: Getter, useImmediateExecutorInAndroidBridgeless: Getter, @@ -192,6 +193,10 @@ export const fuseboxEnabledDebug: Getter = createNativeFlagGetter('fuse * Flag determining if the React Native DevTools (Fusebox) CDP backend should be enabled in release builds. This flag is global and should not be changed across React Host lifetimes. */ export const fuseboxEnabledRelease: Getter = createNativeFlagGetter('fuseboxEnabledRelease', false); +/** + * Construct modules that requires eager init on the dedicate native modules thread + */ +export const initEagerTurboModulesOnNativeModulesQueueAndroid: Getter = createNativeFlagGetter('initEagerTurboModulesOnNativeModulesQueueAndroid', false); /** * Only enqueue Choreographer calls if there is an ongoing animation, instead of enqueueing every frame. */ diff --git a/packages/react-native/src/private/featureflags/specs/NativeReactNativeFeatureFlags.js b/packages/react-native/src/private/featureflags/specs/NativeReactNativeFeatureFlags.js index 02b380e83950..134f984574fe 100644 --- a/packages/react-native/src/private/featureflags/specs/NativeReactNativeFeatureFlags.js +++ b/packages/react-native/src/private/featureflags/specs/NativeReactNativeFeatureFlags.js @@ -4,7 +4,7 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * - * @generated SignedSource<<4f001bdc6d2161ead95635785f387abe>> + * @generated SignedSource<<44f6fe8269f1b8d2039668e67bca9839>> * @flow strict-local */ @@ -42,6 +42,7 @@ export interface Spec extends TurboModule { +forceBatchingMountItemsOnAndroid?: () => boolean; +fuseboxEnabledDebug?: () => boolean; +fuseboxEnabledRelease?: () => boolean; + +initEagerTurboModulesOnNativeModulesQueueAndroid?: () => boolean; +lazyAnimationCallbacks?: () => boolean; +setAndroidLayoutDirection?: () => boolean; +useImmediateExecutorInAndroidBridgeless?: () => boolean; From 59cce56051db3566568ede066a980ac7cc85f99f Mon Sep 17 00:00:00 2001 From: Pieter De Baets Date: Thu, 11 Jul 2024 06:08:56 -0700 Subject: [PATCH 2/2] Cleanup enableRemoveDeleteTreeInstruction experiment Summary: We ran various experiments with this flag, and this does not turn out to provide any significant benefits at this point to make it worth the complexity and platform divergence it introduced. Changelog: [Internal] Reviewed By: cortinico Differential Revision: D58355433 --- .../ReactAndroid/api/ReactAndroid.api | 2 - .../react/config/ReactFeatureFlags.java | 5 - .../mounting/SurfaceMountingManager.java | 320 ------------------ .../mountitems/IntBufferBatchMountItem.java | 11 - .../featureflags/ReactNativeFeatureFlags.kt | 8 +- .../ReactNativeFeatureFlagsCxxAccessor.kt | 12 +- .../ReactNativeFeatureFlagsCxxInterop.kt | 4 +- .../ReactNativeFeatureFlagsDefaults.kt | 4 +- .../ReactNativeFeatureFlagsLocalAccessor.kt | 13 +- .../ReactNativeFeatureFlagsProvider.kt | 4 +- .../src/main/jni/react/fabric/Binding.cpp | 4 - .../react/fabric/FabricMountingManager.cpp | 8 +- .../JReactNativeFeatureFlagsCxxInterop.cpp | 16 +- .../JReactNativeFeatureFlagsCxxInterop.h | 5 +- .../featureflags/ReactNativeFeatureFlags.cpp | 6 +- .../featureflags/ReactNativeFeatureFlags.h | 7 +- .../ReactNativeFeatureFlagsAccessor.cpp | 46 +-- .../ReactNativeFeatureFlagsAccessor.h | 6 +- .../ReactNativeFeatureFlagsDefaults.h | 6 +- .../ReactNativeFeatureFlagsProvider.h | 3 +- .../NativeReactNativeFeatureFlags.cpp | 7 +- .../NativeReactNativeFeatureFlags.h | 4 +- .../renderer/mounting/Differentiator.cpp | 39 +-- .../renderer/mounting/ShadowViewMutation.cpp | 21 +- .../renderer/mounting/ShadowViewMutation.h | 20 +- .../ReactNativeFeatureFlags.config.js | 5 - .../featureflags/ReactNativeFeatureFlags.js | 7 +- .../specs/NativeReactNativeFeatureFlags.js | 3 +- 28 files changed, 47 insertions(+), 549 deletions(-) diff --git a/packages/react-native/ReactAndroid/api/ReactAndroid.api b/packages/react-native/ReactAndroid/api/ReactAndroid.api index 055aade54dc8..c12785d1a3bb 100644 --- a/packages/react-native/ReactAndroid/api/ReactAndroid.api +++ b/packages/react-native/ReactAndroid/api/ReactAndroid.api @@ -1972,7 +1972,6 @@ public class com/facebook/react/config/ReactFeatureFlags { public static field enableFabricLogs Z public static field enableFabricRenderer Z public static field enableFabricRendererExclusively Z - public static field enableRemoveDeleteTreeInstruction Z public static field enableViewRecycling Z public static field excludeYogaFromRawProps Z public static field rejectTurboModulePromiseOnNativeError Z @@ -2804,7 +2803,6 @@ public class com/facebook/react/fabric/mounting/SurfaceMountingManager { public fun printSurfaceState ()V public fun receiveCommand (IILcom/facebook/react/bridge/ReadableArray;)V public fun receiveCommand (ILjava/lang/String;Lcom/facebook/react/bridge/ReadableArray;)V - public fun removeDeleteTreeAt (III)V public fun removeViewAt (III)V public fun scheduleMountItemOnViewAttach (Lcom/facebook/react/fabric/mounting/mountitems/MountItem;)V public fun sendAccessibilityEvent (II)V 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 e57d5aa1a555..4f38fe0cc5a1 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 @@ -76,11 +76,6 @@ public class ReactFeatureFlags { */ public static boolean enableCppPropsIteratorSetter = false; - /** - * Allow Differentiator.cpp and FabricMountingManager.cpp to generate a RemoveDeleteTree mega-op. - */ - public static boolean enableRemoveDeleteTreeInstruction = false; - /** When enabled, rawProps in Props will not include Yoga specific props. */ public static boolean excludeYogaFromRawProps = false; diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/SurfaceMountingManager.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/SurfaceMountingManager.java index 813f9516e19c..9de211ea6b0a 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/SurfaceMountingManager.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/SurfaceMountingManager.java @@ -21,7 +21,6 @@ import com.facebook.common.logging.FLog; import com.facebook.infer.annotation.Assertions; import com.facebook.infer.annotation.ThreadConfined; -import com.facebook.react.bridge.ReactContext; import com.facebook.react.bridge.ReactSoftExceptionLogger; import com.facebook.react.bridge.ReadableArray; import com.facebook.react.bridge.ReadableMap; @@ -31,12 +30,10 @@ import com.facebook.react.bridge.WritableMap; import com.facebook.react.common.build.ReactBuildConfig; import com.facebook.react.config.ReactFeatureFlags; -import com.facebook.react.fabric.GuardedFrameCallback; import com.facebook.react.fabric.events.EventEmitterWrapper; import com.facebook.react.fabric.mounting.MountingManager.MountItemExecutor; import com.facebook.react.fabric.mounting.mountitems.MountItem; import com.facebook.react.internal.featureflags.ReactNativeFeatureFlags; -import com.facebook.react.modules.core.ReactChoreographer; import com.facebook.react.touch.JSResponderHandler; import com.facebook.react.uimanager.IViewGroupManager; import com.facebook.react.uimanager.IllegalViewOperationException; @@ -56,7 +53,6 @@ import java.util.Map; import java.util.Queue; import java.util.Set; -import java.util.Stack; import java.util.concurrent.ConcurrentHashMap; public class SurfaceMountingManager { @@ -78,18 +74,9 @@ public class SurfaceMountingManager { private RootViewManager mRootViewManager; private MountItemExecutor mMountItemExecutor; - // Stack of deferred-removal tags for Views that can be - // removed asynchronously. Guaranteed to be disconnected - // from the viewport and these tags will not be reused in the future. - @ThreadConfined(UI) - private final Stack mReactTagsToRemove = new Stack<>(); - @ThreadConfined(UI) private final Set mErroneouslyReaddedReactTags = new HashSet<>(); - @ThreadConfined(UI) - private @Nullable RemoveDeleteTreeUIFrameCallback mRemoveDeleteTreeUIFrameCallback; - // This is null *until* StopSurface is called. private SparseArrayCompat mTagSetForStoppedSurface; @@ -324,9 +311,6 @@ public void stopSurface() { mRootViewManager = null; mMountItemExecutor = null; mThemedReactContext = null; - if (ReactNativeFeatureFlags.fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak()) { - mRemoveDeleteTreeUIFrameCallback = null; - } mOnViewAttachMountItems.clear(); if (ReactFeatureFlags.enableViewRecycling) { @@ -619,182 +603,6 @@ public void run() { } } - @UiThread - public void removeDeleteTreeAt(final int tag, final int parentTag, int index) { - if (isStopped()) { - return; - } - - UiThreadUtil.assertOnUiThread(); - ViewState parentViewState = getNullableViewState(parentTag); - - // TODO: throw exception here? - if (parentViewState == null) { - ReactSoftExceptionLogger.logSoftException( - MountingManager.TAG, - new IllegalStateException( - "Unable to find viewState for tag: [" + parentTag + "] for removeDeleteTreeAt")); - return; - } - - if (!(parentViewState.mView instanceof ViewGroup)) { - String message = - "Unable to remove+delete a view from a view that is not a ViewGroup. ParentTag: " - + parentTag - + " - Tag: " - + tag - + " - Index: " - + index; - FLog.e(TAG, message); - throw new IllegalStateException(message); - } - - final ViewGroup parentView = (ViewGroup) parentViewState.mView; - - if (parentView == null) { - throw new IllegalStateException("Unable to find view for tag [" + parentTag + "]"); - } - - if (SHOW_CHANGED_VIEW_HIERARCHIES) { - // Display children before deleting any - FLog.e( - TAG, - "removeDeleteTreeAt: [" + tag + "] -> [" + parentTag + "] idx: " + index + " BEFORE"); - logViewHierarchy(parentView, false); - } - - IViewGroupManager viewGroupManager = getViewGroupManager(parentViewState); - - // Verify that the view we're about to remove has the same tag we expect - View view = viewGroupManager.getChildAt(parentView, index); - int actualTag = (view != null ? view.getId() : -1); - if (actualTag != tag) { - int tagActualIndex = -1; - int parentChildrenCount = parentView.getChildCount(); - for (int i = 0; i < parentChildrenCount; i++) { - if (parentView.getChildAt(i).getId() == tag) { - tagActualIndex = i; - break; - } - } - - // TODO T74425739: previously, we did not do this check and `removeViewAt` would be executed - // below, sometimes crashing there. *However*, interestingly enough, `removeViewAt` would not - // complain if you removed views from an already-empty parent. This seems necessary currently - // for certain ViewManagers that remove their own children - like BottomSheet? - // This workaround seems not-great, but for now, we just return here for - // backwards-compatibility. Essentially, if a view has already been removed from the - // hierarchy, we treat it as a noop. - if (tagActualIndex == -1) { - FLog.e( - TAG, - "removeDeleteTreeAt: [" - + tag - + "] -> [" - + parentTag - + "] @" - + index - + ": view already removed from parent! Children in parent: " - + parentChildrenCount); - return; - } - - // Here we are guaranteed that the view is still in the View hierarchy, just - // at a different index. In debug mode we'll crash here; in production, we'll remove - // the child from the parent and move on. - // This is an issue that is safely recoverable 95% of the time. If this allows corruption - // of the view hierarchy and causes bugs or a crash after this point, there will be logs - // indicating that this happened. - // This is likely *only* necessary because of Fabric's LayoutAnimations implementation. - // If we can fix the bug there, or remove the need for LayoutAnimation index adjustment - // entirely, we can just throw this exception without regression user experience. - logViewHierarchy(parentView, true); - ReactSoftExceptionLogger.logSoftException( - TAG, - new IllegalStateException( - "Tried to remove+delete view [" - + tag - + "] of parent [" - + parentTag - + "] at index " - + index - + ", but got view tag " - + actualTag - + " - actual index of view: " - + tagActualIndex)); - index = tagActualIndex; - } - - try { - viewGroupManager.removeViewAt(parentView, index); - } catch (RuntimeException e) { - // Note: `getChildCount` may not always be accurate! - // We don't currently have a good explanation other than, in situations where you - // would empirically expect to see childCount > 0, the childCount is reported as 0. - // This is likely due to a ViewManager overriding getChildCount or some other methods - // in a way that is strictly incorrect, but potentially only visible here. - // The failure mode is actually that in `removeViewAt`, a NullPointerException is - // thrown when we try to perform an operation on a View that doesn't exist, and - // is therefore null. - // We try to add some extra diagnostics here, but we always try to remove the View - // from the hierarchy first because detecting by looking at childCount will not work. - // - // Note that the lesson here is that `getChildCount` is not /required/ to adhere to - // any invariants. If you add 9 children to a parent, the `getChildCount` of the parent - // may not be equal to 9. This apparently causes no issues with Android and is common - // enough that we shouldn't try to change this invariant, without a lot of thought. - int childCount = viewGroupManager.getChildCount(parentView); - - logViewHierarchy(parentView, true); - - throw new IllegalStateException( - "Cannot remove child at index " - + index - + " from parent ViewGroup [" - + parentView.getId() - + "], only " - + childCount - + " children in parent. Warning: childCount may be incorrect!", - e); - } - - // Display children after deleting any - if (SHOW_CHANGED_VIEW_HIERARCHIES) { - final int finalIndex = index; - UiThreadUtil.runOnUiThread( - new Runnable() { - @Override - public void run() { - FLog.e( - TAG, - "removeViewAt: [" - + tag - + "] -> [" - + parentTag - + "] idx: " - + finalIndex - + " AFTER"); - logViewHierarchy(parentView, false); - } - }); - } - - // The View has been removed from the View hierarchy; now it - // and all of its children, if any, need to be deleted, recursively. - // Schedule the Runnable first, to detect if we need to schedule a Runnable at all. - // Since this current function and the Runnable both run on the UI thread, there is - // no race condition here. - if (mReactTagsToRemove.empty()) { - if (mRemoveDeleteTreeUIFrameCallback == null) { - mRemoveDeleteTreeUIFrameCallback = new RemoveDeleteTreeUIFrameCallback(mThemedReactContext); - } - ReactChoreographer.getInstance() - .postFrameCallback( - ReactChoreographer.CallbackType.IDLE_EVENT, mRemoveDeleteTreeUIFrameCallback); - } - mReactTagsToRemove.push(tag); - } - @UiThread public void createView( @NonNull String componentName, @@ -1427,132 +1235,4 @@ public void dispatch(EventEmitterWrapper eventEmitter) { } } } - - private class RemoveDeleteTreeUIFrameCallback extends GuardedFrameCallback { - private static final long FRAME_TIME_MS = 16; - private static final long MAX_TIME_IN_FRAME = 9; - - private RemoveDeleteTreeUIFrameCallback(@NonNull ReactContext reactContext) { - super(reactContext); - } - - /** - * Detect if we still have processing time left in this frame. Technically, it should be fine - * for this to take up to 15ms since it executes after all other important UI work. - */ - private boolean haveExceededNonBatchedFrameTime(long frameTimeNanos) { - long timeLeftInFrame = FRAME_TIME_MS - ((System.nanoTime() - frameTimeNanos) / 1000000); - return timeLeftInFrame < MAX_TIME_IN_FRAME; - } - - @Override - @UiThread - @ThreadConfined(UI) - public void doFrameGuarded(long frameTimeNanos) { - int deletedViews = 0; - Stack localChildren = new Stack<>(); - try { - while (!mReactTagsToRemove.empty()) { - int reactTag = mReactTagsToRemove.pop(); - deletedViews++; - - // This is "impossible". See comments above. - if (mErroneouslyReaddedReactTags.contains(reactTag)) { - ReactSoftExceptionLogger.logSoftException( - TAG, - new IllegalViewOperationException( - "RemoveDeleteTree recursively tried to remove a React View that was actually" - + " reused. This indicates a bug in the Differ. [" - + reactTag - + "]")); - continue; - } - - localChildren.clear(); - - ViewState thisViewState = getNullableViewState(reactTag); - if (thisViewState != null) { - View thisView = thisViewState.mView; - ViewManager thisViewManager = thisViewState.mViewManager; - if (thisViewManager instanceof IViewGroupManager) { - IViewGroupManager viewManager = (IViewGroupManager) thisViewManager; - - // Children are managed by React Native if both of the following are true: - // 1) There are 1 or more children of this View, which must be a ViewGroup - // 2) Those children are managed by RN (this is not the case for certain native - // components, like embedded Litho hierarchies) - boolean childrenAreManaged = false; - - // For reasons documented elsewhere in this class, getChildCount is not - // necessarily reliable, and so we rely instead on requesting children directly. - View nextChild = null; - int numChildren = 0; - while ((nextChild = viewManager.getChildAt(thisView, numChildren)) != null) { - int childId = nextChild.getId(); - childrenAreManaged = childrenAreManaged || getNullableViewState(childId) != null; - localChildren.push(nextChild.getId()); - numChildren++; - } - // Removing all at once is more efficient than removing one-by-one - // If the children are not managed by RN, we simply drop the entire - // subtree instead of recursing further. - if (childrenAreManaged) { - try { - // This can happen if the removeAllViews method is overridden to throw, - // which it is explicitly in some cases (for example embedded Litho views, - // but there could be other cases). In those cases, we want to fail silently - // and then assume the subtree is /not/ managed by React Native. - // In this case short-lived memory-leaks could occur if we aren't clearing - // out the ViewState map properly; but the risk should be small. - // In debug mode, the SoftException will cause a crash. In production it - // will not. This should give good visibility into whether or not this is - // a problem without causing user-facing errors. - viewManager.removeAllViews(thisView); - } catch (RuntimeException e) { - childrenAreManaged = false; - ReactSoftExceptionLogger.logSoftException(TAG, e); - } - } - - if (childrenAreManaged) { - // Push tags onto the stack so we process all children - mReactTagsToRemove.addAll(localChildren); - } - } - - // Immediately remove tag and notify listeners. - // Note that this causes RemoveDeleteTree to call onViewStateDeleted - // in a top-down matter (parents first) vs a bottom-up matter (leaf nodes first). - // Hopefully this doesn't matter but you should ensure that any custom - // onViewStateDeleted logic is resilient to both semantics. - // In the initial version of RemoveDeleteTree we attempted to maintain - // the bottom-up event listener behavior but this causes additional - // memory pressure as well as complexity. - mTagToViewState.remove(reactTag); - onViewStateDeleted(thisViewState); - - // Circuit breaker: after processing every N tags, check that we haven't - // exceeded the max allowed time. Since we don't know what other work needs - // to happen on the UI thread during this frame, and since this works tends to be - // low-priority, we only give this function a fraction of a frame to run. - if (deletedViews % 20 == 0 && haveExceededNonBatchedFrameTime(frameTimeNanos)) { - break; - } - } - } - } finally { - if (!mReactTagsToRemove.empty()) { - ReactChoreographer.getInstance() - .postFrameCallback(ReactChoreographer.CallbackType.IDLE_EVENT, this); - } else { - // If there are no more tags to process, then clear the "reused" - // tag set. Since the RemoveDeleteTree runner executes /after/ all - // other mounting instructions have been executed, all in-band Remove - // instructions have already had a chance to execute here. - mErroneouslyReaddedReactTags.clear(); - mReactTagsToRemove.clear(); - } - } - } - } } diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/IntBufferBatchMountItem.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/IntBufferBatchMountItem.java index 2db50ef134d0..52ea0ed8a992 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/IntBufferBatchMountItem.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/IntBufferBatchMountItem.java @@ -50,7 +50,6 @@ final class IntBufferBatchMountItem implements BatchMountItem { static final int INSTRUCTION_UPDATE_EVENT_EMITTER = 256; static final int INSTRUCTION_UPDATE_PADDING = 512; static final int INSTRUCTION_UPDATE_OVERFLOW_INSET = 1024; - static final int INSTRUCTION_REMOVE_DELETE_TREE = 2048; private final int mSurfaceId; private final int mCommitNumber; @@ -117,9 +116,6 @@ public void execute(MountingManager mountingManager) { surfaceMountingManager.addViewAt(parentTag, tag, mIntBuffer[i++]); } else if (type == INSTRUCTION_REMOVE) { surfaceMountingManager.removeViewAt(mIntBuffer[i++], mIntBuffer[i++], mIntBuffer[i++]); - } else if (type == INSTRUCTION_REMOVE_DELETE_TREE) { - surfaceMountingManager.removeDeleteTreeAt( - mIntBuffer[i++], mIntBuffer[i++], mIntBuffer[i++]); } else if (type == INSTRUCTION_UPDATE_PROPS) { surfaceMountingManager.updateProps(mIntBuffer[i++], (ReadableMap) mObjBuffer[j++]); } else if (type == INSTRUCTION_UPDATE_STATE) { @@ -207,11 +203,6 @@ public String toString() { s.append( String.format( "REMOVE [%d]->[%d] @%d\n", mIntBuffer[i++], mIntBuffer[i++], mIntBuffer[i++])); - } else if (type == INSTRUCTION_REMOVE_DELETE_TREE) { - s.append( - String.format( - "REMOVE+DELETE TREE [%d]->[%d] @%d\n", - mIntBuffer[i++], mIntBuffer[i++], mIntBuffer[i++])); } else if (type == INSTRUCTION_UPDATE_PROPS) { Object props = mObjBuffer[j++]; String propsString = @@ -298,8 +289,6 @@ private static String nameForInstructionString(int type) { return "INSERT"; } else if (type == INSTRUCTION_REMOVE) { return "REMOVE"; - } else if (type == INSTRUCTION_REMOVE_DELETE_TREE) { - return "REMOVE_DELETE_TREE"; } else if (type == INSTRUCTION_UPDATE_PROPS) { return "UPDATE_PROPS"; } else if (type == INSTRUCTION_UPDATE_STATE) { diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlags.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlags.kt index 6762d902fd63..60f790fcffc3 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlags.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlags.kt @@ -4,7 +4,7 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * - * @generated SignedSource<> + * @generated SignedSource<<9f31fa21b0dd908ec53d1586c0bd271d>> */ /** @@ -118,12 +118,6 @@ public object ReactNativeFeatureFlags { @JvmStatic public fun fixMissedFabricStateUpdatesOnAndroid(): Boolean = accessor.fixMissedFabricStateUpdatesOnAndroid() - /** - * Fixes a leak in SurfaceMountingManager.mRemoveDeleteTreeUIFrameCallback - */ - @JvmStatic - public fun fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak(): Boolean = accessor.fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak() - /** * Forces the mounting layer on Android to always batch mount items instead of dispatching them immediately. This might fix some crashes related to synchronous state updates, where some views dispatch state updates during mount. */ diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsCxxAccessor.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsCxxAccessor.kt index 6be47b1bef00..bc2c35ed2a76 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsCxxAccessor.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsCxxAccessor.kt @@ -4,7 +4,7 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * - * @generated SignedSource<> + * @generated SignedSource<<718c08632274058c4ece4c15ed06fbfe>> */ /** @@ -35,7 +35,6 @@ public class ReactNativeFeatureFlagsCxxAccessor : ReactNativeFeatureFlagsAccesso private var fixIncorrectScrollViewStateUpdateOnAndroidCache: Boolean? = null private var fixMappingOfEventPrioritiesBetweenFabricAndReactCache: Boolean? = null private var fixMissedFabricStateUpdatesOnAndroidCache: Boolean? = null - private var fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeakCache: Boolean? = null private var forceBatchingMountItemsOnAndroidCache: Boolean? = null private var fuseboxEnabledDebugCache: Boolean? = null private var fuseboxEnabledReleaseCache: Boolean? = null @@ -185,15 +184,6 @@ public class ReactNativeFeatureFlagsCxxAccessor : ReactNativeFeatureFlagsAccesso return cached } - override fun fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak(): Boolean { - var cached = fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeakCache - if (cached == null) { - cached = ReactNativeFeatureFlagsCxxInterop.fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak() - fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeakCache = cached - } - return cached - } - override fun forceBatchingMountItemsOnAndroid(): Boolean { var cached = forceBatchingMountItemsOnAndroidCache if (cached == null) { diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsCxxInterop.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsCxxInterop.kt index 0403a5cd011b..a67e0e40d2d6 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsCxxInterop.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsCxxInterop.kt @@ -4,7 +4,7 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * - * @generated SignedSource<<956a18f139bec734e985a48bf3265775>> + * @generated SignedSource<<9af9576364f2ec8e978604eaa1c15ee8>> */ /** @@ -58,8 +58,6 @@ public object ReactNativeFeatureFlagsCxxInterop { @DoNotStrip @JvmStatic public external fun fixMissedFabricStateUpdatesOnAndroid(): Boolean - @DoNotStrip @JvmStatic public external fun fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak(): Boolean - @DoNotStrip @JvmStatic public external fun forceBatchingMountItemsOnAndroid(): Boolean @DoNotStrip @JvmStatic public external fun fuseboxEnabledDebug(): Boolean diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsDefaults.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsDefaults.kt index 116469104b5f..e2d120fad0ad 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsDefaults.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsDefaults.kt @@ -4,7 +4,7 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * - * @generated SignedSource<<54256999bd721723c1e7a95669311490>> + * @generated SignedSource<> */ /** @@ -53,8 +53,6 @@ public open class ReactNativeFeatureFlagsDefaults : ReactNativeFeatureFlagsProvi override fun fixMissedFabricStateUpdatesOnAndroid(): Boolean = false - override fun fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak(): Boolean = false - override fun forceBatchingMountItemsOnAndroid(): Boolean = false override fun fuseboxEnabledDebug(): Boolean = false diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsLocalAccessor.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsLocalAccessor.kt index 510ba531553c..0c12b461c0a1 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsLocalAccessor.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsLocalAccessor.kt @@ -4,7 +4,7 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * - * @generated SignedSource<<97de4303f4682dff2e159390db982010>> + * @generated SignedSource<<63cb1200d25489b5cc9fcfa0061eccd0>> */ /** @@ -39,7 +39,6 @@ public class ReactNativeFeatureFlagsLocalAccessor : ReactNativeFeatureFlagsAcces private var fixIncorrectScrollViewStateUpdateOnAndroidCache: Boolean? = null private var fixMappingOfEventPrioritiesBetweenFabricAndReactCache: Boolean? = null private var fixMissedFabricStateUpdatesOnAndroidCache: Boolean? = null - private var fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeakCache: Boolean? = null private var forceBatchingMountItemsOnAndroidCache: Boolean? = null private var fuseboxEnabledDebugCache: Boolean? = null private var fuseboxEnabledReleaseCache: Boolean? = null @@ -204,16 +203,6 @@ public class ReactNativeFeatureFlagsLocalAccessor : ReactNativeFeatureFlagsAcces return cached } - override fun fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak(): Boolean { - var cached = fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeakCache - if (cached == null) { - cached = currentProvider.fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak() - accessedFeatureFlags.add("fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak") - fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeakCache = cached - } - return cached - } - override fun forceBatchingMountItemsOnAndroid(): Boolean { var cached = forceBatchingMountItemsOnAndroidCache if (cached == null) { diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsProvider.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsProvider.kt index 8e7d8757e1d1..b5efd53ad37e 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsProvider.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsProvider.kt @@ -4,7 +4,7 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * - * @generated SignedSource<<60d71718bb4be3be613ffa20ea953324>> + * @generated SignedSource<<588266c6ec144651b7fdc84b77b490e7>> */ /** @@ -53,8 +53,6 @@ public interface ReactNativeFeatureFlagsProvider { @DoNotStrip public fun fixMissedFabricStateUpdatesOnAndroid(): Boolean - @DoNotStrip public fun fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak(): Boolean - @DoNotStrip public fun forceBatchingMountItemsOnAndroid(): Boolean @DoNotStrip public fun fuseboxEnabledDebug(): Boolean 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 d4d5c4cd289a..fb076f1ca20a 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 @@ -400,10 +400,6 @@ void Binding::installFabricUIManager( CoreFeatures::excludeYogaFromRawProps = getFeatureFlagValue("excludeYogaFromRawProps"); - // RemoveDelete mega-op - ShadowViewMutation::PlatformSupportsRemoveDeleteTreeInstruction = - getFeatureFlagValue("enableRemoveDeleteTreeInstruction"); - auto toolbox = SchedulerToolbox{}; toolbox.contextContainer = contextContainer; toolbox.componentRegistryFactory = componentsRegistry->buildRegistryFunction; diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/fabric/FabricMountingManager.cpp b/packages/react-native/ReactAndroid/src/main/jni/react/fabric/FabricMountingManager.cpp index ea6ed97c4b3d..7a5dca8a8391 100644 --- a/packages/react-native/ReactAndroid/src/main/jni/react/fabric/FabricMountingManager.cpp +++ b/packages/react-native/ReactAndroid/src/main/jni/react/fabric/FabricMountingManager.cpp @@ -287,7 +287,7 @@ void FabricMountingManager::executeMount( break; } case ShadowViewMutation::Remove: { - if (!isVirtual && !mutation.isRedundantOperation) { + if (!isVirtual) { cppCommonMountItems.push_back(CppMountItem::RemoveMountItem( parentShadowView, oldChildShadowView, index)); } @@ -302,10 +302,8 @@ void FabricMountingManager::executeMount( break; } case ShadowViewMutation::Delete: { - if (!mutation.isRedundantOperation) { - cppDeleteMountItems.push_back( - CppMountItem::DeleteMountItem(oldChildShadowView)); - } + cppDeleteMountItems.push_back( + CppMountItem::DeleteMountItem(oldChildShadowView)); break; } case ShadowViewMutation::Update: { diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/featureflags/JReactNativeFeatureFlagsCxxInterop.cpp b/packages/react-native/ReactAndroid/src/main/jni/react/featureflags/JReactNativeFeatureFlagsCxxInterop.cpp index cfd8a671cbbf..dcb2fbd3c4c2 100644 --- a/packages/react-native/ReactAndroid/src/main/jni/react/featureflags/JReactNativeFeatureFlagsCxxInterop.cpp +++ b/packages/react-native/ReactAndroid/src/main/jni/react/featureflags/JReactNativeFeatureFlagsCxxInterop.cpp @@ -4,7 +4,7 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * - * @generated SignedSource<<0a287c69e07600efe13acfcf8bd29f0d>> + * @generated SignedSource<<9389654cf7ca12851a1af307656c807b>> */ /** @@ -129,12 +129,6 @@ class ReactNativeFeatureFlagsProviderHolder return method(javaProvider_); } - bool fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak() override { - static const auto method = - getReactNativeFeatureFlagsProviderJavaClass()->getMethod("fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak"); - return method(javaProvider_); - } - bool forceBatchingMountItemsOnAndroid() override { static const auto method = getReactNativeFeatureFlagsProviderJavaClass()->getMethod("forceBatchingMountItemsOnAndroid"); @@ -292,11 +286,6 @@ bool JReactNativeFeatureFlagsCxxInterop::fixMissedFabricStateUpdatesOnAndroid( return ReactNativeFeatureFlags::fixMissedFabricStateUpdatesOnAndroid(); } -bool JReactNativeFeatureFlagsCxxInterop::fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak( - facebook::jni::alias_ref /*unused*/) { - return ReactNativeFeatureFlags::fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak(); -} - bool JReactNativeFeatureFlagsCxxInterop::forceBatchingMountItemsOnAndroid( facebook::jni::alias_ref /*unused*/) { return ReactNativeFeatureFlags::forceBatchingMountItemsOnAndroid(); @@ -424,9 +413,6 @@ void JReactNativeFeatureFlagsCxxInterop::registerNatives() { makeNativeMethod( "fixMissedFabricStateUpdatesOnAndroid", JReactNativeFeatureFlagsCxxInterop::fixMissedFabricStateUpdatesOnAndroid), - makeNativeMethod( - "fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak", - JReactNativeFeatureFlagsCxxInterop::fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak), makeNativeMethod( "forceBatchingMountItemsOnAndroid", JReactNativeFeatureFlagsCxxInterop::forceBatchingMountItemsOnAndroid), diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/featureflags/JReactNativeFeatureFlagsCxxInterop.h b/packages/react-native/ReactAndroid/src/main/jni/react/featureflags/JReactNativeFeatureFlagsCxxInterop.h index 8390e2a4da75..8b4a761c7d6f 100644 --- a/packages/react-native/ReactAndroid/src/main/jni/react/featureflags/JReactNativeFeatureFlagsCxxInterop.h +++ b/packages/react-native/ReactAndroid/src/main/jni/react/featureflags/JReactNativeFeatureFlagsCxxInterop.h @@ -4,7 +4,7 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * - * @generated SignedSource<<8bc3813da5b0d8ad6a46064b30f48ea8>> + * @generated SignedSource<<9d36812c37a99a514fc80e83ec056365>> */ /** @@ -75,9 +75,6 @@ class JReactNativeFeatureFlagsCxxInterop static bool fixMissedFabricStateUpdatesOnAndroid( facebook::jni::alias_ref); - static bool fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak( - facebook::jni::alias_ref); - static bool forceBatchingMountItemsOnAndroid( facebook::jni::alias_ref); diff --git a/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlags.cpp b/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlags.cpp index 2260f4273350..b4297c3390cf 100644 --- a/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlags.cpp +++ b/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlags.cpp @@ -4,7 +4,7 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * - * @generated SignedSource<<1bea72c406359550598787ea5da29b17>> + * @generated SignedSource<> */ /** @@ -81,10 +81,6 @@ bool ReactNativeFeatureFlags::fixMissedFabricStateUpdatesOnAndroid() { return getAccessor().fixMissedFabricStateUpdatesOnAndroid(); } -bool ReactNativeFeatureFlags::fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak() { - return getAccessor().fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak(); -} - bool ReactNativeFeatureFlags::forceBatchingMountItemsOnAndroid() { return getAccessor().forceBatchingMountItemsOnAndroid(); } diff --git a/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlags.h b/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlags.h index 947ec66fb802..2a089c98d03e 100644 --- a/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlags.h +++ b/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlags.h @@ -4,7 +4,7 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * - * @generated SignedSource<<627fb0dc656780d46131fad9bf87efa8>> + * @generated SignedSource<> */ /** @@ -112,11 +112,6 @@ class ReactNativeFeatureFlags { */ RN_EXPORT static bool fixMissedFabricStateUpdatesOnAndroid(); - /** - * Fixes a leak in SurfaceMountingManager.mRemoveDeleteTreeUIFrameCallback - */ - RN_EXPORT static bool fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak(); - /** * Forces the mounting layer on Android to always batch mount items instead of dispatching them immediately. This might fix some crashes related to synchronous state updates, where some views dispatch state updates during mount. */ diff --git a/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlagsAccessor.cpp b/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlagsAccessor.cpp index af1e85974499..c929d91bce23 100644 --- a/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlagsAccessor.cpp +++ b/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlagsAccessor.cpp @@ -4,7 +4,7 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * - * @generated SignedSource<<23169873ef7f5c07f5c07f59a7440d47>> + * @generated SignedSource<<213b150588cbaf067c937cfa755791ca>> */ /** @@ -299,24 +299,6 @@ bool ReactNativeFeatureFlagsAccessor::fixMissedFabricStateUpdatesOnAndroid() { return flagValue.value(); } -bool ReactNativeFeatureFlagsAccessor::fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak() { - auto flagValue = fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak_.load(); - - if (!flagValue.has_value()) { - // This block is not exclusive but it is not necessary. - // If multiple threads try to initialize the feature flag, we would only - // be accessing the provider multiple times but the end state of this - // instance and the returned flag value would be the same. - - markFlagAsAccessed(15, "fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak"); - - flagValue = currentProvider_->fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak(); - fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak_ = flagValue; - } - - return flagValue.value(); -} - bool ReactNativeFeatureFlagsAccessor::forceBatchingMountItemsOnAndroid() { auto flagValue = forceBatchingMountItemsOnAndroid_.load(); @@ -326,7 +308,7 @@ bool ReactNativeFeatureFlagsAccessor::forceBatchingMountItemsOnAndroid() { // be accessing the provider multiple times but the end state of this // instance and the returned flag value would be the same. - markFlagAsAccessed(16, "forceBatchingMountItemsOnAndroid"); + markFlagAsAccessed(15, "forceBatchingMountItemsOnAndroid"); flagValue = currentProvider_->forceBatchingMountItemsOnAndroid(); forceBatchingMountItemsOnAndroid_ = flagValue; @@ -344,7 +326,7 @@ bool ReactNativeFeatureFlagsAccessor::fuseboxEnabledDebug() { // be accessing the provider multiple times but the end state of this // instance and the returned flag value would be the same. - markFlagAsAccessed(17, "fuseboxEnabledDebug"); + markFlagAsAccessed(16, "fuseboxEnabledDebug"); flagValue = currentProvider_->fuseboxEnabledDebug(); fuseboxEnabledDebug_ = flagValue; @@ -362,7 +344,7 @@ bool ReactNativeFeatureFlagsAccessor::fuseboxEnabledRelease() { // be accessing the provider multiple times but the end state of this // instance and the returned flag value would be the same. - markFlagAsAccessed(18, "fuseboxEnabledRelease"); + markFlagAsAccessed(17, "fuseboxEnabledRelease"); flagValue = currentProvider_->fuseboxEnabledRelease(); fuseboxEnabledRelease_ = flagValue; @@ -380,7 +362,7 @@ bool ReactNativeFeatureFlagsAccessor::initEagerTurboModulesOnNativeModulesQueueA // be accessing the provider multiple times but the end state of this // instance and the returned flag value would be the same. - markFlagAsAccessed(19, "initEagerTurboModulesOnNativeModulesQueueAndroid"); + markFlagAsAccessed(18, "initEagerTurboModulesOnNativeModulesQueueAndroid"); flagValue = currentProvider_->initEagerTurboModulesOnNativeModulesQueueAndroid(); initEagerTurboModulesOnNativeModulesQueueAndroid_ = flagValue; @@ -398,7 +380,7 @@ bool ReactNativeFeatureFlagsAccessor::lazyAnimationCallbacks() { // be accessing the provider multiple times but the end state of this // instance and the returned flag value would be the same. - markFlagAsAccessed(20, "lazyAnimationCallbacks"); + markFlagAsAccessed(19, "lazyAnimationCallbacks"); flagValue = currentProvider_->lazyAnimationCallbacks(); lazyAnimationCallbacks_ = flagValue; @@ -416,7 +398,7 @@ bool ReactNativeFeatureFlagsAccessor::setAndroidLayoutDirection() { // be accessing the provider multiple times but the end state of this // instance and the returned flag value would be the same. - markFlagAsAccessed(21, "setAndroidLayoutDirection"); + markFlagAsAccessed(20, "setAndroidLayoutDirection"); flagValue = currentProvider_->setAndroidLayoutDirection(); setAndroidLayoutDirection_ = flagValue; @@ -434,7 +416,7 @@ bool ReactNativeFeatureFlagsAccessor::useImmediateExecutorInAndroidBridgeless() // be accessing the provider multiple times but the end state of this // instance and the returned flag value would be the same. - markFlagAsAccessed(22, "useImmediateExecutorInAndroidBridgeless"); + markFlagAsAccessed(21, "useImmediateExecutorInAndroidBridgeless"); flagValue = currentProvider_->useImmediateExecutorInAndroidBridgeless(); useImmediateExecutorInAndroidBridgeless_ = flagValue; @@ -452,7 +434,7 @@ bool ReactNativeFeatureFlagsAccessor::useModernRuntimeScheduler() { // be accessing the provider multiple times but the end state of this // instance and the returned flag value would be the same. - markFlagAsAccessed(23, "useModernRuntimeScheduler"); + markFlagAsAccessed(22, "useModernRuntimeScheduler"); flagValue = currentProvider_->useModernRuntimeScheduler(); useModernRuntimeScheduler_ = flagValue; @@ -470,7 +452,7 @@ bool ReactNativeFeatureFlagsAccessor::useNativeViewConfigsInBridgelessMode() { // be accessing the provider multiple times but the end state of this // instance and the returned flag value would be the same. - markFlagAsAccessed(24, "useNativeViewConfigsInBridgelessMode"); + markFlagAsAccessed(23, "useNativeViewConfigsInBridgelessMode"); flagValue = currentProvider_->useNativeViewConfigsInBridgelessMode(); useNativeViewConfigsInBridgelessMode_ = flagValue; @@ -488,7 +470,7 @@ bool ReactNativeFeatureFlagsAccessor::useNewReactImageViewBackgroundDrawing() { // be accessing the provider multiple times but the end state of this // instance and the returned flag value would be the same. - markFlagAsAccessed(25, "useNewReactImageViewBackgroundDrawing"); + markFlagAsAccessed(24, "useNewReactImageViewBackgroundDrawing"); flagValue = currentProvider_->useNewReactImageViewBackgroundDrawing(); useNewReactImageViewBackgroundDrawing_ = flagValue; @@ -506,7 +488,7 @@ bool ReactNativeFeatureFlagsAccessor::useRuntimeShadowNodeReferenceUpdate() { // be accessing the provider multiple times but the end state of this // instance and the returned flag value would be the same. - markFlagAsAccessed(26, "useRuntimeShadowNodeReferenceUpdate"); + markFlagAsAccessed(25, "useRuntimeShadowNodeReferenceUpdate"); flagValue = currentProvider_->useRuntimeShadowNodeReferenceUpdate(); useRuntimeShadowNodeReferenceUpdate_ = flagValue; @@ -524,7 +506,7 @@ bool ReactNativeFeatureFlagsAccessor::useRuntimeShadowNodeReferenceUpdateOnLayou // be accessing the provider multiple times but the end state of this // instance and the returned flag value would be the same. - markFlagAsAccessed(27, "useRuntimeShadowNodeReferenceUpdateOnLayout"); + markFlagAsAccessed(26, "useRuntimeShadowNodeReferenceUpdateOnLayout"); flagValue = currentProvider_->useRuntimeShadowNodeReferenceUpdateOnLayout(); useRuntimeShadowNodeReferenceUpdateOnLayout_ = flagValue; @@ -542,7 +524,7 @@ bool ReactNativeFeatureFlagsAccessor::useStateAlignmentMechanism() { // be accessing the provider multiple times but the end state of this // instance and the returned flag value would be the same. - markFlagAsAccessed(28, "useStateAlignmentMechanism"); + markFlagAsAccessed(27, "useStateAlignmentMechanism"); flagValue = currentProvider_->useStateAlignmentMechanism(); useStateAlignmentMechanism_ = flagValue; diff --git a/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlagsAccessor.h b/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlagsAccessor.h index d3ed1db3de2b..b47082c84a23 100644 --- a/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlagsAccessor.h +++ b/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlagsAccessor.h @@ -4,7 +4,7 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * - * @generated SignedSource<> + * @generated SignedSource<<086584dd69b004ee0f083b76d6368f84>> */ /** @@ -46,7 +46,6 @@ class ReactNativeFeatureFlagsAccessor { bool fixIncorrectScrollViewStateUpdateOnAndroid(); bool fixMappingOfEventPrioritiesBetweenFabricAndReact(); bool fixMissedFabricStateUpdatesOnAndroid(); - bool fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak(); bool forceBatchingMountItemsOnAndroid(); bool fuseboxEnabledDebug(); bool fuseboxEnabledRelease(); @@ -70,7 +69,7 @@ class ReactNativeFeatureFlagsAccessor { std::unique_ptr currentProvider_; bool wasOverridden_; - std::array, 29> accessedFeatureFlags_; + std::array, 28> accessedFeatureFlags_; std::atomic> commonTestFlag_; std::atomic> allowCollapsableChildren_; @@ -87,7 +86,6 @@ class ReactNativeFeatureFlagsAccessor { std::atomic> fixIncorrectScrollViewStateUpdateOnAndroid_; std::atomic> fixMappingOfEventPrioritiesBetweenFabricAndReact_; std::atomic> fixMissedFabricStateUpdatesOnAndroid_; - std::atomic> fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak_; std::atomic> forceBatchingMountItemsOnAndroid_; std::atomic> fuseboxEnabledDebug_; std::atomic> fuseboxEnabledRelease_; diff --git a/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlagsDefaults.h b/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlagsDefaults.h index 6b5e88cc0775..521155c6f428 100644 --- a/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlagsDefaults.h +++ b/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlagsDefaults.h @@ -4,7 +4,7 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * - * @generated SignedSource<<031622096a93ca9da5e278dab84324b9>> + * @generated SignedSource<<108ca764e4098dab0467165c974d1278>> */ /** @@ -87,10 +87,6 @@ class ReactNativeFeatureFlagsDefaults : public ReactNativeFeatureFlagsProvider { return false; } - bool fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak() override { - return false; - } - bool forceBatchingMountItemsOnAndroid() override { return false; } diff --git a/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlagsProvider.h b/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlagsProvider.h index 6b6c1c2cffb9..50eb628b6529 100644 --- a/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlagsProvider.h +++ b/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlagsProvider.h @@ -4,7 +4,7 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * - * @generated SignedSource<<55e4299def48d95ea40cd988e70fd608>> + * @generated SignedSource<> */ /** @@ -40,7 +40,6 @@ class ReactNativeFeatureFlagsProvider { virtual bool fixIncorrectScrollViewStateUpdateOnAndroid() = 0; virtual bool fixMappingOfEventPrioritiesBetweenFabricAndReact() = 0; virtual bool fixMissedFabricStateUpdatesOnAndroid() = 0; - virtual bool fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak() = 0; virtual bool forceBatchingMountItemsOnAndroid() = 0; virtual bool fuseboxEnabledDebug() = 0; virtual bool fuseboxEnabledRelease() = 0; diff --git a/packages/react-native/ReactCommon/react/nativemodule/featureflags/NativeReactNativeFeatureFlags.cpp b/packages/react-native/ReactCommon/react/nativemodule/featureflags/NativeReactNativeFeatureFlags.cpp index 944e9ef00a1a..945656695194 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/featureflags/NativeReactNativeFeatureFlags.cpp +++ b/packages/react-native/ReactCommon/react/nativemodule/featureflags/NativeReactNativeFeatureFlags.cpp @@ -4,7 +4,7 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * - * @generated SignedSource<<345d27de837a248fbfd5e299754097f0>> + * @generated SignedSource<> */ /** @@ -112,11 +112,6 @@ bool NativeReactNativeFeatureFlags::fixMissedFabricStateUpdatesOnAndroid( return ReactNativeFeatureFlags::fixMissedFabricStateUpdatesOnAndroid(); } -bool NativeReactNativeFeatureFlags::fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak( - jsi::Runtime& /*runtime*/) { - return ReactNativeFeatureFlags::fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak(); -} - bool NativeReactNativeFeatureFlags::forceBatchingMountItemsOnAndroid( jsi::Runtime& /*runtime*/) { return ReactNativeFeatureFlags::forceBatchingMountItemsOnAndroid(); diff --git a/packages/react-native/ReactCommon/react/nativemodule/featureflags/NativeReactNativeFeatureFlags.h b/packages/react-native/ReactCommon/react/nativemodule/featureflags/NativeReactNativeFeatureFlags.h index ea92618e04ca..2435f7aa9f43 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/featureflags/NativeReactNativeFeatureFlags.h +++ b/packages/react-native/ReactCommon/react/nativemodule/featureflags/NativeReactNativeFeatureFlags.h @@ -4,7 +4,7 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * - * @generated SignedSource<<4121c9b419a41e84ec2c1066cb2707e2>> + * @generated SignedSource<<735fa03ec34232ee74161db833eadf25>> */ /** @@ -65,8 +65,6 @@ class NativeReactNativeFeatureFlags bool fixMissedFabricStateUpdatesOnAndroid(jsi::Runtime& runtime); - bool fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak(jsi::Runtime& runtime); - bool forceBatchingMountItemsOnAndroid(jsi::Runtime& runtime); bool fuseboxEnabledDebug(jsi::Runtime& runtime); diff --git a/packages/react-native/ReactCommon/react/renderer/mounting/Differentiator.cpp b/packages/react-native/ReactCommon/react/renderer/mounting/Differentiator.cpp index aa511645bbf0..a1027e89b049 100644 --- a/packages/react-native/ReactCommon/react/renderer/mounting/Differentiator.cpp +++ b/packages/react-native/ReactCommon/react/renderer/mounting/Differentiator.cpp @@ -362,8 +362,7 @@ static void calculateShadowViewMutations( ShadowViewMutation::List& mutations, const ShadowView& parentShadowView, ShadowViewNodePair::NonOwningList&& oldChildPairs, - ShadowViewNodePair::NonOwningList&& newChildPairs, - bool isRecursionRedundant = false); + ShadowViewNodePair::NonOwningList&& newChildPairs); struct OrderedMutationInstructionContainer { ShadowViewMutation::List createMutations{}; @@ -1052,8 +1051,7 @@ static void calculateShadowViewMutations( ShadowViewMutation::List& mutations, const ShadowView& parentShadowView, ShadowViewNodePair::NonOwningList&& oldChildPairs, - ShadowViewNodePair::NonOwningList&& newChildPairs, - bool isRecursionRedundant) { + ShadowViewNodePair::NonOwningList&& newChildPairs) { if (oldChildPairs.empty() && newChildPairs.empty()) { return; } @@ -1171,39 +1169,13 @@ static void calculateShadowViewMutations( continue; } - // If we take this path, technically the operations and recursion below - // are redundant. However, some parts of the Fabric ecosystem (namely, as - // of writing this, LayoutAnimations) rely heavily on getting /explicit/ - // Remove/Delete instructions for every single node in the tree. Thus, we - // generate the "RemoveDeleteTree" instruction as well as all of the - // individual Remove/Delete operations below, but we mark those as - // redundant. The platform layer can then discard the unnecessary - // instructions. RemoveDeleteTreeMutation is a significant performance - // improvement but could be improved significantly by eliminating the need - // for any of the redundant instructions in the future. - if (ShadowViewMutation::PlatformSupportsRemoveDeleteTreeInstruction && - !isRecursionRedundant) { - mutationContainer.removeMutations.push_back( - ShadowViewMutation::RemoveDeleteTreeMutation( - parentShadowView, - oldChildPair.shadowView, - static_cast(oldChildPair.mountIndex))); - } - mutationContainer.deleteMutations.push_back( - ShadowViewMutation::DeleteMutation( - oldChildPair.shadowView, - isRecursionRedundant || - ShadowViewMutation:: - PlatformSupportsRemoveDeleteTreeInstruction)); + ShadowViewMutation::DeleteMutation(oldChildPair.shadowView)); mutationContainer.removeMutations.push_back( ShadowViewMutation::RemoveMutation( parentShadowView, oldChildPair.shadowView, - static_cast(oldChildPair.mountIndex), - isRecursionRedundant || - ShadowViewMutation:: - PlatformSupportsRemoveDeleteTreeInstruction)); + static_cast(oldChildPair.mountIndex))); // We also have to call the algorithm recursively to clean up the entire // subtree starting from the removed view. @@ -1214,8 +1186,7 @@ static void calculateShadowViewMutations( oldChildPair.shadowView, sliceChildShadowNodeViewPairsFromViewNodePair( oldChildPair, innerScope), - {}, - ShadowViewMutation::PlatformSupportsRemoveDeleteTreeInstruction); + {}); } } else if (index == oldChildPairs.size()) { // If we don't have any more existing children we can choose a fast path diff --git a/packages/react-native/ReactCommon/react/renderer/mounting/ShadowViewMutation.cpp b/packages/react-native/ReactCommon/react/renderer/mounting/ShadowViewMutation.cpp index 25445b1badc6..c64b1dff6487 100644 --- a/packages/react-native/ReactCommon/react/renderer/mounting/ShadowViewMutation.cpp +++ b/packages/react-native/ReactCommon/react/renderer/mounting/ShadowViewMutation.cpp @@ -11,12 +11,6 @@ namespace facebook::react { -/** - * Initialize static feature flags for this module. - * These flags should be treated as temporary. - */ -bool ShadowViewMutation::PlatformSupportsRemoveDeleteTreeInstruction = false; - ShadowViewMutation ShadowViewMutation::CreateMutation(ShadowView shadowView) { return { /* .type = */ Create, @@ -27,16 +21,13 @@ ShadowViewMutation ShadowViewMutation::CreateMutation(ShadowView shadowView) { }; } -ShadowViewMutation ShadowViewMutation::DeleteMutation( - ShadowView shadowView, - bool isRedundantOperation) { +ShadowViewMutation ShadowViewMutation::DeleteMutation(ShadowView shadowView) { return { /* .type = */ Delete, /* .parentShadowView = */ {}, /* .oldChildShadowView = */ std::move(shadowView), /* .newChildShadowView = */ {}, /* .index = */ -1, - /* .isRedundantOperation */ isRedundantOperation, }; } @@ -56,15 +47,13 @@ ShadowViewMutation ShadowViewMutation::InsertMutation( ShadowViewMutation ShadowViewMutation::RemoveMutation( ShadowView parentShadowView, ShadowView childShadowView, - int index, - bool isRedundantOperation) { + int index) { return { /* .type = */ Remove, /* .parentShadowView = */ std::move(parentShadowView), /* .oldChildShadowView = */ std::move(childShadowView), /* .newChildShadowView = */ {}, /* .index = */ index, - /* .isRedundantOperation */ isRedundantOperation, }; } @@ -115,14 +104,12 @@ ShadowViewMutation::ShadowViewMutation( ShadowView parentShadowView, ShadowView oldChildShadowView, ShadowView newChildShadowView, - int index, - bool isRedundantOperation) + int index) : type(type), parentShadowView(std::move(parentShadowView)), oldChildShadowView(std::move(oldChildShadowView)), newChildShadowView(std::move(newChildShadowView)), - index(index), - isRedundantOperation(isRedundantOperation) {} + index(index) {} #if RN_DEBUG_STRING_CONVERTIBLE diff --git a/packages/react-native/ReactCommon/react/renderer/mounting/ShadowViewMutation.h b/packages/react-native/ReactCommon/react/renderer/mounting/ShadowViewMutation.h index de399dfe1c6a..f5725fcb1b31 100644 --- a/packages/react-native/ReactCommon/react/renderer/mounting/ShadowViewMutation.h +++ b/packages/react-native/ReactCommon/react/renderer/mounting/ShadowViewMutation.h @@ -24,10 +24,6 @@ struct ShadowViewMutation final { ShadowViewMutation() = delete; -#pragma mark - Platform feature flags - - static bool PlatformSupportsRemoveDeleteTreeInstruction; - #pragma mark - Designated Initializers /* @@ -38,9 +34,7 @@ struct ShadowViewMutation final { /* * Creates and returns an `Delete` mutation. */ - static ShadowViewMutation DeleteMutation( - ShadowView shadowView, - bool isRedundantOperation = false); + static ShadowViewMutation DeleteMutation(ShadowView shadowView); /* * Creates and returns an `Insert` mutation. @@ -56,8 +50,7 @@ struct ShadowViewMutation final { static ShadowViewMutation RemoveMutation( ShadowView parentShadowView, ShadowView childShadowView, - int index, - bool isRedundantOperation = false); + int index); /* * Creates and returns a `RemoveDelete` mutation. @@ -97,12 +90,6 @@ struct ShadowViewMutation final { ShadowView newChildShadowView = {}; int index = -1; - // RemoveDeleteTree causes many Remove/Delete operations to be redundant. - // However, we must internally produce all of them for any consumers that - // rely on explicit instructions to remove/delete every node in the tree. - // Notably (as of the time of writing this) LayoutAnimations. - bool isRedundantOperation = false; - // Some platforms can have the notion of virtual views - views that are in the // ShadowTree hierarchy but never are on the platform. Generally this is used // so notify the platform that a view exists so that we can keep EventEmitters @@ -116,8 +103,7 @@ struct ShadowViewMutation final { ShadowView parentShadowView, ShadowView oldChildShadowView, ShadowView newChildShadowView, - int index, - bool isRedundantOperation = false); + int index); }; using ShadowViewMutationList = std::vector; diff --git a/packages/react-native/scripts/featureflags/ReactNativeFeatureFlags.config.js b/packages/react-native/scripts/featureflags/ReactNativeFeatureFlags.config.js index da677636559f..3a4e403e6f10 100644 --- a/packages/react-native/scripts/featureflags/ReactNativeFeatureFlags.config.js +++ b/packages/react-native/scripts/featureflags/ReactNativeFeatureFlags.config.js @@ -108,11 +108,6 @@ const definitions: FeatureFlagDefinitions = { description: 'Enables a fix to prevent the possibility of state updates in Fabric being missed due to race conditions with previous state updates.', }, - fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak: { - defaultValue: false, - description: - 'Fixes a leak in SurfaceMountingManager.mRemoveDeleteTreeUIFrameCallback', - }, forceBatchingMountItemsOnAndroid: { defaultValue: false, description: diff --git a/packages/react-native/src/private/featureflags/ReactNativeFeatureFlags.js b/packages/react-native/src/private/featureflags/ReactNativeFeatureFlags.js index 3fd57ddfddb7..1eb0dc4b9a16 100644 --- a/packages/react-native/src/private/featureflags/ReactNativeFeatureFlags.js +++ b/packages/react-native/src/private/featureflags/ReactNativeFeatureFlags.js @@ -4,7 +4,7 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * - * @generated SignedSource<<3ee4e03eae552b81423262351ee2a494>> + * @generated SignedSource<<07014a208bbac8efec4d01b7f09c6910>> * @flow strict-local */ @@ -56,7 +56,6 @@ export type ReactNativeFeatureFlags = { fixIncorrectScrollViewStateUpdateOnAndroid: Getter, fixMappingOfEventPrioritiesBetweenFabricAndReact: Getter, fixMissedFabricStateUpdatesOnAndroid: Getter, - fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak: Getter, forceBatchingMountItemsOnAndroid: Getter, fuseboxEnabledDebug: Getter, fuseboxEnabledRelease: Getter, @@ -177,10 +176,6 @@ export const fixMappingOfEventPrioritiesBetweenFabricAndReact: Getter = * Enables a fix to prevent the possibility of state updates in Fabric being missed due to race conditions with previous state updates. */ export const fixMissedFabricStateUpdatesOnAndroid: Getter = createNativeFlagGetter('fixMissedFabricStateUpdatesOnAndroid', false); -/** - * Fixes a leak in SurfaceMountingManager.mRemoveDeleteTreeUIFrameCallback - */ -export const fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak: Getter = createNativeFlagGetter('fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak', false); /** * Forces the mounting layer on Android to always batch mount items instead of dispatching them immediately. This might fix some crashes related to synchronous state updates, where some views dispatch state updates during mount. */ diff --git a/packages/react-native/src/private/featureflags/specs/NativeReactNativeFeatureFlags.js b/packages/react-native/src/private/featureflags/specs/NativeReactNativeFeatureFlags.js index 134f984574fe..eb3073c48db7 100644 --- a/packages/react-native/src/private/featureflags/specs/NativeReactNativeFeatureFlags.js +++ b/packages/react-native/src/private/featureflags/specs/NativeReactNativeFeatureFlags.js @@ -4,7 +4,7 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * - * @generated SignedSource<<44f6fe8269f1b8d2039668e67bca9839>> + * @generated SignedSource<> * @flow strict-local */ @@ -38,7 +38,6 @@ export interface Spec extends TurboModule { +fixIncorrectScrollViewStateUpdateOnAndroid?: () => boolean; +fixMappingOfEventPrioritiesBetweenFabricAndReact?: () => boolean; +fixMissedFabricStateUpdatesOnAndroid?: () => boolean; - +fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak?: () => boolean; +forceBatchingMountItemsOnAndroid?: () => boolean; +fuseboxEnabledDebug?: () => boolean; +fuseboxEnabledRelease?: () => boolean;