Skip to content

Commit b9f9945

Browse files
Pull model: pull each surface's pending transaction at the start of the mount phase (#59029)
Summary: With `enableMountingCoordinatorPullModelAndroid`, each commit notification queues a `PullTransactionMountItem` in `MountItemDispatcher`. `MountingCoordinator::pullTransaction` mounts the latest revision, so a surface's first queued item mounts every pending commit and its later items pull nothing. The item's queue position only reflects when the notification arrived. The queued item breaks root view attach. While the root view is detached, the item waits in the surface's attach queue, ahead of the preallocations for the revisions it pulls. #58152 handles this by re-queueing the pulled batch on the dispatcher. That puts the batch behind a later pull of the same surface, so the newer revision mounts first. This PR replaces the queued item with the set of surfaces that have a transaction available. A dispatch runs view commands, then pre-mount items, then one `FabricUIManagerBinding.pullAndExecuteTransaction` per pending surface whose root view is attached, then the queued mount items. Pulls share the mount items' "mount" trace, their handling of ignorable mounting exceptions, and `reportMount`. A surface waiting for its root view stays pending. Its attach already runs through `MountingManager.MountItemExecutor`, which now also schedules a frame, and that frame pulls the surface after its preallocations. This removes the #58152 re-queue and `PullTransactionMountItem`. With `disableIdleMountItemFrameCallbackRearmAndroid`, an attached pending surface keeps the frame callback armed, like a queued item. When a dispatch has both a pending surface and queued mount items, it now pulls the surface first. On main, an item queued before the surface's first pull notification ran before the pull. Push behaves like main here: its batch is queued at the end of the JS task, behind the items queued during it. An item that targets a view the commit creates, such as an accessibility event or a `SynchronousMountItem`, now finds it. An item that targets a view the commit deletes now misses it, although that view is gone after this frame either way. It logs a soft exception and is dropped, or for a view command, retries once first. With the default flags, view commands still run before the pulls, as on main. ## Changelog: [ANDROID] [FIXED] - Pull model: fix batches applied out of order when a surface's root view attaches Test Plan: `FabricUIManagerPullModelTest` models each surface's `MountingCoordinator`: a pull mounts every revision committed since the previous pull, as one batch. ``` ./gradlew :packages:react-native:ReactAndroid:testDebugUnitTest \ --tests 'com.facebook.react.fabric.*' ``` | Test | Before | After | | --- | --- | --- | | `pullDeferredByRootAttach_isAppliedBeforeLaterPulls` | fails: revisions mount as 3, 1, 2 | passes | | `onTransactionAvailable_pullsEachPendingSurfaceOnce` | fails: pulls `[1, 2, 1, 3]` | passes | | `mountItemQueuedBeforeCommit_runsAfterThePull` | fails: the item runs before the batch | passes | | `pendingPullOfStoppedSurface_runsOnceAndIsDropped` | fails: the stop clears the queued pull, nothing is pulled | passes | | `viewCommandsInMountItemOrder_runAfterThePull` | fails: the command dispatched before the commit runs before the batch | passes | - The new tests for pre-mount items, `reportMount` and the frame callback with `disableIdleMountItemFrameCallbackRearmAndroid` pass before and after. One of them, a commit notified from the commit thread while a frame pulls another surface, fails if `hasPendingItems` ignores pending surfaces. - `onTransactionAvailable_enqueuesOnePullPerNotification` is now `onTransactionAvailable_pullsEachPendingSurfaceOnce`. `synchronousBatch_waitingForRootAttach_isDeferred` is removed, because a synchronous batch can no longer run while its root view is detached. - All `ReactAndroid` JVM tests pass. - A few one-off checks, not committed, passed on main and with this PR: commits on two surfaces, an item queued after a commit, and a dispatch without a commit. A push-model check also passed, with the pull flag on: batches queued around a root view attach apply in commit order. - RNTester debug on an Android emulator with `enableMountingCoordinatorPullModelAndroid`, `enableAccumulatedUpdatesInRawPropsAndroid` and `enablePreallocatedPropsDiffOnInsertAndroid`: FlatList, Image, Animated and Accessibility examples render and update, including after leaving and reopening the app. There were no crashes or mounting exceptions. Differential Revision: D124966304 Pulled By: bartlomiejbloniarz
1 parent 8aa97bb commit b9f9945

