Repository navigation
Conversation
|
I would appreciate feedback from Android/Fabric maintainers and anyone familiar with mounting lifecycle or view recycling. Below is the implementation rationale and the remaining uncertainty. 1. Where the experiment is applied
2. What stays synchronousValidated subtree roots are detached immediately, so they are removed from the mounted hierarchy in the original transaction. The plan retires all affected tags from Only descendant hierarchy removal and view-manager deletion/recycling work are deferred. Cleanup retains the original state and view objects rather than resolving tags later. If a tag is reused before cleanup, old cleanup therefore acts on the retired object, not the replacement view. A surface stop drains retired objects before recycling pools are evicted. 3. Scheduling and budgets
Each slice stops after 256 cleanup steps or approximately 1 ms, whichever is reached first. The deadline is checked between steps; one expensive framework callback can exceed it. Remaining work waits for another draw. Hidden or detached surfaces can drain without waiting for a draw, while a visible surface never starts cleanup merely because a timeout elapsed. Observer replacement on window reattachment and synchronous/reentrant surface shutdown are covered. Admission is capped at eight tasks and 2,048 estimated operations across the queue. Each retired state reserves two operations, conservatively covering descendant removals and deletion callbacks; a single admitted plan is therefore at most 1,024 states. Exceeding admission limits falls back synchronously without flushing or corrupting previously queued work. Very large unmounts can consequently still hit the original synchronous cost. 4. Why validation was adjustedThe plan uses one tag-to-index map for DELETE membership, duplicate detection, and state indexing, plus indexed arrays for REMOVE coverage and cached parent data. This avoids repeated set construction and repeated lookups across separate passes. Validation is still linear and synchronous, and it allocates a plan. The change moves selected cleanup work; it does not make unmounting constant-time or remove validation cost. 5. Validation and performance boundaryThe PR test plan lists the local commands. After merging upstream main at eaf4c26, the focused Android JVM run passed all 145 tests in 16 suites, with zero failures, errors, or skipped tests. RNTester Debug x86_64 assembly, Android lint, Kotlin formatting, Prettier, and generated feature-flag consistency were also checked. Tests exercise both the enabled experiment and the default synchronous path, including tag reuse, emitter/state destruction, recycling, queue saturation, unsupported transactions, and draw/stop ordering. The local tracing work is Debug x86_64 evidence only. Cleanup backlog was reduced, but end-to-end frame-time improvement was not consistent, and synchronous subtree validation remains material. There is no Release physical-device benchmark supporting a performance claim yet. The flag remains off by default. 6. Feedback requested before extending this
I am happy to narrow or revise the design based on this feedback. I would like to establish the lifecycle contract and a useful benchmark before expanding eligibility or proposing that the flag be enabled. |
Summary:
Related to #59448. This is an experimental, default-off Android Fabric implementation for reviewing whether descendant cleanup can safely cross a draw boundary after an unmounted subtree has been detached. It does not claim to resolve the reported frame-time problem yet.
enableDeferredViewCleanupAndroid, disabled by default, with regenerated feature-flag accessors.ReactViewGroup/ReactViewManagerinstances. Unsupported or mixed transactions retain the existing synchronous path.onDropViewInstance, so tag reuse cannot redirect old cleanup to a new view.Local Debug x86_64 traces showed reduced cleanup backlog, but did not establish a stable end-to-end frame-time improvement. Validation itself remains synchronous and can offset the benefit. Release testing on physical devices is still needed. I am opening this to request feedback on lifecycle correctness, scope, and whether this direction is worth pursuing before broader optimization or rollout.
Changelog:
[ANDROID] [ADDED] - Add a default-off experimental flag for deferred cleanup of validated pure-view unmount batches in Fabric.
Test Plan:
Validated locally on Windows with Android Debug x86_64 after merging upstream main at eaf4c26. The focused JVM run passed all 145 tests across 16 suites, with zero failures, errors, or skipped tests. The feature-flag generator was used; generated accessors were not hand-edited.
The five Kotlin files checked are
DeferredViewCleanupQueue.kt,SurfaceMountingManager.kt,IntBufferBatchMountItem.kt,DeferredViewCleanupTest.kt, andIntBufferBatchMountItemTest.kt. The local Gradle executable is Gradle 9.4.1; the codegen CLI was already built before the Gradle commands. The local runner temporarily resolves the Hermes compiler placeholder against the installed compiler and restores the tracked package file afterward.Regression coverage includes immediate reference retirement, draw ordering, bounded slices, queue saturation, stopping a surface, detach/reattach, reentrant flushing, recycling, tag reuse, active touches, malformed/incomplete batches, sibling indices, and synchronous fallbacks.
Not run: the complete JS, Flow, Fantom, iOS, or all-architecture native suites. No Release physical-device performance results are available. This PR should be reviewed as an experiment, not as a demonstrated frame-time fix.