Skip to content

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

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

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

Conversation

@014-code

@014-code 014-code commented Oct 28, 2026 •

Copy link
Copy Markdown

Summary:

Related to #59448. Add a default-off Android Fabric experiment that detaches a
validated pure-View subtree and retires its tags, state wrappers and event
emitters immediately, then cleans up descendants in bounded UI-thread slices
after a draw callback returns. Cleanup retains object references so reused tags
cannot redirect old work to new views.

Only whole REMOVE-then-DELETE batches of exact core View classes are eligible.
Mixed/unsupported transactions and admission-limit overflow keep the synchronous
path. Shutdown evicts recycling pools before flushing retired views, preserving
the ordering introduced by #45913. Trace markers expose admission/rejection,
validation, cleanup slices and queue reservations.

This remains an experiment. There are no Release physical-device results proving
representative-workload eligibility, better frame timing or acceptable retained
memory. The flag stays disabled; the previous removal in #45385 is a reason to
require this evidence before merging or expanding scope.

Changelog:

[ANDROID] [ADDED] - Add a default-off experimental flag for deferred cleanup of validated pure-view unmount batches in Fabric.

Test Plan:

Validated in a Linux Docker container after merging main at e57dcb3cc4.

  • Android focused JVM tests: 149 tests in 16 suites, zero failures/errors/skips.
    The two new recycling shutdown tests fail with the previous ordering and pass
    after the fix. New fallback cases cover layout/overflow updates and 1,025 states.
  • Full JS suite on Node 22.13.0 and 24.3.0: 262 suites, 6,457 tests passed,
    one skipped, 1,684 snapshots passed on each runtime. Both runs emitted a worker
    forced-exit/open-handles warning.
  • ESLint, Flow, JS API type snapshot validation, legacy TypeScript checks,
    generated feature-flag consistency and authored Kotlin formatting passed.
  • The external RNTester probe bundles successfully for Android in Release mode.
  • Both arm64-v8a Release probe APKs (cleanup OFF/ON, 256-node pure workload,
    recycling disabled) build successfully and pass signature/content checks.
    Saved fixture and Kotlin input hashes match; configurations differ only in
    the cleanup default. Source defaults and Playground are restored afterward.
    These are RNTester local test APKs using the repository debug signing config.
  • Android Debug native compilation and lintDebug complete. The repository sets
    abortOnError=false: the lint report contains 63 errors, 161 warnings and two
    hints, with none pointing to the four authored Kotlin files. This is not a
    diagnostic-free lint report.
  • Fantom runner and Hermes build passed; the View integration suite passed all
    232 tests. It emitted a one-second open-handles warning and exited normally.
  • Android 15/API 35 x86_64 emulator, Release builds from Docker: OFF/ON each
    completed 30 mount/unmount cycles, three background/resume cycles and three
    Activity exits/reopens with no crashes/ANRs. Attached views fell from 287 to 31.
    ON recorded 33 accepted 256-state unmounts and 33 unique tasks completed once;
    pending tasks/reserved operations ended at zero. OFF had no experiment markers.
  • An ON Release mixed-width probe completed ten emulator cycles with zero
    admissions or queued tasks, and 20 object-buffer rejections across mount/unmount
    batches. These are synthetic eligibility checks; representative-workload rates
    and physical-device performance remain unmeasured. Recycling was disabled in
    the emulator probes; deterministic JVM tests cover recycling shutdown ordering.

Commands:

yarn lint
yarn test --maxWorkers=2  # once with each Node runtime
yarn flow-check --max-workers 2
yarn build-types --validate
yarn test-typescript-legacy
node packages/react-native/scripts/featureflags/index.js --verify-unchanged
CC=clang CXX=clang++ bash /validation/gradle-check.sh :private:react-native-fantom:buildFantomTester
taskset -c 0,1 yarn fantom Libraries/Components/View/__tests__/View-itest.js --runInBand
bash /validation/build-benchmark.sh off 256 pure arm64-v8a false
bash /validation/build-benchmark.sh on 256 pure arm64-v8a false
python3 /validation/verify-release-pair.py
python3 /validation/verify-restoration.py
bash /validation/build-benchmark.sh off 256 pure x86_64 false
bash /validation/build-benchmark.sh on 256 pure x86_64 false
python3 /validation/verify-release-pair.py --abi x86_64
bash /validation/build-benchmark.sh on 256 mixed x86_64 false
bash /validation/gradle-check.sh \
  :packages:react-native:ReactAndroid:testDebugUnitTest \
  --tests 'com.facebook.react.fabric.*' \
  --tests 'com.facebook.react.views.view.*' \
  :packages:react-native:ReactAndroid:lintDebug

