From b4dab1a537a8fc7919bc9666083580d90da0c024 Mon Sep 17 00:00:00 2001 From: Xin Chen Date: Mon, 20 Dec 2021 18:42:27 -0800 Subject: [PATCH] Setup mobile config and create experiment for using overflowInset in Android Summary: The previous diff shows how `overflowInset` could improve hit test algorithm performance. This diff adds experiment to it in order to: 1. Understand the perf improvement in production 2. Provide quick way to rollback in production -- the changes are used by FB4A as well as VR 3. The changes will pass more instructions over JNI, which may have impact on perf. To share the MC param values in both Java and C++ side, the value is hosted by `ReactFeatureFlags` and fetched in `FbReactInstanceHolder`. Changelog: [Internal] Reviewed By: mdvacca Differential Revision: D33179310 fbshipit-source-id: 327100d41f7b5a668ff0d2afabcdd1fc16cb5a18 --- .../react/config/ReactFeatureFlags.java | 11 ++++++++++ .../fabric/jni/FabricMountingManager.cpp | 21 +++++++++++++++---- .../react/fabric/jni/FabricMountingManager.h | 4 ++++ .../react/uimanager/TouchTargetHelper.java | 4 +++- 4 files changed, 35 insertions(+), 5 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java b/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java index 2136c6cce57..2d2af6db441 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java +++ b/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java @@ -94,6 +94,17 @@ public class ReactFeatureFlags { return mapBufferSerializationEnabled; } + /** Feature Flag to use overflowInset values provided by Yoga */ + private static boolean useOverflowInset = false; + + public static void setUseOverflowInset(boolean enabled) { + useOverflowInset = enabled; + } + + public static boolean doesUseOverflowInset() { + return useOverflowInset; + } + /** Enables Fabric for LogBox */ public static boolean enableFabricInLogBox = false; diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/FabricMountingManager.cpp b/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/FabricMountingManager.cpp index 68bf552f04e..65a1ba4eb5c 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/FabricMountingManager.cpp +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/FabricMountingManager.cpp @@ -307,8 +307,9 @@ void FabricMountingManager::executeMount( // children of the current view. The layout of current view may not // change, and we separate this part from layout mount items to not // pack too much data there. - if (oldChildShadowView.layoutMetrics.overflowInset != - newChildShadowView.layoutMetrics.overflowInset) { + if (useOverflowInset_ && + (oldChildShadowView.layoutMetrics.overflowInset != + newChildShadowView.layoutMetrics.overflowInset)) { cppUpdateOverflowInsetMountItems.push_back( CppMountItem::UpdateOverflowInsetMountItem(newChildShadowView)); } @@ -355,8 +356,10 @@ void FabricMountingManager::executeMount( // children of the current view. The layout of current view may not // change, and we separate this part from layout mount items to not // pack too much data there. - cppUpdateOverflowInsetMountItems.push_back( - CppMountItem::UpdateOverflowInsetMountItem(newChildShadowView)); + if (useOverflowInset_) { + cppUpdateOverflowInsetMountItems.push_back( + CppMountItem::UpdateOverflowInsetMountItem(newChildShadowView)); + } } // EventEmitter @@ -860,6 +863,15 @@ void FabricMountingManager::onAllAnimationsComplete() { allAnimationsCompleteJNI(javaUIManager_); } +bool doesUseOverflowInset() { + static const auto reactFeatureFlagsJavaDescriptor = jni::findClassStatic( + FabricMountingManager::ReactFeatureFlagsJavaDescriptor); + static const auto doesUseOverflowInset = + reactFeatureFlagsJavaDescriptor->getStaticMethod( + "doesUseOverflowInset"); + return doesUseOverflowInset(reactFeatureFlagsJavaDescriptor); +} + FabricMountingManager::FabricMountingManager( std::shared_ptr &config, global_ref &javaUIManager) @@ -872,6 +884,7 @@ FabricMountingManager::FabricMountingManager( config->getBool("react_fabric:disabled_view_preallocation_android"); disableRevisionCheckForPreallocation_ = config->getBool("react_fabric:disable_revision_check_for_preallocation"); + useOverflowInset_ = doesUseOverflowInset(); } } // namespace react diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/FabricMountingManager.h b/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/FabricMountingManager.h index e95ad1ef173..980060c1d0b 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/FabricMountingManager.h +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/FabricMountingManager.h @@ -28,6 +28,9 @@ class FabricMountingManager { constexpr static auto UIManagerJavaDescriptor = "com/facebook/react/fabric/FabricUIManager"; + constexpr static auto ReactFeatureFlagsJavaDescriptor = + "com/facebook/react/config/ReactFeatureFlags"; + FabricMountingManager( std::shared_ptr &config, jni::global_ref &javaUIManager); @@ -65,6 +68,7 @@ class FabricMountingManager { bool enableEarlyEventEmitterUpdate_{false}; bool disablePreallocateViews_{false}; bool disableRevisionCheckForPreallocation_{false}; + bool useOverflowInset_{false}; }; } // namespace react diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/TouchTargetHelper.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/TouchTargetHelper.java index 9a1bc71f885..68338ffcf97 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/TouchTargetHelper.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/TouchTargetHelper.java @@ -17,6 +17,7 @@ import android.view.ViewGroup; import androidx.annotation.Nullable; import com.facebook.react.bridge.JSApplicationIllegalArgumentException; import com.facebook.react.bridge.UiThreadUtil; +import com.facebook.react.config.ReactFeatureFlags; import com.facebook.react.touch.ReactHitSlopView; import java.util.ArrayList; import java.util.EnumSet; @@ -185,7 +186,8 @@ public class TouchTargetHelper { if (view instanceof ReactOverflowViewWithInset) { // If the touch point is outside of the overflowinset for the view, we can safely ignore // it. - if (!isTouchPointInViewWithOverflowInset(eventCoords[0], eventCoords[1], view)) { + if (ReactFeatureFlags.doesUseOverflowInset() + && !isTouchPointInViewWithOverflowInset(eventCoords[0], eventCoords[1], view)) { return null; }