11 files changed

Lines changed: 443 additions & 114 deletions

File tree

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

Lines changed: 34 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,6 @@
6767
import com.facebook.react.fabric.mounting.mountitems.MountItem;
6868
import com.facebook.react.fabric.mounting.mountitems.MountItemFactory;
6969
import com.facebook.react.fabric.mounting.mountitems.PrefetchResourcesMountItem;
70-
import com.facebook.react.fabric.mounting.mountitems.PullTransactionMountItem;
7170
import com.facebook.react.fabric.mounting.mountitems.SynchronousMountItem;
7271
import com.facebook.react.internal.featureflags.ReactNativeFeatureFlags;
7372
import com.facebook.react.internal.featureflags.ReactNativeNewArchitectureFeatureFlags;
@@ -237,6 +236,11 @@ public void executeItems(Queue<MountItem> items) {
237236
// This executor can be technically accessed before the dispatcher is created,
238237
// but if that happens, something is terribly wrong
239238
mMountItemDispatcher.dispatchMountItems(items);
239+
// Only called when a surface's root view attaches, before the surface counts as attached,
240+
// so the dispatch above skips its pull. Pull model: the next frame pulls it.
241+
if (mMountItemDispatcher.hasSurfaceIdsToPull()) {
242+
mDispatchUIFrameCallback.schedule();
243+
}
240244
}
241245
};
242246