The local Gradle wrapper uses Gradle 9.4.1, JDK 17, Android SDK 37.0,
NDK 28.2.13676358, x86_64 for focused tests and arm64-v8a for Release APKs,
two workers and warnings-as-errors. The external
validation wrapper configures SDK paths, bounded heap/CPU settings and local
Maven/codeload download workarounds; it does not change repository dependencies.
Feature-flag outputs were regenerated from their source configuration.

Not run: physical-device Release performance/interaction tests, macOS/iOS E2E,
and all-architecture native suites. OFF/ON probe scripts and a Perfetto protocol
are prepared outside the PR source tree. No performance improvement is claimed.

@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
@javache

javache commented Oct 30, 2026

Copy link
Copy Markdown
Contributor

This seems to match an optimisation previously tried as RemoveDeleteTree:

This PR is narrower and improves safety by retiring tags/state/event emitters immediately, retaining object references for cleanup, and using bounded post-draw slices, but two concerns remain:

  • Shutdown ordering: pending cleanup is flushed before onSurfaceStopped, so views can be prepared for recycling immediately before their pool is discarded. #45913 deliberately established the opposite ordering to avoid that wasted work. Preserve it and add a pending-cleanup shutdown regression test.
  • Benchmark eligibility: the parser accepts only pure REMOVE-then-DELETE batches, while the traces also contain layout/overflow updates. Those batches would fall back synchronously. Demonstrate that representative workloads actually enter the fast path before assessing its benefit.

Recommendation: fix shutdown ordering and obtain Release physical-device measurements covering the unmount frame, subsequent frames, memory retention and recycling behaviour before merging. The historical removal is a reason to require stronger evidence—not proof that deferred cleanup cannot help.

@014-code

014-code commented Oct 31, 2026 •

Copy link
Copy Markdown
Author

@javache Thanks for the historical context and the specific shutdown/eligibility concerns. I updated this PR in c7f6749.

Shutdown now calls ViewManagerRegistry.onSurfaceStopped(surfaceId) before flushing retired views, preserving #45913's ordering. Two new tests cover pending cleanup at shutdown and shutdown reentrantly during child removal. Both fail with the previous ordering and pass with the fix; they also check no prepareToRecycleView, duplicate drops or reuse after restart. Two additional tests cover layout/overflow mixed-batch fallback and the 1,025-state admission limit.

I kept the strict whole-batch admission contract and added reason-specific rejection markers, accepted-state counts, queue reservation counters and a shutdown-flush section. I then built the Release probes in Docker and ran them on an Android 15/API 35 x86_64 emulator:

  • A fixed 256-state, non-collapsible View subtree, with stable controls, a fixed viewport and recycling disabled: both OFF and ON completed 30 mount/unmount cycles, three background/resume cycles and three Activity exits/reopens without crashes or ANRs. Attached native views went from 287 to 31 after unmounting in both variants.
  • ON recorded 33 accepted unmounts, each with 256 states, covering the 30 cycles plus the three unmounts before backgrounding. All 33 async cleanup tasks ended exactly once; pending-task and reserved-operation counters returned to zero. OFF emitted no experimental-path markers.
  • Changing a surviving sibling's width in the same update as unmounting completed ten cycles with zero admissions or queued tasks. The trace recorded 20 object-buffer rejections across the mount/unmount batches, exercising the synchronous fallback.

The focused Android JVM run passes 149 tests in 16 suites; Fantom View passes 232 tests. Full Jest passes 6,457 tests on each of Node 22.13.0 and 24.3.0, and ESLint, Flow, type snapshot validation and generated flags pass. I also merged the upstream Kotlin build and Hermes extraction fixes and corrected the flag configuration's alphabetical ordering. Android lintDebug completes under the repository's non-fatal configuration, but still reports 63 errors and 161 warnings; none point to the four authored Kotlin files.

I do not have a physical Android device. The emulator results establish functionality and synthetic-workload eligibility; representative-workload admission rates, physical-device frame windows, retained memory and recycling measurements are still missing. I am keeping the flag disabled and making no performance or merge-readiness claim. I understand that #45385 makes stronger evidence necessary before merging.

The new Test All run is currently awaiting maintainer approval and has not started any jobs yet.

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.

2 participants