Generated from a GitHub Pull Request. Run 'jf sync' on this diff to load the correct commit data.

Differential Revision: D124966304

Author: bartlomiejbloniarz

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java

@@ -67,7 +67,6 @@
 import com.facebook.react.fabric.mounting.mountitems.MountItem;
 import com.facebook.react.fabric.mounting.mountitems.MountItemFactory;
 import com.facebook.react.fabric.mounting.mountitems.PrefetchResourcesMountItem;
-import com.facebook.react.fabric.mounting.mountitems.PullTransactionMountItem;
 import com.facebook.react.fabric.mounting.mountitems.SynchronousMountItem;
 import com.facebook.react.internal.featureflags.ReactNativeFeatureFlags;
 import com.facebook.react.internal.featureflags.ReactNativeNewArchitectureFeatureFlags;
@@ -237,6 +236,11 @@ public void executeItems(Queue<MountItem> items) {
           // This executor can be technically accessed before the dispatcher is created,
           // but if that happens, something is terribly wrong
           mMountItemDispatcher.dispatchMountItems(items);
+          // Only called when a surface's root view attaches, before the surface counts as attached,
+          // so the dispatch above skips its pull. Pull model: the next frame pulls it.
+          if (mMountItemDispatcher.hasSurfaceIdsToPull()) {
+            mDispatchUIFrameCallback.schedule();
+          }
         }
       };
 
@@ -1015,15 +1019,9 @@ private void scheduleMountItem(
     if (shouldSchedule) {
       Assertions.assertNotNull(mountItem, "MountItem is null");
       if (synchronous) {
-        if (mMountingManager.isWaitingForViewAttach(mountItem.getSurfaceId())) {
-          // Regular mount items are still being deferred into the surface's attach queue.
-          // Executing this batch inline would run it ahead of the preallocations queued there.
-          mMountItemDispatcher.addMountItem(mountItem);
-        } else {
-          // Pull model: we are already on the UI thread, inside the dispatcher's loop executing
-          // a PullTransactionMountItem. We don't schedule the item, we execute it directly.
-          mountItem.execute(mMountingManager);
-        }
+        // Pull model: we are already on the UI thread, inside the dispatcher's pull of a surface
+        // whose root view is attached. We don't schedule the item, we execute it directly.
+        mountItem.execute(mMountingManager);
       } else {
         mMountItemDispatcher.addMountItem(mountItem);
         if (UiThreadUtil.isOnUiThread()) {
@@ -1073,19 +1071,18 @@ public void runGuarded() {
 
   /**
    * Pull model: called from C++ via JNI (usually on the commit thread) to signal that a transaction
-   * is available for {@code surfaceId}. Enqueues a PullTransactionMountItem so the UI thread pulls
-   * and applies the transaction itself, preserving mount-item ordering.
+   * is available for {@code surfaceId}. The next dispatch on the UI thread pulls and applies it
+   * before the queued mount items.
    */
   @SuppressWarnings("unused")
   @AnyThread
   @ThreadConfined(ANY)
   @VisibleForTesting
   void onTransactionAvailable(int surfaceId) {
-    FabricUIManagerBinding binding = mBinding;
-    if (binding == null) {
+    if (mBinding == null) {
       return;
     }
-    mMountItemDispatcher.addMountItem(new PullTransactionMountItem(surfaceId, binding));
+    mMountItemDispatcher.addSurfaceIdToPull(surfaceId);
     if (UiThreadUtil.isOnUiThread()) {
       mMountItemDispatcher.tryDispatchMountItems();
     }
@@ -1550,16 +1547,14 @@ public void didMountItems(@Nullable List<? extends MountItem> mountItems) {
         listener.didMountItems(FabricUIManager.this);
       }
 
-      if (mountItems == null || mountItems.isEmpty()) {
-        return;
-      }
-
-      // Collect surface IDs for all the mount items
-      for (MountItem mountItem : mountItems) {
-        if (mountItem != null
-            && mountItem.getSurfaceId() != View.NO_ID
-            && !mSurfaceIdsWithPendingMountNotification.contains(mountItem.getSurfaceId())) {
-          mSurfaceIdsWithPendingMountNotification.add(mountItem.getSurfaceId());
+      if (mountItems != null) {
+        // Collect surface IDs for all the mount items
+        for (MountItem mountItem : mountItems) {
+          if (mountItem != null
+              && mountItem.getSurfaceId() != View.NO_ID
+              && !mSurfaceIdsWithPendingMountNotification.contains(mountItem.getSurfaceId())) {
+            mSurfaceIdsWithPendingMountNotification.add(mountItem.getSurfaceId());
+          }
         }
       }
 
@@ -1590,6 +1585,20 @@ public void didMountItems(@Nullable List<? extends MountItem> mountItems) {
       }
     }
 
+    @UiThread
+    @ThreadConfined(UI)
+    @Override
+    public void pullTransaction(int surfaceId) {
+      FabricUIManagerBinding binding = mBinding;
+      if (binding == null) {
+        return;
+      }
+      binding.pullAndExecuteTransaction(surfaceId);
+      if (!mSurfaceIdsWithPendingMountNotification.contains(surfaceId)) {
+        mSurfaceIdsWithPendingMountNotification.add(surfaceId);
+      }
+    }
+
     @Override
     public void didDispatchMountItems() {
       for (UIManagerListener listener : mListeners) {

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountItemDispatcher.kt

@@ -24,6 +24,7 @@ import com.facebook.react.internal.featureflags.ReactNativeFeatureFlags
 import com.facebook.react.internal.tracing.PerformanceTracer
 import com.facebook.systrace.Systrace
 import java.util.Queue
+import java.util.concurrent.ConcurrentHashMap
 import java.util.concurrent.ConcurrentLinkedQueue
 
 internal class MountItemDispatcher(
@@ -33,10 +34,20 @@ internal class MountItemDispatcher(
   private val viewCommandMountItems: Queue<DispatchCommandMountItem> = ConcurrentLinkedQueue()
   private val mountItems: Queue<MountItem> = ConcurrentLinkedQueue()
   private val preMountItems: Queue<MountItem> = ConcurrentLinkedQueue()
+  private val surfaceIdsToPull: MutableSet<Int> = ConcurrentHashMap.newKeySet()
 
-  /** @return true if any mount items, pre-mount items or view commands are still pending */
+  /**
+   * @return true if any mount items, pre-mount items, view commands or pulls of surfaces with an
+   *   attached root view are still pending. The attach schedules the frame that pulls its surface.
+   */
   fun hasPendingItems(): Boolean =
-      !viewCommandMountItems.isEmpty() || !mountItems.isEmpty() || !preMountItems.isEmpty()
+      !viewCommandMountItems.isEmpty() ||
+          !mountItems.isEmpty() ||
+          !preMountItems.isEmpty() ||
+          surfaceIdsToPull.any { !mountingManager.isWaitingForViewAttach(it) }
+
+  /** Pull model: @return true if any surface has a transaction waiting to be pulled. */
+  fun hasSurfaceIdsToPull(): Boolean = surfaceIdsToPull.isNotEmpty()
 
   // Items can be queued from any thread while the DISPATCH_UI frame callback is disarmed at
   // idle (see FabricUIManager's doFrameGuarded); the listener re-arms it.
@@ -69,6 +80,12 @@ internal class MountItemDispatcher(
     notifyItemsQueued()
   }
 
+  /** Pull model: a pull mounts the surface's latest revision, so it is pending at most once. */
+  fun addSurfaceIdToPull(surfaceId: Int) {
+    surfaceIdsToPull.add(surfaceId)
+    notifyItemsQueued()
+  }
+
   fun addPreAllocateMountItem(mountItem: MountItem) {
     // We do this check only for PreAllocateViewMountItem - and not DispatchMountItem or regular
     // MountItem - because PreAllocateViewMountItem are not batched, and is relatively more
@@ -147,9 +164,10 @@ internal class MountItemDispatcher(
    * Executes view commands, pre mount items and mount items in the respective order:
    * 1. View commands.
    * 2. Pre mount items.
-   * 3. Regular mount items.
+   * 3. Pulls of surfaces with an available transaction and an attached root view (pull model).
+   * 4. Regular mount items.
    *
-   * Does nothing if `viewCommandMountItemsToDispatch` and `mountItemsToDispatch` are empty.
+   * Does nothing if there are no view commands, mount items or surfaces to pull.
    * Nothing should call this directly except for `tryDispatchMountItems`.
    */
   @UiThread
@@ -160,8 +178,13 @@ internal class MountItemDispatcher(
 
     val viewCommandMountItemsToDispatch = getAndResetViewCommandMountItems()
     val mountItemsToDispatch = getAndResetMountItems()
+    val attachedSurfaceIdsToPull = getAttachedSurfaceIdsToPull()
 
-    if (mountItemsToDispatch == null && viewCommandMountItemsToDispatch == null) {
+    if (
+        mountItemsToDispatch == null &&
+            viewCommandMountItemsToDispatch == null &&
+            attachedSurfaceIdsToPull == null
+    ) {
       return
     }
 
@@ -256,7 +279,8 @@ internal class MountItemDispatcher(
       Systrace.endSection(Systrace.TRACE_TAG_REACT)
     }
 
-    mountItemsToDispatch?.let { items ->
+    if (mountItemsToDispatch != null || attachedSurfaceIdsToPull != null) {
+      val items = mountItemsToDispatch.orEmpty()
       Systrace.beginSection(
           Systrace.TRACE_TAG_REACT,
           "MountItemDispatcher::mountViews mountItems to execute",
@@ -269,6 +293,16 @@ internal class MountItemDispatcher(
           { ->
             val batchedExecutionStartTime = SystemClock.uptimeMillis()
 
+            for (surfaceId in attachedSurfaceIdsToPull.orEmpty()) {
+              surfaceIdsToPull.remove(surfaceId)
+              try {
+                itemDispatchListener.pullTransaction(surfaceId)
+              } catch (e: Throwable) {
+                FLog.e(TAG, "dispatchMountItems: caught exception pulling surface $surfaceId", e)
+                logOrRethrowMountingException(e, surfaceId)
+              }
+            }
+
             for (mountItem in items) {
               if (ReactNativeFeatureFlags.enableFabricLogs()) {
                 printMountItem(mountItem, "dispatchMountItems: Executing mountItem")
@@ -298,15 +332,7 @@ internal class MountItemDispatcher(
                   }
                 }
 
-                if (mountItem.getSurfaceId() != View.NO_ID) {
-                  mountingManager.getSurfaceManager(mountItem.getSurfaceId())?.printSurfaceState()
-                }
-
-                if (ReactIgnorableMountingException.isIgnorable(e)) {
-                  ReactSoftExceptionLogger.logSoftException(TAG, e)
-                } else {
-                  throw e
-                }
+                logOrRethrowMountingException(e, mountItem.getSurfaceId())
               }
             }
             batchedExecutionTime += SystemClock.uptimeMillis() - batchedExecutionStartTime
@@ -375,6 +401,18 @@ internal class MountItemDispatcher(
     Systrace.endSection(Systrace.TRACE_TAG_REACT)
   }
 
+  private fun logOrRethrowMountingException(e: Throwable, surfaceId: Int) {
+    if (surfaceId != View.NO_ID) {
+      mountingManager.getSurfaceManager(surfaceId)?.printSurfaceState()
+    }
+
+    if (ReactIgnorableMountingException.isIgnorable(e)) {
+      ReactSoftExceptionLogger.logSoftException(TAG, e)
+    } else {
+      throw e
+    }
+  }
+
   private fun executeOrEnqueue(item: MountItem) {
     if (mountingManager.isWaitingForViewAttach(item.getSurfaceId())) {
       if (ReactNativeFeatureFlags.enableFabricLogs()) {
@@ -408,13 +446,23 @@ internal class MountItemDispatcher(
   @UiThread
   private fun getAndResetPreMountItems(): List<MountItem>? = drainConcurrentItemQueue(preMountItems)
 
+  /** A surface waiting for its root view keeps its pull until the frame after the attach. */
+  @ThreadConfined(UI)
+  @UiThread
+  private fun getAttachedSurfaceIdsToPull(): List<Int>? =
+      if (surfaceIdsToPull.isEmpty()) null
+      else surfaceIdsToPull.filterNot(mountingManager::isWaitingForViewAttach).ifEmpty { null }
+
   interface ItemDispatchListener {
     fun willMountItems(mountItems: List<MountItem>?)
 
     fun didMountItems(mountItems: List<MountItem>?)
 
     fun didDispatchMountItems()
 
+    /** Pull model: pulls the surface's latest transaction and mounts it synchronously. */
+    fun pullTransaction(surfaceId: Int)
+
     /** Called (from any thread) whenever new items are queued into the dispatcher. */
     fun onItemsQueued()
   }

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/PullTransactionMountItem.kt

@@ -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<<eb0bafceba60cb439a4be3d68b890725>>
+ * @generated SignedSource<<7fbc80af8ddd69f5986f2c955272c334>>
  */
 
 /**
@@ -289,7 +289,7 @@ public object ReactNativeFeatureFlags {
   public fun enableModuleArgumentNSNullConversionIOS(): Boolean = accessor.enableModuleArgumentNSNullConversionIOS()
 
   /**
-   * When enabled, Android mounts transactions with the pull model (like iOS): the commit thread no longer pulls and builds the mount batch in schedulerShouldRenderTransactions. Instead the UI thread pulls the transaction itself via a PullTransactionMountItem enqueued in the MountItemDispatcher, builds the IntBufferBatchMountItem, and applies it synchronously. Requires `enableAccumulatedUpdatesInRawPropsAndroid` to be enabled as well, since a single pull may collapse several commits into one diff and therefore needs complete accumulated rawProps; when used together with Props 2.0, also enable `enableExclusivePropsUpdateAndroid` and `enablePropsUpdateReconciliationAndroid`.
+   * When enabled, Android mounts transactions with the pull model (like iOS): the commit thread no longer pulls and builds the mount batch in schedulerShouldRenderTransactions. Instead the UI thread pulls the transaction itself at its next mount dispatch, builds the IntBufferBatchMountItem, and applies it synchronously. Requires `enableAccumulatedUpdatesInRawPropsAndroid` to be enabled as well, since a single pull may collapse several commits into one diff and therefore needs complete accumulated rawProps; when used together with Props 2.0, also enable `enableExclusivePropsUpdateAndroid` and `enablePropsUpdateReconciliationAndroid`.
    */
   @JvmStatic
   public fun enableMountingCoordinatorPullModelAndroid(): Boolean = accessor.enableMountingCoordinatorPullModelAndroid()

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlags.kt

@@ -69,7 +69,7 @@ class FabricMountingManager final {
 
   /*
    * Pull model: notify Java that a transaction is available for `surfaceId` so
-   * the UI thread can pull it via a PullTransactionMountItem.
+   * the UI thread pulls it at its next mount dispatch.
    */
   void onTransactionAvailable(SurfaceId surfaceId);
 

packages/react-native/ReactAndroid/src/main/jni/react/fabric/FabricMountingManager.h

@@ -730,8 +730,8 @@ void FabricUIManagerBinding::schedulerShouldRenderTransactions(
   if (ReactNativeFeatureFlags::enableMountingCoordinatorPullModelAndroid()) {
     // Pull model: do NOT pull the transaction or build the batch here (on the
     // commit thread). Just notify Java that a transaction is available; the UI
-    // thread will pull it via a PullTransactionMountItem and call back into
-    // `pullAndExecuteTransaction`.
+    // thread calls back into `pullAndExecuteTransaction` at its next mount
+    // dispatch.
     mountingManager->onTransactionAvailable(
         mountingCoordinator->getSurfaceId());
   } else if (ReactNativeFeatureFlags::