@@ -1015,15 +1019,9 @@ private void scheduleMountItem(
10151019
if (shouldSchedule) {
10161020
Assertions.assertNotNull(mountItem, "MountItem is null");
10171021
if (synchronous) {
1018-
if (mMountingManager.isWaitingForViewAttach(mountItem.getSurfaceId())) {
1019-
// Regular mount items are still being deferred into the surface's attach queue.
1020-
// Executing this batch inline would run it ahead of the preallocations queued there.
1021-
mMountItemDispatcher.addMountItem(mountItem);
1022-
} else {
1023-
// Pull model: we are already on the UI thread, inside the dispatcher's loop executing
1024-
// a PullTransactionMountItem. We don't schedule the item, we execute it directly.
1025-
mountItem.execute(mMountingManager);
1026-
}
1022+
// Pull model: we are already on the UI thread, inside the dispatcher's pull of a surface
1023+
// whose root view is attached. We don't schedule the item, we execute it directly.
1024+
mountItem.execute(mMountingManager);
10271025
} else {
10281026
mMountItemDispatcher.addMountItem(mountItem);
10291027
if (UiThreadUtil.isOnUiThread()) {
@@ -1073,19 +1071,18 @@ public void runGuarded() {
10731071

10741072
/**
10751073
* Pull model: called from C++ via JNI (usually on the commit thread) to signal that a transaction
1076-
* is available for {@code surfaceId}. Enqueues a PullTransactionMountItem so the UI thread pulls
1077-
* and applies the transaction itself, preserving mount-item ordering.
1074+
* is available for {@code surfaceId}. The next dispatch on the UI thread pulls and applies it
1075+
* before the queued mount items.
10781076
*/
10791077
@SuppressWarnings("unused")
10801078
@AnyThread
10811079
@ThreadConfined(ANY)
10821080
@VisibleForTesting
10831081
void onTransactionAvailable(int surfaceId) {
1084-
FabricUIManagerBinding binding = mBinding;
1085-
if (binding == null) {
1082+
if (mBinding == null) {
10861083
return;
10871084
}
1088-
mMountItemDispatcher.addMountItem(new PullTransactionMountItem(surfaceId, binding));
1085+
mMountItemDispatcher.addSurfaceIdToPull(surfaceId);
10891086
if (UiThreadUtil.isOnUiThread()) {
10901087
mMountItemDispatcher.tryDispatchMountItems();
10911088
}
@@ -1550,16 +1547,14 @@ public void didMountItems(@Nullable List<? extends MountItem> mountItems) {
15501547
listener.didMountItems(FabricUIManager.this);
15511548
}
15521549

1553-
if (mountItems == null || mountItems.isEmpty()) {
1554-
return;
1555-
}
1556-
1557-
// Collect surface IDs for all the mount items
1558-
for (MountItem mountItem : mountItems) {
1559-
if (mountItem != null
1560-
&& mountItem.getSurfaceId() != View.NO_ID
1561-
&& !mSurfaceIdsWithPendingMountNotification.contains(mountItem.getSurfaceId())) {
1562-
mSurfaceIdsWithPendingMountNotification.add(mountItem.getSurfaceId());
1550+
if (mountItems != null) {
1551+
// Collect surface IDs for all the mount items
1552+
for (MountItem mountItem : mountItems) {
1553+
if (mountItem != null
1554+
&& mountItem.getSurfaceId() != View.NO_ID
1555+
&& !mSurfaceIdsWithPendingMountNotification.contains(mountItem.getSurfaceId())) {
1556+
mSurfaceIdsWithPendingMountNotification.add(mountItem.getSurfaceId());
1557+
}
15631558
}
15641559
}
15651560

@@ -1590,6 +1585,20 @@ public void didMountItems(@Nullable List<? extends MountItem> mountItems) {
15901585
}
15911586
}
15921587

1588+
@UiThread
1589+
@ThreadConfined(UI)
1590+
@Override
1591+
public void pullTransaction(int surfaceId) {
1592+
FabricUIManagerBinding binding = mBinding;
1593+
if (binding == null) {
1594+
return;
1595+
}
1596+
binding.pullAndExecuteTransaction(surfaceId);
1597+
if (!mSurfaceIdsWithPendingMountNotification.contains(surfaceId)) {
1598+
mSurfaceIdsWithPendingMountNotification.add(surfaceId);
1599+
}
1600+
}
1601+
15931602
@Override
15941603
public void didDispatchMountItems() {
15951604
for (UIManagerListener listener : mListeners) {

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

Lines changed: 63 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import com.facebook.react.internal.featureflags.ReactNativeFeatureFlags
2424
import com.facebook.react.internal.tracing.PerformanceTracer
2525
import com.facebook.systrace.Systrace
2626
import java.util.Queue
27+
import java.util.concurrent.ConcurrentHashMap
2728
import java.util.concurrent.ConcurrentLinkedQueue
2829

2930
internal class MountItemDispatcher(
@@ -33,10 +34,20 @@ internal class MountItemDispatcher(
3334
private val viewCommandMountItems: Queue<DispatchCommandMountItem> = ConcurrentLinkedQueue()
3435
private val mountItems: Queue<MountItem> = ConcurrentLinkedQueue()
3536
private val preMountItems: Queue<MountItem> = ConcurrentLinkedQueue()
37+
private val surfaceIdsToPull: MutableSet<Int> = ConcurrentHashMap.newKeySet()
3638

37-
/** @return true if any mount items, pre-mount items or view commands are still pending */
39+
/**
40+
* @return true if any mount items, pre-mount items, view commands or pulls of surfaces with an
41+
* attached root view are still pending. The attach schedules the frame that pulls its surface.
42+
*/
3843
fun hasPendingItems(): Boolean =
39-
!viewCommandMountItems.isEmpty() || !mountItems.isEmpty() || !preMountItems.isEmpty()
44+
!viewCommandMountItems.isEmpty() ||
45+
!mountItems.isEmpty() ||
46+
!preMountItems.isEmpty() ||
47+
surfaceIdsToPull.any { !mountingManager.isWaitingForViewAttach(it) }
48+
49+
/** Pull model: @return true if any surface has a transaction waiting to be pulled. */
50+
fun hasSurfaceIdsToPull(): Boolean = surfaceIdsToPull.isNotEmpty()
4051

4152
// Items can be queued from any thread while the DISPATCH_UI frame callback is disarmed at
4253
// idle (see FabricUIManager's doFrameGuarded); the listener re-arms it.
@@ -69,6 +80,12 @@ internal class MountItemDispatcher(
6980
notifyItemsQueued()
7081
}
7182

83+
/** Pull model: a pull mounts the surface's latest revision, so it is pending at most once. */
84+
fun addSurfaceIdToPull(surfaceId: Int) {
85+
surfaceIdsToPull.add(surfaceId)
86+
notifyItemsQueued()
87+
}
88+
7289
fun addPreAllocateMountItem(mountItem: MountItem) {
7390
// We do this check only for PreAllocateViewMountItem - and not DispatchMountItem or regular
7491
// MountItem - because PreAllocateViewMountItem are not batched, and is relatively more
@@ -147,9 +164,10 @@ internal class MountItemDispatcher(
147164
* Executes view commands, pre mount items and mount items in the respective order:
148165
* 1. View commands.
149166
* 2. Pre mount items.
150-
* 3. Regular mount items.
167+
* 3. Pulls of surfaces with an available transaction and an attached root view (pull model).
168+
* 4. Regular mount items.
151169
*
152-
* Does nothing if `viewCommandMountItemsToDispatch` and `mountItemsToDispatch` are empty.
170+
* Does nothing if there are no view commands, mount items or surfaces to pull.
153171
* Nothing should call this directly except for `tryDispatchMountItems`.
154172
*/
155173
@UiThread
@@ -160,8 +178,13 @@ internal class MountItemDispatcher(
160178

161179
val viewCommandMountItemsToDispatch = getAndResetViewCommandMountItems()
162180
val mountItemsToDispatch = getAndResetMountItems()
181+
val attachedSurfaceIdsToPull = getAttachedSurfaceIdsToPull()
163182

164-
if (mountItemsToDispatch == null && viewCommandMountItemsToDispatch == null) {
183+
if (
184+
mountItemsToDispatch == null &&
185+
viewCommandMountItemsToDispatch == null &&
186+
attachedSurfaceIdsToPull == null
187+
) {
165188
return
166189
}
167190

@@ -256,7 +279,8 @@ internal class MountItemDispatcher(
256279
Systrace.endSection(Systrace.TRACE_TAG_REACT)
257280
}
258281

259-
mountItemsToDispatch?.let { items ->
282+
if (mountItemsToDispatch != null || attachedSurfaceIdsToPull != null) {
283+
val items = mountItemsToDispatch.orEmpty()
260284
Systrace.beginSection(
261285
Systrace.TRACE_TAG_REACT,
262286
"MountItemDispatcher::mountViews mountItems to execute",
@@ -269,6 +293,16 @@ internal class MountItemDispatcher(
269293
{ ->
270294
val batchedExecutionStartTime = SystemClock.uptimeMillis()
271295

296+
for (surfaceId in attachedSurfaceIdsToPull.orEmpty()) {
297+
surfaceIdsToPull.remove(surfaceId)
298+
try {
299+
itemDispatchListener.pullTransaction(surfaceId)
300+
} catch (e: Throwable) {
301+
FLog.e(TAG, "dispatchMountItems: caught exception pulling surface $surfaceId", e)
302+
logOrRethrowMountingException(e, surfaceId)
303+
}
304+
}
305+
272306
for (mountItem in items) {
273307
if (ReactNativeFeatureFlags.enableFabricLogs()) {
274308
printMountItem(mountItem, "dispatchMountItems: Executing mountItem")
@@ -298,15 +332,7 @@ internal class MountItemDispatcher(
298332
}
299333
}
300334

301-
if (mountItem.getSurfaceId() != View.NO_ID) {
302-
mountingManager.getSurfaceManager(mountItem.getSurfaceId())?.printSurfaceState()
303-
}
304-
305-
if (ReactIgnorableMountingException.isIgnorable(e)) {
306-
ReactSoftExceptionLogger.logSoftException(TAG, e)
307-
} else {
308-
throw e
309-
}
335+
logOrRethrowMountingException(e, mountItem.getSurfaceId())
310336
}
311337
}
312338
batchedExecutionTime += SystemClock.uptimeMillis() - batchedExecutionStartTime
@@ -375,6 +401,18 @@ internal class MountItemDispatcher(
375401
Systrace.endSection(Systrace.TRACE_TAG_REACT)
376402
}
377403

404+
private fun logOrRethrowMountingException(e: Throwable, surfaceId: Int) {
405+
if (surfaceId != View.NO_ID) {
406+
mountingManager.getSurfaceManager(surfaceId)?.printSurfaceState()
407+
}
408+
409+
if (ReactIgnorableMountingException.isIgnorable(e)) {
410+
ReactSoftExceptionLogger.logSoftException(TAG, e)
411+
} else {
412+
throw e
413+
}
414+
}
415+
378416
private fun executeOrEnqueue(item: MountItem) {
379417
if (mountingManager.isWaitingForViewAttach(item.getSurfaceId())) {
380418
if (ReactNativeFeatureFlags.enableFabricLogs()) {
@@ -408,13 +446,23 @@ internal class MountItemDispatcher(
408446
@UiThread
409447
private fun getAndResetPreMountItems(): List<MountItem>? = drainConcurrentItemQueue(preMountItems)
410448

449+
/** A surface waiting for its root view keeps its pull until the frame after the attach. */
450+
@ThreadConfined(UI)
451+
@UiThread
452+
private fun getAttachedSurfaceIdsToPull(): List<Int>? =
453+
if (surfaceIdsToPull.isEmpty()) null
454+
else surfaceIdsToPull.filterNot(mountingManager::isWaitingForViewAttach).ifEmpty { null }
455+
411456
interface ItemDispatchListener {
412457
fun willMountItems(mountItems: List<MountItem>?)
413458

414459
fun didMountItems(mountItems: List<MountItem>?)
415460

416461
fun didDispatchMountItems()
417462

463+
/** Pull model: pulls the surface's latest transaction and mounts it synchronously. */
464+
fun pullTransaction(surfaceId: Int)
465+
418466
/** Called (from any thread) whenever new items are queued into the dispatcher. */
419467
fun onItemsQueued()
420468
}

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

Lines changed: 0 additions & 33 deletions
This file was deleted.

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
* This source code is licensed under the MIT license found in the
55
* LICENSE file in the root directory of this source tree.
66
*
7-
* @generated SignedSource<<eb0bafceba60cb439a4be3d68b890725>>
7+
* @generated SignedSource<<7fbc80af8ddd69f5986f2c955272c334>>
88
*/
99

1010
/**
@@ -289,7 +289,7 @@ public object ReactNativeFeatureFlags {
289289
public fun enableModuleArgumentNSNullConversionIOS(): Boolean = accessor.enableModuleArgumentNSNullConversionIOS()
290290

291291
/**
292-
* 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`.
292+
* 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`.
293293
*/
294294
@JvmStatic
295295
public fun enableMountingCoordinatorPullModelAndroid(): Boolean = accessor.enableMountingCoordinatorPullModelAndroid()

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ class FabricMountingManager final {
6969

7070
/*
7171
* Pull model: notify Java that a transaction is available for `surfaceId` so
72-
* the UI thread can pull it via a PullTransactionMountItem.
72+
* the UI thread pulls it at its next mount dispatch.
7373
*/
7474
void onTransactionAvailable(SurfaceId surfaceId);
7575

‎packages/react-native/ReactAndroid/src/main/jni/react/fabric/FabricUIManagerBinding.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -730,8 +730,8 @@ void FabricUIManagerBinding::schedulerShouldRenderTransactions(
730730
if (ReactNativeFeatureFlags::enableMountingCoordinatorPullModelAndroid()) {
731731
// Pull model: do NOT pull the transaction or build the batch here (on the
732732
// commit thread). Just notify Java that a transaction is available; the UI
733-
// thread will pull it via a PullTransactionMountItem and call back into
734-
// `pullAndExecuteTransaction`.
733+
// thread calls back into `pullAndExecuteTransaction` at its next mount
734+
// dispatch.
735735
mountingManager->onTransactionAvailable(
736736
mountingCoordinator->getSurfaceId());
737737
} else if (ReactNativeFeatureFlags::

0 commit comments

Comments
 (0)