From 70447775f7b688553fe3fc30c28ab6d5a24c5eb0 Mon Sep 17 00:00:00 2001 From: David Vacca Date: Sun, 3 Mar 2019 08:55:26 -0800 Subject: [PATCH] Update Props during pre-allocation avoiding re-updating the same props during mounting Summary: Update Props during pre-allocation avoiding re-updating the same props during mounting MobileLab test showed an improvement of: MARKETPLACE_YOU_TTI_SUCCESS: -3.34% = 58ms Reviewed By: shergin Differential Revision: D14252170 fbshipit-source-id: 1f4e9ad5dcecbc06651fa065135ffeed4892d984 --- React/Fabric/RCTScheduler.mm | 36 +++++++++---------- .../react/fabric/FabricUIManager.java | 19 +++++----- .../facebook/react/fabric/jsi/jni/Binding.cpp | 24 ++++++------- .../fabric/mounting/MountingManager.java | 9 ++--- .../mounting/mountitems/CreateMountItem.java | 9 ++++- ReactCommon/fabric/uimanager/Scheduler.cpp | 4 +-- 6 files changed, 53 insertions(+), 48 deletions(-) diff --git a/React/Fabric/RCTScheduler.mm b/React/Fabric/RCTScheduler.mm index 2b9961e395e..e47ba28dfd7 100644 --- a/React/Fabric/RCTScheduler.mm +++ b/React/Fabric/RCTScheduler.mm @@ -19,31 +19,34 @@ using namespace facebook::react; -class SchedulerDelegateProxy: public SchedulerDelegate { -public: - SchedulerDelegateProxy(void *scheduler): - scheduler_(scheduler) {} +class SchedulerDelegateProxy : public SchedulerDelegate { + public: + SchedulerDelegateProxy(void *scheduler) : scheduler_(scheduler) {} - void schedulerDidFinishTransaction(Tag rootTag, const ShadowViewMutationList &mutations, const long commitStartTime, const long layoutTime) override { + void schedulerDidFinishTransaction( + Tag rootTag, + const ShadowViewMutationList &mutations, + const long commitStartTime, + const long layoutTime) override + { RCTScheduler *scheduler = (__bridge RCTScheduler *)scheduler_; [scheduler.delegate schedulerDidFinishTransaction:mutations rootTag:rootTag]; } void schedulerDidRequestPreliminaryViewAllocation( SurfaceId surfaceId, - ComponentName componentName, bool isLayoutable, - ComponentHandle componentHandle) override + const ShadowView &shadowView) override { if (!isLayoutable) { return; } RCTScheduler *scheduler = (__bridge RCTScheduler *)scheduler_; - [scheduler.delegate schedulerOptimisticallyCreateComponentViewWithComponentHandle:componentHandle]; + [scheduler.delegate schedulerOptimisticallyCreateComponentViewWithComponentHandle:shadowView.componentHandle]; } -private: + private: void *scheduler_; }; @@ -56,7 +59,8 @@ private: { if (self = [super init]) { _delegateProxy = std::make_shared((__bridge void *)self); - _scheduler = std::make_shared(std::static_pointer_cast(contextContainer), getDefaultComponentRegistryFactory()); + _scheduler = std::make_shared( + std::static_pointer_cast(contextContainer), getDefaultComponentRegistryFactory()); _scheduler->setDelegate(_delegateProxy.get()); } @@ -77,17 +81,9 @@ private: SystraceSection s("-[RCTScheduler startSurfaceWithSurfaceId:...]"); auto props = convertIdToFollyDynamic(initialProps); - _scheduler->startSurface( - surfaceId, - RCTStringFromNSString(moduleName), - props, - layoutConstraints, - layoutContext); + _scheduler->startSurface(surfaceId, RCTStringFromNSString(moduleName), props, layoutConstraints, layoutContext); _scheduler->renderTemplateToSurface( - surfaceId, - props.getDefault("navigationConfig") - .getDefault("initialUITemplate", "") - .getString()); + surfaceId, props.getDefault("navigationConfig").getDefault("initialUITemplate", "").getString()); } - (void)stopSurfaceWithSurfaceId:(SurfaceId)surfaceId diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java b/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java index 93426732c6e..9b58679f448 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java @@ -21,7 +21,6 @@ import android.support.annotation.UiThread; import android.util.Log; import android.view.View; import com.facebook.common.logging.FLog; -import com.facebook.infer.annotation.Assertions; import com.facebook.infer.annotation.ThreadConfined; import com.facebook.proguard.annotations.DoNotStrip; import com.facebook.react.bridge.LifecycleEventListener; @@ -73,6 +72,7 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { private static final Map sComponentNames = new HashMap<>(); private static final int FRAME_TIME_MS = 16; private static final int MAX_TIME_IN_FRAME_FOR_NON_BATCHED_OPERATIONS_MS = 8; + private static final int PRE_MOUNT_ITEMS_INITIAL_SIZE_ARRAY = 250; static { FabricSoLoader.staticInit(); @@ -105,7 +105,8 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { private List mMountItems = new ArrayList<>(); @GuardedBy("mPreMountItemsLock") - private ArrayDeque mPreMountItems = new ArrayDeque<>(); + private ArrayDeque mPreMountItems = new ArrayDeque<>( + PRE_MOUNT_ITEMS_INITIAL_SIZE_ARRAY); @ThreadConfined(UI) private final DispatchUIFrameCallback mDispatchUIFrameCallback; @@ -165,7 +166,7 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { @DoNotStrip @SuppressWarnings("unused") private MountItem createMountItem( - String componentName, int reactRootTag, int reactTag, boolean isVirtual) { + String componentName, int reactRootTag, int reactTag, boolean isVirtual, ReadableMap props) { String component = sComponentNames.get(componentName); if (component == null) { throw new IllegalArgumentException("Unable to find component with name " + componentName); @@ -174,7 +175,7 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { if (reactContext == null) { throw new IllegalArgumentException("Unable to find ReactContext for root: " + reactRootTag); } - return new CreateMountItem(reactContext, component, reactTag, isVirtual); + return new CreateMountItem(reactContext, component, reactTag, isVirtual, props); } @Override @@ -197,11 +198,10 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { // There is no reason to allocate views ahead of time on the main thread. return; } + + ThemedReactContext context = mReactContextForRootTag.get(rootTag); + String component = sComponentNames.get(componentName); synchronized (mPreMountItemsLock) { - ThemedReactContext context = - Assertions.assertNotNull(mReactContextForRootTag.get(rootTag)); - String component = sComponentNames.get(componentName); - Assertions.assertNotNull(component); mPreMountItems.add(new PreAllocateViewMountItem(context, rootTag, reactTag, component, props)); } } @@ -315,6 +315,9 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { mRunStartTime = SystemClock.uptimeMillis(); List mountItemsToDispatch; synchronized (mMountItemsLock) { + if (mMountItems.isEmpty()) { + return; + } mountItemsToDispatch = mMountItems; mMountItems = new ArrayList<>(); } diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/jsi/jni/Binding.cpp b/ReactAndroid/src/main/java/com/facebook/react/fabric/jsi/jni/Binding.cpp index b0775e10721..2d6143a3d06 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/jsi/jni/Binding.cpp +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/jsi/jni/Binding.cpp @@ -173,7 +173,7 @@ local_ref createCreateMountItem( const Tag rootTag) { static auto createJavaInstruction = jni::findClassStatic(UIManagerJavaDescriptor) - ->getMethod(jstring, jint, jint, jboolean)>( + ->getMethod(jstring, jint, jint, jboolean, ReadableMap::javaobject)>( "createMountItem"); auto newChildShadowView = mutation.newChildShadowView; @@ -183,12 +183,16 @@ local_ref createCreateMountItem( jboolean isVirtual = newChildShadowView.layoutMetrics == EmptyLayoutMetrics; + local_ref props = isVirtual ? nullptr : + castReadableMap(ReadableNativeMap::newObjectCxxArgs(newChildShadowView.props->rawProps)); + return createJavaInstruction( javaUIManager, componentName.get(), rootTag, newChildShadowView.tag, - isVirtual); + isVirtual, + props != nullptr ? props.get() : nullptr); } local_ref createUpdateEventEmitterMountItem( @@ -402,7 +406,7 @@ void Binding::schedulerDidFinishTransaction( mountItems[position++] = createInsertMountItem(javaUIManager_, mutation); - if (mutation.newChildShadowView.props->revision != 0) { + if (mutation.newChildShadowView.props->revision > 1) { mountItems[position++] = createUpdatePropsMountItem(javaUIManager_, mutation); } @@ -465,20 +469,16 @@ void Binding::schedulerDidRequestPreliminaryViewAllocation( const SurfaceId surfaceId, bool isLayoutable, const ShadowView &shadowView) { + if (isLayoutable) { static auto preallocateView = jni::findClassStatic(UIManagerJavaDescriptor) ->getMethod("preallocateView"); - if (shadowView.props->rawProps.isNull()) { - local_ref readableMap = - castReadableMap(ReadableNativeMap::newObjectCxxArgs(shadowView.props->rawProps)); - preallocateView( - javaUIManager_, surfaceId, shadowView.tag, make_jstring(shadowView.componentName).get(), readableMap.get()); - } else { - preallocateView( - javaUIManager_, surfaceId, shadowView.tag, make_jstring(shadowView.componentName).get(), nullptr); - } + local_ref readableMap = + castReadableMap(ReadableNativeMap::newObjectCxxArgs(shadowView.props->rawProps)); + preallocateView( + javaUIManager_, surfaceId, shadowView.tag, make_jstring(shadowView.componentName).get(), readableMap.get()); } } diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountingManager.java b/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountingManager.java index 02143b21835..2ffaa4d8d01 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountingManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountingManager.java @@ -168,8 +168,8 @@ public class MountingManager { ThemedReactContext themedReactContext, String componentName, int reactTag, + boolean isVirtual) { - UiThreadUtil.assertOnUiThread(); View view = null; ViewManager viewManager = null; @@ -280,10 +280,11 @@ public class MountingManager { String componentName, int reactTag, ReadableMap props) { + + if (mTagToViewState.get(reactTag) != null) return; + createView(reactContext, componentName, reactTag, false); - if (props != null) { - updateProps(reactTag, props); - } + updateProps(reactTag, props); } @UiThread diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/CreateMountItem.java b/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/CreateMountItem.java index 58e77d8b0ca..4106999cdc5 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/CreateMountItem.java +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/CreateMountItem.java @@ -6,6 +6,7 @@ */ package com.facebook.react.fabric.mounting.mountitems; +import com.facebook.react.bridge.ReadableMap; import com.facebook.react.fabric.mounting.MountingManager; import com.facebook.react.uimanager.ThemedReactContext; @@ -15,21 +16,27 @@ public class CreateMountItem implements MountItem { private final int mReactTag; private final ThemedReactContext mThemedReactContext; private final boolean mIsVirtual; + private final ReadableMap mProps; public CreateMountItem( ThemedReactContext themedReactContext, String componentName, int reactTag, - boolean isVirtual) { + boolean isVirtual, + ReadableMap props) { mReactTag = reactTag; mThemedReactContext = themedReactContext; mComponentName = componentName; mIsVirtual = isVirtual; + mProps = props; } @Override public void execute(MountingManager mountingManager) { mountingManager.createView(mThemedReactContext, mComponentName, mReactTag, mIsVirtual); + if (mProps != null && !mIsVirtual) { + mountingManager.updateProps(mReactTag, mProps); + } } public String getComponentName() { diff --git a/ReactCommon/fabric/uimanager/Scheduler.cpp b/ReactCommon/fabric/uimanager/Scheduler.cpp index 81e1480fb75..1dd618b290c 100644 --- a/ReactCommon/fabric/uimanager/Scheduler.cpp +++ b/ReactCommon/fabric/uimanager/Scheduler.cpp @@ -261,9 +261,7 @@ void Scheduler::uiManagerDidCreateShadowNode( auto shadowView = ShadowView(*shadowNode); delegate_->schedulerDidRequestPreliminaryViewAllocation( - shadowNode->getRootTag(), - isLayoutable, - shadowView); + shadowNode->getRootTag(), isLayoutable, shadowView); } }