From aa69e51096e84f0b09ec5d3d76f9d52a5eaa6e11 Mon Sep 17 00:00:00 2001 From: David Vacca Date: Sun, 3 Mar 2019 08:55:25 -0800 Subject: [PATCH] Preallocate Image views Summary: This diff changes the pre-allocation of Images update the props on the ImageViews during the creation of ShadowNodes instead of during rendering. The purpose of this change is to optimize TTI. This was originally landed in D14214844 and then backed out in D14247897 Reviewed By: shergin Differential Revision: D14286232 fbshipit-source-id: 8437f57f9473eb22ef98d80531b4020ee5fbb9ae --- .../facebook/react/fabric/FabricUIManager.java | 4 ++-- .../facebook/react/fabric/jsi/jni/Binding.cpp | 18 +++++++++++++----- .../facebook/react/fabric/jsi/jni/Binding.h | 3 +-- .../react/fabric/mounting/MountingManager.java | 15 +++++++++++++-- .../mountitems/PreAllocateViewMountItem.java | 9 +++++++-- ReactCommon/fabric/uimanager/Scheduler.cpp | 4 ++-- .../fabric/uimanager/SchedulerDelegate.h | 3 +-- 7 files changed, 39 insertions(+), 17 deletions(-) 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 95a97a89900..58cdcd7d970 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java @@ -188,7 +188,7 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { } @DoNotStrip - private void preallocateView(final int rootTag, final String componentName) { + private void preallocateView(int rootTag, int reactTag, final String componentName, ReadableMap props) { if (UiThreadUtil.isOnUiThread()) { // There is no reason to allocate views ahead of time on the main thread. return; @@ -198,7 +198,7 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { Assertions.assertNotNull(mReactContextForRootTag.get(rootTag)); String component = sComponentNames.get(componentName); Assertions.assertNotNull(component); - mPreMountItems.add(new PreAllocateViewMountItem(context, rootTag, component)); + mPreMountItems.add(new PreAllocateViewMountItem(context, rootTag, reactTag, component, props)); } } 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 a28c6388ce3..5e29c0cd4af 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 @@ -461,16 +461,24 @@ void Binding::setPixelDensity(float pointScaleFactor) { void Binding::schedulerDidRequestPreliminaryViewAllocation( const SurfaceId surfaceId, - const ComponentName componentName, bool isLayoutable, - const ComponentHandle componentHandle) { + const ShadowView &shadowView) { if (isLayoutable) { static auto preallocateView = jni::findClassStatic(UIManagerJavaDescriptor) - ->getMethod("preallocateView"); + ->getMethod("preallocateView"); - preallocateView( - javaUIManager_, surfaceId, make_jstring(componentName).get()); + // TODO: T31905686 Experiment to check what is the impact on TTI of pre loading of Images during + // pre-allocation of views + if (shadowView.componentName == "Image") { + 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); + } } } diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/jsi/jni/Binding.h b/ReactAndroid/src/main/java/com/facebook/react/fabric/jsi/jni/Binding.h index ad12aa53955..3e3d80fb6db 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/jsi/jni/Binding.h +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/jsi/jni/Binding.h @@ -64,9 +64,8 @@ class Binding : public jni::HybridClass, public SchedulerDelegate { void schedulerDidRequestPreliminaryViewAllocation( const SurfaceId surfaceId, - const ComponentName componentName, bool isLayoutable, - const ComponentHandle componentHandle); + const ShadowView &shadowView); void setPixelDensity(float pointScaleFactor); 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 299b0f00ab8..02143b21835 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 @@ -172,6 +172,10 @@ public class MountingManager { UiThreadUtil.assertOnUiThread(); View view = null; ViewManager viewManager = null; + + // This can be possible if the view was already pre-allocated + if (mTagToViewState.get(reactTag) != null) return; + if (!isVirtual) { viewManager = mViewManagerRegistry.get(componentName); view = mViewPool.getOrCreateView(componentName, themedReactContext); @@ -271,8 +275,15 @@ public class MountingManager { } @UiThread - public void preallocateView(ThemedReactContext reactContext, String componentName) { - mViewPool.createView(reactContext, componentName); + public void preallocateView( + ThemedReactContext reactContext, + String componentName, + int reactTag, + ReadableMap props) { + createView(reactContext, componentName, reactTag, false); + if (props != null) { + updateProps(reactTag, props); + } } @UiThread diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/PreAllocateViewMountItem.java b/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/PreAllocateViewMountItem.java index 858faf1fe6e..6ca1d21db15 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/PreAllocateViewMountItem.java +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/PreAllocateViewMountItem.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; @@ -16,17 +17,21 @@ public class PreAllocateViewMountItem implements MountItem { private final String mComponent; private final int mRootTag; + private final int mReactTag; + private final ReadableMap mProps; private final ThemedReactContext mContext; - public PreAllocateViewMountItem(ThemedReactContext context, int rootTag, String component){ + public PreAllocateViewMountItem(ThemedReactContext context, int rootTag, int reactTag, String component, ReadableMap props){ mContext = context; mComponent = component; mRootTag = rootTag; + mProps = props; + mReactTag = reactTag; } @Override public void execute(MountingManager mountingManager) { - mountingManager.preallocateView(mContext, mComponent); + mountingManager.preallocateView(mContext, mComponent, mReactTag, mProps); } @Override diff --git a/ReactCommon/fabric/uimanager/Scheduler.cpp b/ReactCommon/fabric/uimanager/Scheduler.cpp index e8904e531f5..81e1480fb75 100644 --- a/ReactCommon/fabric/uimanager/Scheduler.cpp +++ b/ReactCommon/fabric/uimanager/Scheduler.cpp @@ -259,11 +259,11 @@ void Scheduler::uiManagerDidCreateShadowNode( dynamic_cast(shadowNode.get()); auto isLayoutable = layoutableShadowNode != nullptr; + auto shadowView = ShadowView(*shadowNode); delegate_->schedulerDidRequestPreliminaryViewAllocation( shadowNode->getRootTag(), - shadowNode->getComponentName(), isLayoutable, - shadowNode->getComponentHandle()); + shadowView); } } diff --git a/ReactCommon/fabric/uimanager/SchedulerDelegate.h b/ReactCommon/fabric/uimanager/SchedulerDelegate.h index e62c31b7fa8..7976005c647 100644 --- a/ReactCommon/fabric/uimanager/SchedulerDelegate.h +++ b/ReactCommon/fabric/uimanager/SchedulerDelegate.h @@ -35,9 +35,8 @@ class SchedulerDelegate { */ virtual void schedulerDidRequestPreliminaryViewAllocation( SurfaceId surfaceId, - ComponentName componentName, bool isLayoutable, - ComponentHandle componentHandle) = 0; + const ShadowView &shadowView) = 0; virtual ~SchedulerDelegate() noexcept = default; };