Skip to content

Commit

Permalink
Create flag to disable reentrancy in MountItemDispatcher (#46561)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #46561

Changelog: [internal]

We have some logic in `MountItemDispatcher.tryDispatchMountItems` to recursively call itself for as long as there are new items to mount in the general queue. This queue is copied and processed every time `tryDispatchMountItems` is called, but it's possible that during this time new mount operations are added (either from JS or from the mount operations themselves).

This logic can cause frame drops, as we could be finish processing the current frame without processing new mount items (we could do that in the next frame, and in many cases it's just pre-rendering work and state updates that the user can't perceive anyway).

This creates a feature flag to test disabling this behavior and only process whatever mount items are queued at the start of the frame.

Reviewed By: sammy-SC

Differential Revision: D62958009

fbshipit-source-id: ca038c827715411375410215deb5d5e374f5fdb4
  • Loading branch information
rubennorte authored and facebook-github-bot committed Sep 18, 2024
1 parent 42bad68 commit a5dd56f
Show file tree
Hide file tree
Showing 20 changed files with 151 additions and 46 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -137,22 +137,25 @@ public void tryDispatchMountItems() {
// scheduled
mItemDispatchListener.didDispatchMountItems();

// Decide if we want to try reentering
if (mReDispatchCounter < 10 && didDispatchItems) {
// Executing twice in a row is normal. Only log after that point.
if (mReDispatchCounter > 2) {
ReactSoftExceptionLogger.logSoftException(
TAG,
new ReactNoCrashSoftException(
"Re-dispatched "
+ mReDispatchCounter
+ " times. This indicates setState (?) is likely being called too many times"
+ " during mounting."));
}
if (!ReactNativeFeatureFlags.removeNestedCallsToDispatchMountItemsOnAndroid()) {
// Decide if we want to try reentering
if (mReDispatchCounter < 10 && didDispatchItems) {
// Executing twice in a row is normal. Only log after that point.
if (mReDispatchCounter > 2) {
ReactSoftExceptionLogger.logSoftException(
TAG,
new ReactNoCrashSoftException(
"Re-dispatched "
+ mReDispatchCounter
+ " times. This indicates setState (?) is likely being called too many"
+ " times during mounting."));
}

mReDispatchCounter++;
tryDispatchMountItems();
mReDispatchCounter++;
tryDispatchMountItems();
}
}

mReDispatchCounter = 0;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<<ac309a5ea9ff969824d34cca43b45ddf>>
* @generated SignedSource<<60fcb02736ca814a842225291e3a55b1>>
*/

/**
Expand Down Expand Up @@ -244,6 +244,12 @@ public object ReactNativeFeatureFlags {
@JvmStatic
public fun loadVectorDrawablesOnImages(): Boolean = accessor.loadVectorDrawablesOnImages()

/**
* Removes nested calls to MountItemDispatcher.dispatchMountItems on Android, so we do less work per frame on the UI thread.
*/
@JvmStatic
public fun removeNestedCallsToDispatchMountItemsOnAndroid(): Boolean = accessor.removeNestedCallsToDispatchMountItemsOnAndroid()

/**
* Propagate layout direction to Android views.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<<f504580bc8284172ad6a35c7ddbd7417>>
* @generated SignedSource<<f2a64fef9775f1890adcf3d761061182>>
*/

/**
Expand Down Expand Up @@ -56,6 +56,7 @@ public class ReactNativeFeatureFlagsCxxAccessor : ReactNativeFeatureFlagsAccesso
private var initEagerTurboModulesOnNativeModulesQueueAndroidCache: Boolean? = null
private var lazyAnimationCallbacksCache: Boolean? = null
private var loadVectorDrawablesOnImagesCache: Boolean? = null
private var removeNestedCallsToDispatchMountItemsOnAndroidCache: Boolean? = null
private var setAndroidLayoutDirectionCache: Boolean? = null
private var traceTurboModulePromiseRejectionsOnAndroidCache: Boolean? = null
private var useFabricInteropCache: Boolean? = null
Expand Down Expand Up @@ -394,6 +395,15 @@ public class ReactNativeFeatureFlagsCxxAccessor : ReactNativeFeatureFlagsAccesso
return cached
}

override fun removeNestedCallsToDispatchMountItemsOnAndroid(): Boolean {
var cached = removeNestedCallsToDispatchMountItemsOnAndroidCache
if (cached == null) {
cached = ReactNativeFeatureFlagsCxxInterop.removeNestedCallsToDispatchMountItemsOnAndroid()
removeNestedCallsToDispatchMountItemsOnAndroidCache = cached
}
return cached
}

override fun setAndroidLayoutDirection(): Boolean {
var cached = setAndroidLayoutDirectionCache
if (cached == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<<31fb0e1bcd77d57f41d69cdbbdd15af4>>
* @generated SignedSource<<03618f182004add50655a88e6e65e96d>>
*/

/**
Expand Down Expand Up @@ -100,6 +100,8 @@ public object ReactNativeFeatureFlagsCxxInterop {

@DoNotStrip @JvmStatic public external fun loadVectorDrawablesOnImages(): Boolean

@DoNotStrip @JvmStatic public external fun removeNestedCallsToDispatchMountItemsOnAndroid(): Boolean

@DoNotStrip @JvmStatic public external fun setAndroidLayoutDirection(): Boolean

@DoNotStrip @JvmStatic public external fun traceTurboModulePromiseRejectionsOnAndroid(): Boolean
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<<06791e428f1e199b718f11c6bc1f1414>>
* @generated SignedSource<<fc795741e932cf24e5bfb7e2fe1046ae>>
*/

/**
Expand Down Expand Up @@ -95,6 +95,8 @@ public open class ReactNativeFeatureFlagsDefaults : ReactNativeFeatureFlagsProvi

override fun loadVectorDrawablesOnImages(): Boolean = false

override fun removeNestedCallsToDispatchMountItemsOnAndroid(): Boolean = false

override fun setAndroidLayoutDirection(): Boolean = false

override fun traceTurboModulePromiseRejectionsOnAndroid(): Boolean = false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<<3c321dea3f540266bec3769031460128>>
* @generated SignedSource<<0dbb1f1a08649132955698ac51050e62>>
*/

/**
Expand Down Expand Up @@ -60,6 +60,7 @@ public class ReactNativeFeatureFlagsLocalAccessor : ReactNativeFeatureFlagsAcces
private var initEagerTurboModulesOnNativeModulesQueueAndroidCache: Boolean? = null
private var lazyAnimationCallbacksCache: Boolean? = null
private var loadVectorDrawablesOnImagesCache: Boolean? = null
private var removeNestedCallsToDispatchMountItemsOnAndroidCache: Boolean? = null
private var setAndroidLayoutDirectionCache: Boolean? = null
private var traceTurboModulePromiseRejectionsOnAndroidCache: Boolean? = null
private var useFabricInteropCache: Boolean? = null
Expand Down Expand Up @@ -434,6 +435,16 @@ public class ReactNativeFeatureFlagsLocalAccessor : ReactNativeFeatureFlagsAcces
return cached
}

override fun removeNestedCallsToDispatchMountItemsOnAndroid(): Boolean {
var cached = removeNestedCallsToDispatchMountItemsOnAndroidCache
if (cached == null) {
cached = currentProvider.removeNestedCallsToDispatchMountItemsOnAndroid()
accessedFeatureFlags.add("removeNestedCallsToDispatchMountItemsOnAndroid")
removeNestedCallsToDispatchMountItemsOnAndroidCache = cached
}
return cached
}

override fun setAndroidLayoutDirection(): Boolean {
var cached = setAndroidLayoutDirectionCache
if (cached == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<<cab6789c15431b677b1ccff477933a74>>
* @generated SignedSource<<79cd9812257f42d553b49a3f6d3d4989>>
*/

/**
Expand Down Expand Up @@ -95,6 +95,8 @@ public interface ReactNativeFeatureFlagsProvider {

@DoNotStrip public fun loadVectorDrawablesOnImages(): Boolean

@DoNotStrip public fun removeNestedCallsToDispatchMountItemsOnAndroid(): Boolean

@DoNotStrip public fun setAndroidLayoutDirection(): Boolean

@DoNotStrip public fun traceTurboModulePromiseRejectionsOnAndroid(): Boolean
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<<b91f02111aeabf0a3556c869eabb3bb3>>
* @generated SignedSource<<9e891a5d26221136d7334982fc2e99d3>>
*/

/**
Expand Down Expand Up @@ -255,6 +255,12 @@ class ReactNativeFeatureFlagsProviderHolder
return method(javaProvider_);
}

bool removeNestedCallsToDispatchMountItemsOnAndroid() override {
static const auto method =
getReactNativeFeatureFlagsProviderJavaClass()->getMethod<jboolean()>("removeNestedCallsToDispatchMountItemsOnAndroid");
return method(javaProvider_);
}

bool setAndroidLayoutDirection() override {
static const auto method =
getReactNativeFeatureFlagsProviderJavaClass()->getMethod<jboolean()>("setAndroidLayoutDirection");
Expand Down Expand Up @@ -517,6 +523,11 @@ bool JReactNativeFeatureFlagsCxxInterop::loadVectorDrawablesOnImages(
return ReactNativeFeatureFlags::loadVectorDrawablesOnImages();
}

bool JReactNativeFeatureFlagsCxxInterop::removeNestedCallsToDispatchMountItemsOnAndroid(
facebook::jni::alias_ref<JReactNativeFeatureFlagsCxxInterop> /*unused*/) {
return ReactNativeFeatureFlags::removeNestedCallsToDispatchMountItemsOnAndroid();
}

bool JReactNativeFeatureFlagsCxxInterop::setAndroidLayoutDirection(
facebook::jni::alias_ref<JReactNativeFeatureFlagsCxxInterop> /*unused*/) {
return ReactNativeFeatureFlags::setAndroidLayoutDirection();
Expand Down Expand Up @@ -707,6 +718,9 @@ void JReactNativeFeatureFlagsCxxInterop::registerNatives() {
makeNativeMethod(
"loadVectorDrawablesOnImages",
JReactNativeFeatureFlagsCxxInterop::loadVectorDrawablesOnImages),
makeNativeMethod(
"removeNestedCallsToDispatchMountItemsOnAndroid",
JReactNativeFeatureFlagsCxxInterop::removeNestedCallsToDispatchMountItemsOnAndroid),
makeNativeMethod(
"setAndroidLayoutDirection",
JReactNativeFeatureFlagsCxxInterop::setAndroidLayoutDirection),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<<a0630578504b5c2c3ce8d7d8621bc650>>
* @generated SignedSource<<70edc4176da74aed5eaa1561a907a7ee>>
*/

/**
Expand Down Expand Up @@ -138,6 +138,9 @@ class JReactNativeFeatureFlagsCxxInterop
static bool loadVectorDrawablesOnImages(
facebook::jni::alias_ref<JReactNativeFeatureFlagsCxxInterop>);

static bool removeNestedCallsToDispatchMountItemsOnAndroid(
facebook::jni::alias_ref<JReactNativeFeatureFlagsCxxInterop>);

static bool setAndroidLayoutDirection(
facebook::jni::alias_ref<JReactNativeFeatureFlagsCxxInterop>);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<<d3deef1be546ac4cd55512c8a8c1972e>>
* @generated SignedSource<<80b130f32b0bc59b9a9605777537feeb>>
*/

/**
Expand Down Expand Up @@ -165,6 +165,10 @@ bool ReactNativeFeatureFlags::loadVectorDrawablesOnImages() {
return getAccessor().loadVectorDrawablesOnImages();
}

bool ReactNativeFeatureFlags::removeNestedCallsToDispatchMountItemsOnAndroid() {
return getAccessor().removeNestedCallsToDispatchMountItemsOnAndroid();
}

bool ReactNativeFeatureFlags::setAndroidLayoutDirection() {
return getAccessor().setAndroidLayoutDirection();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<<8d28b4f3ffac32a287c9b858d62e39e1>>
* @generated SignedSource<<3b5ff29faaf1575c051131bdf24a3237>>
*/

/**
Expand Down Expand Up @@ -217,6 +217,11 @@ class ReactNativeFeatureFlags {
*/
RN_EXPORT static bool loadVectorDrawablesOnImages();

/**
* Removes nested calls to MountItemDispatcher.dispatchMountItems on Android, so we do less work per frame on the UI thread.
*/
RN_EXPORT static bool removeNestedCallsToDispatchMountItemsOnAndroid();

/**
* Propagate layout direction to Android views.
*/
Expand Down
Loading

0 comments on commit a5dd56f

Please sign in to comment.