Skip to content

Commit cc514f2

Browse files
Pull each surface's pending transaction at the start of the mount phase in the Android pull model
With enableMountingCoordinatorPullModelAndroid, every commit notification queued a PullTransactionMountItem in MountItemDispatcher's mount item queue. Its queue position has no semantic meaning: - MountingCoordinator::pullTransaction mounts the latest revision, so the first pending item of a surface mounts every commit made before it runs, and the surface's later items pull nothing. - The position only orders the pull after items queued before the notification. That is enqueue timing, not intent: under push the batch is queued at the end of the JS task, so an accessibility event sent in the same task for a view the commit creates is queued before the batch and can miss its target. - While the root view is detached the item went to the attach queue, where it pulled revisions whose preallocations were queued behind it. #58152 re-queued that batch on the dispatcher, behind a later pull of the same surface, which then mounted the newer revision first. Now MountItemDispatcher keeps a concurrent set of surface ids with an available transaction. onTransactionAvailable adds the surface (and, on the UI thread, dispatches as before). A dispatch runs view commands, pre-mount items, then pulls every pending surface whose root view is attached through ItemDispatchListener.pullTransaction (FabricUIManagerBinding .pullAndExecuteTransaction directly), then the queued mount items. Pulls run inside the "mount" trace and BatchedExecutionTime, with the same ignorable exception handling as mount items (shared logOrRethrowMountingException), and their surfaces are reported to reportMount like before. hasPendingItems counts pending surfaces whose root view is attached, so with disableIdleMountItemFrameCallbackRearmAndroid a pending pull keeps the frame callback armed; a surface waiting for its root view does not. A surface waiting for its root view stays pending. The attach already calls the MountItemExecutor to run its attach queue; that callback now also schedules the frame callback, and the next frame pulls the surface after the preallocations ran. A pulled batch therefore never runs with a detached root. The #58152 re-queue in scheduleMountItem is removed, and with it that reorder. A stopped surface's pending id is pulled once (C++ finds no surface) and dropped; a restarted surface is added again by its next commit. The one ordering change: an item queued before a commit's notification, for a surface with no pull pending, in the same frame, now runs after that commit mounts. It serves events for views the commit creates; an item for a view the same commit deletes now misses a view that disappears in this frame anyway. With disableEarlyViewCommandExecution (default false), view commands are such items: a command dispatched before a commit runs after it mounts. Commands dispatched after a commit still run after it, as the flag intends. PullTransactionMountItem is deleted. Empty pulls still run the mount hooks, but one pull per pending surface per dispatch makes them rare. Tests (FabricUIManagerPullModelTest): - pullDeferredByRootAttach_isAppliedBeforeLaterPulls passes. - onTransactionAvailable_enqueuesOnePullPerNotification becomes onTransactionAvailable_pullsEachPendingSurfaceOnce: repeated notifications of a pending surface add no pull ([1, 2, 3] in any order instead of [1, 2, 1, 3]; the dropped pull was empty). - synchronousBatch_waitingForRootAttach_isDeferred is removed: it scheduled a synchronous batch while the root was detached, which can no longer happen. pullWaitingForRootAttach_isMountedByTheFrameAfterTheAttach covers the #58152 property through onTransactionAvailable. - New: mountItemQueuedBeforeCommit_runsAfterThePull and viewCommandsInMountItemOrder_runAfterThePull document the order (fail on main), and pendingPullOfStoppedSurface_runsOnceAndIsDropped. - New, with disableIdleMountItemFrameCallbackRearmAndroid: pendingPull_keepsTheIdleFrameCallbackArmed (a commit notified from the commit thread during a frame's pull keeps the callback armed; fails without the hasPendingItems change) and commitFromUiThreadDuringPull_isMountedByTheNextFrame (the nested dispatch returns early; the next frame mounts the commit).
1 parent 8573f91 commit cc514f2

10 files changed

Lines changed: 277 additions & 147 deletions

File tree

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

Lines changed: 31 additions & 27 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,9 @@ 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+
// Runs as the surface's root view attaches. Pull model: the frame pulls the transaction
240+
// that waited for the root view.
241+
mDispatchUIFrameCallback.schedule();
240242
}
241243
};
242244

