Skip to content

Android Fabric: experiment with deferred pure-view subtree cleanup - #59564

Open
014-code wants to merge 2 commits into
react:mainfrom
014-code:fix/android-fabric-subtree-unmount
Open

014-code wants to merge 2 commits into
react:mainfrom
014-code:fix/android-fabric-subtree-unmount

Conversation

@014-code

Copy link
Copy Markdown

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.

  • Add enableDeferredViewCleanupAndroid, disabled by default, with regenerated feature-flag accessors.
  • Accept only an entire REMOVE-then-DELETE batch for a fully validated subtree of exact core ReactViewGroup / ReactViewManager instances. Unsupported or mixed transactions retain the existing synchronous path.
  • Detach subtree roots immediately and retire tag registrations, synchronous props, state wrappers, and event emitters in the mounting transaction. Use retained object references for subsequent descendant removal and onDropViewInstance, so tag reuse cannot redirect old cleanup to a new view.
  • Run descendant cleanup on the UI thread through a bounded queue after a draw callback has returned. Flush retired views when the surface stops.

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.

gradle :packages:react-native:ReactAndroid:testDebugUnitTest --tests "com.facebook.react.fabric.*" --tests "com.facebook.react.views.view.*" -PreactNativeArchitectures=x86_64 -Preact.internal.windowsBashPath=D:/Git/bin/bash.exe -x :packages:react-native:ReactAndroid:buildCodegenCLI --max-workers=2 --console=plain --stacktrace
gradle :packages:rn-tester:android:app:assembleDebug -PreactNativeArchitectures=x86_64 -Preact.internal.windowsBashPath=D:/Git/bin/bash.exe -x :packages:react-native:ReactAndroid:buildCodegenCLI --max-workers=2 --console=plain --stacktrace
gradle :packages:react-native:ReactAndroid:lintDebug -PreactNativeArchitectures=x86_64 -Preact.internal.windowsBashPath=D:/Git/bin/bash.exe -x :packages:react-native:ReactAndroid:buildCodegenCLI --max-workers=2 --console=plain --stacktrace
node node_modules/ktfmt/bin/ktfmt.js --do-not-remove-unused-imports --dry-run --set-exit-if-changed packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/SurfaceMountingManager.kt packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/DeferredViewCleanupQueue.kt packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/IntBufferBatchMountItem.kt packages/react-native/ReactAndroid/src/test/java/com/facebook/react/fabric/DeferredViewCleanupTest.kt packages/react-native/ReactAndroid/src/test/java/com/facebook/react/fabric/mounting/mountitems/IntBufferBatchMountItemTest.kt
node node_modules/prettier/bin/prettier.cjs --check packages/react-native/scripts/featureflags/ReactNativeFeatureFlags.config.js
node packages/react-native/scripts/featureflags/index.js --verify-unchanged
git diff --check upstream/main...HEAD

The five Kotlin files checked are DeferredViewCleanupQueue.kt, SurfaceMountingManager.kt, IntBufferBatchMountItem.kt, DeferredViewCleanupTest.kt, and IntBufferBatchMountItemTest.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.

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 28, 2026
@014-code

Copy link
Copy Markdown
Author

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

IntBufferBatchMountItem attempts this path only when enableDeferredViewCleanupAndroid is enabled and mount-item reordering is allowed. The parser accepts a complete REMOVE phase followed by a DELETE phase, with no object buffer. CREATE, INSERT, updates, animation instructions, interleaved phases, and malformed counts fall back to normal mounting. The parser bounds each operation list at 4,096 entries; queue admission is deliberately stricter.

SurfaceMountingManager validates the whole plan before mutating anything. REMOVE and DELETE membership must match, duplicates are rejected, each child must be the same object held by its view state, and sibling removal indices must descend. Every child of a retired view must be covered. Deleted views must be exact core ReactViewGroup instances managed by exact ReactViewManager instances; custom managers, clipped children, animations, transient state, layout transitions, active touches, and known re-added tags are rejected. Parent checks also guard removal semantics. This narrow scope is intentional while lifecycle assumptions are under review.

2. What stays synchronous

Validated subtree roots are detached immediately, so they are removed from the mounted hierarchy in the original transaction. The plan retires all affected tags from tagToViewState and synchronous-props storage, destroys state wrappers and event emitters, and clears those wrapper references immediately. JSI-backed references are not kept alive until a later draw or VM shutdown. Active-touch cases retain the original emitter-lifetime path by falling back.

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

DeferredViewCleanupQueue runs exclusively on the UI thread. For visible, attached surfaces, each task waits for a subsequent OnDrawListener generation and posts cleanup to a Handler, outside that draw traversal. This is a draw boundary, not a guarantee that the GPU has presented the frame. A task enqueued after a draw must wait for its own later draw.

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 adjusted

The 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 boundary

The 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

  1. Is immediate root detachment and reference retirement followed by delayed descendant removal an acceptable lifecycle split for this restricted core-view case? Are there attachment, accessibility, focus, or observer semantics that need stronger guards?
  2. Is deferring onDropViewInstance / recycling until cleanup acceptable, including when shutdown or tag reuse occurs before the next draw?
  3. Is the strict whole-batch Android-side signal an appropriate first experiment, or would explicit renderer metadata be preferable?
  4. Are the queue admission limits and per-slice budgets reasonable starting points? Which overload behavior would maintainers prefer for larger subtrees?
  5. Which Release physical-device workloads and metrics should decide whether to continue? I would especially welcome suggestions for measuring unmount-frame latency, subsequent-frame cost, memory retention, and touch/animation correctness together.

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.

@facebook-github-tools facebook-github-tools Bot added the Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. label Oct 28, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant