Refactor preallocation of views (#36606)

Summary: Pull Request resolved: #36606 This diff refactors the preallocation of views to minimize visibility of PreAllocateViewMountItem this is not a backward compatible change because nobody should be using this class Changelog: [Internal] Internal Reviewed By: javache Differential Revision: D44115487 fbshipit-source-id: 2d00f9f5edbe0517c3c5fcef94290180fa418fb3
facebook · Mar 31, 2023 · ed448b5 · ed448b5
1 parent 817db84
commit ed448b5
Show file tree

Hide file tree

Showing 5 changed files with 28 additions and 14 deletions.
diff --git a/...es/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java b/...es/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java
@@ -62,7 +62,6 @@
 import com.facebook.react.fabric.mounting.mountitems.IntBufferBatchMountItem;
 import com.facebook.react.fabric.mounting.mountitems.MountItem;
 import com.facebook.react.fabric.mounting.mountitems.MountItemFactory;
-import com.facebook.react.fabric.mounting.mountitems.PreAllocateViewMountItem;
 import com.facebook.react.modules.core.ReactChoreographer;
 import com.facebook.react.modules.i18nmanager.I18nUtil;
 import com.facebook.react.uimanager.IllegalViewOperationException;
@@ -710,7 +709,7 @@ private void preallocateView(
       boolean isLayoutable) {
 
     mMountItemDispatcher.addPreAllocateMountItem(
-        new PreAllocateViewMountItem(
+        MountItemFactory.createPreAllocateViewMountItem(
             rootTag,
             reactTag,
             componentName,

diff --git a/...ve/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountItemDispatcher.java b/...ve/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountItemDispatcher.java
@@ -26,7 +26,6 @@
 import com.facebook.react.bridge.RetryableMountingLayerException;
 import com.facebook.react.fabric.mounting.mountitems.DispatchCommandMountItem;
 import com.facebook.react.fabric.mounting.mountitems.MountItem;
-import com.facebook.react.fabric.mounting.mountitems.PreAllocateViewMountItem;
 import com.facebook.systrace.Systrace;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -51,8 +50,7 @@ public class MountItemDispatcher {
   private final ConcurrentLinkedQueue<MountItem> mMountItems = new ConcurrentLinkedQueue<>();
 
   @NonNull
-  private final ConcurrentLinkedQueue<PreAllocateViewMountItem> mPreMountItems =
-      new ConcurrentLinkedQueue<>();
+  private final ConcurrentLinkedQueue<MountItem> mPreMountItems = new ConcurrentLinkedQueue<>();
 
   private boolean mInDispatch = false;
   private int mReDispatchCounter = 0;
@@ -74,9 +72,10 @@ public void addMountItem(MountItem mountItem) {
     mMountItems.add(mountItem);
   }
 
-  public void addPreAllocateMountItem(PreAllocateViewMountItem mountItem) {
+  public void addPreAllocateMountItem(MountItem mountItem) {
     // We do this check only for PreAllocateViewMountItem - and not DispatchMountItem or regular
-    // MountItem - because PreAllocateViewMountItem is not batched, and is relatively more expensive
+    // MountItem - because PreAllocateViewMountItems are not batched, and is relatively more
+    // expensive
     // both to queue, to drain, and to execute.
     if (!mMountingManager.surfaceIsStopped(mountItem.getSurfaceId())) {
       mPreMountItems.add(mountItem);
@@ -245,13 +244,13 @@ private boolean dispatchMountItems() {
 
     // If there are MountItems to dispatch, we make sure all the "pre mount items" are executed
     // first
-    Collection<PreAllocateViewMountItem> preMountItemsToDispatch = getAndResetPreMountItems();
+    Collection<MountItem> preMountItemsToDispatch = getAndResetPreMountItems();
 
     if (preMountItemsToDispatch != null) {
       Systrace.beginSection(
           Systrace.TRACE_TAG_REACT_JAVA_BRIDGE, "FabricUIManager::mountViews preMountItems");
 
-      for (PreAllocateViewMountItem preMountItem : preMountItemsToDispatch) {
+      for (MountItem preMountItem : preMountItemsToDispatch) {
         executeOrEnqueue(preMountItem);
       }
 
@@ -319,7 +318,7 @@ public void dispatchPreMountItems(long frameTimeNanos) {
           break;
         }
 
-        PreAllocateViewMountItem preMountItemToDispatch = mPreMountItems.poll();
+        MountItem preMountItemToDispatch = mPreMountItems.poll();
         // If list is empty, `poll` will return null, or var will never be set
         if (preMountItemToDispatch == null) {
           break;
@@ -393,7 +392,7 @@ private List<MountItem> getAndResetMountItems() {
     return drainConcurrentItemQueue(mMountItems);
   }
 
-  private Collection<PreAllocateViewMountItem> getAndResetPreMountItems() {
+  private Collection<MountItem> getAndResetPreMountItems() {
     return drainConcurrentItemQueue(mPreMountItems);
   }
 

diff --git a/...ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/SurfaceMountingManager.java b/...ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/SurfaceMountingManager.java
@@ -1202,7 +1202,7 @@ public void deleteView(int reactTag) {
 
   @UiThread
   public void preallocateView(
-      String componentName,
+      @NonNull String componentName,
       int reactTag,
       @Nullable Object props,
       @Nullable StateWrapper stateWrapper,

diff --git a/...Android/src/main/java/com/facebook/react/fabric/mounting/mountitems/MountItemFactory.java b/...Android/src/main/java/com/facebook/react/fabric/mounting/mountitems/MountItemFactory.java
@@ -10,6 +10,8 @@
 import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
 import com.facebook.react.bridge.ReadableArray;
+import com.facebook.react.fabric.events.EventEmitterWrapper;
+import com.facebook.react.uimanager.StateWrapper;
 
 /** Factory class that expose creation of {@link MountItem} */
 public class MountItemFactory {
@@ -31,4 +33,17 @@ public static MountItem createSendAccessibilityEventMountItem(
       int surfaceId, int reactTag, int eventType) {
     return new SendAccessibilityEventMountItem(surfaceId, reactTag, eventType);
   }
+
+  /** @return a {@link MountItem} that will be used to preallocate views */
+  public static MountItem createPreAllocateViewMountItem(
+      int surfaceId,
+      int reactTag,
+      @NonNull String component,
+      @Nullable Object props,
+      @Nullable StateWrapper stateWrapper,
+      @Nullable EventEmitterWrapper eventEmitterWrapper,
+      boolean isLayoutable) {
+    return new PreAllocateViewMountItem(
+        surfaceId, reactTag, component, props, stateWrapper, eventEmitterWrapper, isLayoutable);
+  }
 }
diff --git a/...src/main/java/com/facebook/react/fabric/mounting/mountitems/PreAllocateViewMountItem.java b/...src/main/java/com/facebook/react/fabric/mounting/mountitems/PreAllocateViewMountItem.java
@@ -30,12 +30,12 @@ public class PreAllocateViewMountItem implements MountItem {
   private final @Nullable EventEmitterWrapper mEventEmitterWrapper;
   private final boolean mIsLayoutable;
 
-  public PreAllocateViewMountItem(
+  PreAllocateViewMountItem(
       int surfaceId,
       int reactTag,
       @NonNull String component,
       @Nullable Object props,
-      @NonNull StateWrapper stateWrapper,
+      @Nullable StateWrapper stateWrapper,
       @Nullable EventEmitterWrapper eventEmitterWrapper,
       boolean isLayoutable) {
     mComponent = getFabricComponentName(component);
@@ -66,6 +66,7 @@ public void execute(@NonNull MountingManager mountingManager) {
   }
 
   @Override
+  @NonNull
   public String toString() {
     StringBuilder result =
         new StringBuilder("PreAllocateViewMountItem [")