@@ -1003,15 +1005,9 @@ private void scheduleMountItem(
10031005
if (shouldSchedule) {
10041006
Assertions.assertNotNull(mountItem, "MountItem is null");
10051007
if (synchronous) {
1006-
if (mMountingManager.isWaitingForViewAttach(mountItem.getSurfaceId())) {
1007-
// Regular mount items are still being deferred into the surface's attach queue.
1008-
// Executing this batch inline would run it ahead of the preallocations queued there.
1009-
mMountItemDispatcher.addMountItem(mountItem);
1010-
} else {
1011-
// Pull model: we are already on the UI thread, inside the dispatcher's loop executing
1012-
// a PullTransactionMountItem. We don't schedule the item, we execute it directly.
1013-
mountItem.execute(mMountingManager);
1014-
}
1008+
// Pull model: we are already on the UI thread, inside the dispatcher's pull of a surface
1009+
// whose root view is attached. We don't schedule the item, we execute it directly.
1010+
mountItem.execute(mMountingManager);
10151011
} else {
10161012
mMountItemDispatcher.addMountItem(mountItem);
10171013
if (UiThreadUtil.isOnUiThread()) {
@@ -1061,19 +1057,15 @@ public void runGuarded() {
10611057

10621058
/**
10631059
* Pull model: called from C++ via JNI (usually on the commit thread) to signal that a transaction
1064-
* is available for {@code surfaceId}. Enqueues a PullTransactionMountItem so the UI thread pulls
1065-
* and applies the transaction itself, preserving mount-item ordering.
1060+
* is available for {@code surfaceId}. The next dispatch on the UI thread pulls and applies it
1061+
* before the queued mount items.
10661062
*/
10671063
@SuppressWarnings("unused")
10681064
@AnyThread
10691065
@ThreadConfined(ANY)
10701066
@VisibleForTesting
10711067
void onTransactionAvailable(int surfaceId) {
1072-
FabricUIManagerBinding binding = mBinding;
1073-
if (binding == null) {
1074-
return;
1075-
}
1076-
mMountItemDispatcher.addMountItem(new PullTransactionMountItem(surfaceId, binding));
1068+
mMountItemDispatcher.addSurfaceIdToPull(surfaceId);
10771069
if (UiThreadUtil.isOnUiThread()) {
10781070
mMountItemDispatcher.tryDispatchMountItems();
10791071
}
@@ -1538,16 +1530,14 @@ public void didMountItems(@Nullable List<? extends MountItem> mountItems) {
15381530
listener.didMountItems(FabricUIManager.this);
15391531
}
15401532

1541-
if (mountItems == null || mountItems.isEmpty()) {
1542-
return;
1543-
}
1544-
1545-
// Collect surface IDs for all the mount items
1546-
for (MountItem mountItem : mountItems) {
1547-
if (mountItem != null
1548-
&& mountItem.getSurfaceId() != View.NO_ID
1549-
&& !mSurfaceIdsWithPendingMountNotification.contains(mountItem.getSurfaceId())) {
1550-
mSurfaceIdsWithPendingMountNotification.add(mountItem.getSurfaceId());
1533+
if (mountItems != null) {
1534+
// Collect surface IDs for all the mount items
1535+
for (MountItem mountItem : mountItems) {
1536+
if (mountItem != null
1537+
&& mountItem.getSurfaceId() != View.NO_ID
1538+
&& !mSurfaceIdsWithPendingMountNotification.contains(mountItem.getSurfaceId())) {
1539+
mSurfaceIdsWithPendingMountNotification.add(mountItem.getSurfaceId());
1540+
}
15511541
}
15521542
}
15531543

@@ -1578,6 +1568,20 @@ public void didMountItems(@Nullable List<? extends MountItem> mountItems) {
15781568
}
15791569
}
15801570

1571+
@UiThread
1572+
@ThreadConfined(UI)
1573+
@Override
1574+
public void pullTransaction(int surfaceId) {
1575+
FabricUIManagerBinding binding = mBinding;
1576+
if (binding == null) {
1577+
return;
1578+
}
1579+
binding.pullAndExecuteTransaction(surfaceId);
1580+
if (!mSurfaceIdsWithPendingMountNotification.contains(surfaceId)) {
1581+
mSurfaceIdsWithPendingMountNotification.add(surfaceId);
1582+
}
1583+
}
1584+
15811585
@Override
15821586
public void didDispatchMountItems() {
15831587
for (UIManagerListener listener : mListeners) {

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

Lines changed: 60 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,17 @@ 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) }
4048

4149
// Items can be queued from any thread while the DISPATCH_UI frame callback is disarmed at
4250
// idle (see FabricUIManager's doFrameGuarded); the listener re-arms it.
@@ -69,6 +77,12 @@ internal class MountItemDispatcher(
6977
notifyItemsQueued()
7078
}
7179

80+
/** Pull model: a pull mounts the surface's latest revision, so it is pending at most once. */
81+
fun addSurfaceIdToPull(surfaceId: Int) {
82+
surfaceIdsToPull.add(surfaceId)
83+
notifyItemsQueued()
84+
}
85+
7286
fun addPreAllocateMountItem(mountItem: MountItem) {
7387
// We do this check only for PreAllocateViewMountItem - and not DispatchMountItem or regular
7488
// MountItem - because PreAllocateViewMountItem are not batched, and is relatively more
@@ -147,9 +161,10 @@ internal class MountItemDispatcher(
147161
* Executes view commands, pre mount items and mount items in the respective order:
148162
* 1. View commands.
149163
* 2. Pre mount items.
150-
* 3. Regular mount items.
164+
* 3. Pulls of surfaces with an available transaction and an attached root view (pull model).
165+
* 4. Regular mount items.
151166
*
152-
* Does nothing if `viewCommandMountItemsToDispatch` and `mountItemsToDispatch` are empty.
167+
* Does nothing if there are no view commands, mount items or surfaces to pull.
153168
* Nothing should call this directly except for `tryDispatchMountItems`.
154169
*/
155170
@UiThread
@@ -160,8 +175,13 @@ internal class MountItemDispatcher(
160175

161176
val viewCommandMountItemsToDispatch = getAndResetViewCommandMountItems()
162177
val mountItemsToDispatch = getAndResetMountItems()
178+
val attachedSurfaceIdsToPull = getAttachedSurfaceIdsToPull()
163179

164-
if (mountItemsToDispatch == null && viewCommandMountItemsToDispatch == null) {
180+
if (
181+
mountItemsToDispatch == null &&
182+
viewCommandMountItemsToDispatch == null &&
183+
attachedSurfaceIdsToPull == null
184+
) {
165185
return
166186
}
167187

@@ -256,7 +276,8 @@ internal class MountItemDispatcher(
256276
Systrace.endSection(Systrace.TRACE_TAG_REACT)
257277
}
258278

259-
mountItemsToDispatch?.let { items ->
279+
if (mountItemsToDispatch != null || attachedSurfaceIdsToPull != null) {
280+
val items = mountItemsToDispatch.orEmpty()
260281
Systrace.beginSection(
261282
Systrace.TRACE_TAG_REACT,
262283
"MountItemDispatcher::mountViews mountItems to execute",
@@ -269,6 +290,16 @@ internal class MountItemDispatcher(
269290
{ ->
270291
val batchedExecutionStartTime = SystemClock.uptimeMillis()
271292

293+
for (surfaceId in attachedSurfaceIdsToPull.orEmpty()) {
294+
surfaceIdsToPull.remove(surfaceId)
295+
try {
296+
itemDispatchListener.pullTransaction(surfaceId)
297+
} catch (e: Throwable) {
298+
FLog.e(TAG, "dispatchMountItems: caught exception pulling surface $surfaceId", e)
299+
logOrRethrowMountingException(e, surfaceId)
300+
}
301+
}
302+
272303
for (mountItem in items) {
273304
if (ReactNativeFeatureFlags.enableFabricLogs()) {
274305
printMountItem(mountItem, "dispatchMountItems: Executing mountItem")
@@ -298,15 +329,7 @@ internal class MountItemDispatcher(
298329
}
299330
}
300331

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-
}
332+
logOrRethrowMountingException(e, mountItem.getSurfaceId())
310333
}
311334
}
312335
batchedExecutionTime += SystemClock.uptimeMillis() - batchedExecutionStartTime
@@ -375,6 +398,18 @@ internal class MountItemDispatcher(
375398
Systrace.endSection(Systrace.TRACE_TAG_REACT)
376399
}
377400

401+
private fun logOrRethrowMountingException(e: Throwable, surfaceId: Int) {
402+
if (surfaceId != View.NO_ID) {
403+
mountingManager.getSurfaceManager(surfaceId)?.printSurfaceState()
404+
}
405+
406+
if (ReactIgnorableMountingException.isIgnorable(e)) {
407+
ReactSoftExceptionLogger.logSoftException(TAG, e)
408+
} else {
409+
throw e
410+
}
411+
}
412+
378413
private fun executeOrEnqueue(item: MountItem) {
379414
if (mountingManager.isWaitingForViewAttach(item.getSurfaceId())) {
380415
if (ReactNativeFeatureFlags.enableFabricLogs()) {
@@ -408,13 +443,23 @@ internal class MountItemDispatcher(
408443
@UiThread
409444
private fun getAndResetPreMountItems(): List<MountItem>? = drainConcurrentItemQueue(preMountItems)
410445

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

414456
fun didMountItems(mountItems: List<MountItem>?)
415457

416458
fun didDispatchMountItems()
417459

460+
/** Pull model: pulls the surface's latest transaction and mounts it synchronously. */
461+
fun pullTransaction(surfaceId: Int)
462+
418463
/** Called (from any thread) whenever new items are queued into the dispatcher. */
419464
fun onItemsQueued()
420465
}

‎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<<d824f6746b2cc7d4560164c04d123988>>
7+
* @generated SignedSource<<6a0f316e2ce889fc3b2840936d5eaa81>>
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)