Repository navigation
turbo-tasks: avoid redundant aggregation roots and root-upper optimization cascades - #101604
Merged
Merged
Conversation
Contributor
Tests PassedCommit: 498fd5c |
github-actions
Bot
added
created-by: Turbopack team
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
Co-authored-by: Tobias Koppers <1365881+sokra@users.noreply.github.com>
sokra
force-pushed
the
fleet/turbo-tasks-root-uppers
branch
from
November 12, 2026 07:53
67767ce to
498fd5c
Compare
sokra
commented
Nov 12, 2026
Comment on lines
+76
to
+84
| // Parentless connects (e.g. calls from a `turbo_tasks::run` closure) have no upper at all, so | ||
| // there is nothing to shield the persistent graph from. Persistent tasks that need to be roots | ||
| // (strongly consistent reads and reading collectibles are only allowed on `root` functions) | ||
| // are already made roots when they are created. Making every parentless persistent callee a | ||
| // root would make each of them aggregate its whole (possibly large and existing) subgraph. | ||
| let should_make_root = match parent_task_id { | ||
| None => child_task_id.is_transient(), | ||
| Some(parent_task_id) => parent_task_id.is_transient() && !child_task_id.is_transient(), | ||
| }; |
Member
Author
There was a problem hiding this comment.
By the logic in the comment, parentless transient tasks also doesn't need to be roots. Is that correct?
Contributor
There was a problem hiding this comment.
Yes, I think so. Strongly consistent and collectibles reads are guarded by native_fn.is_root, and that applies to transient function tasks too, since their task type is set in initialize_new_task. root functions are rooted at creation, and Root/Once transient tasks start with the max aggregation number. So the parentless-transient arm is redundant. I only kept it to limit the behavior change to the measured persistent case. I'll send a follow-up that drops it.
sokra
marked this pull request as ready for review
November 12, 2026 08:30
sokra
added a commit
that referenced
this pull request
Nov 12, 2026
…101614) ## Summary Follow-up to #101604, from [this review comment](#101604 (comment)). #101604 stopped making parentless persistent callees, such as tasks called from a `turbo_tasks::run` closure, aggregation roots. This PR does the same for parentless **transient** callees. The same argument applies to transient tasks: - A parentless connect has no upper, so there is nothing to shield. - Tasks that need to be roots already are: - Strongly consistent reads and collectibles reads are only allowed on `root` functions. The check is based on the task's function and applies to transient function tasks as well. - `root` functions are made roots when they are created, whether persistent or transient. - Top-level transient tasks (root and once tasks) are created with the maximum aggregation number. Now `connect_child` only makes a task a root when a transient parent calls a persistent child. That rule shields the persistent graph from the transient caller and is unchanged. The "already constructed root" shortcut is removed. It only applied to the parentless transient case, so it is dead code now. ## Verification - `connect_child` unit test: a parentless transient child is not made a root. This test is flipped from the previous behavior. - New integration test `parentless_calls_of_transient_tasks_are_only_roots_if_root_functions`: - A transient non-root function called from `run` is not a root, and is still held by a transient ref. - A transient `root` operation called from `run` is a root. - Strongly consistent reads of that operation return correct values before and after an invalidation. - It fails on canary on the non-root assertion. - `cargo nextest run -p turbo-tasks-backend`: 456 passed, also with `--features verify_aggregation_graph`. <!-- NEXT_JS_LLM --> <!-- fleet 646973d0-3866-4b17-a7a8-d4e4971dc4dd --> Co-authored-by: vercel-fleet-prod[bot] <318278635+vercel-fleet-prod[bot]@users.noreply.github.com> Co-authored-by: Tobias Koppers <1365881+sokra@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two fixes in
turbo-tasks-backendfor cases where reading already computed tasks from a new root, or from a parentlessturbo_tasks::runclosure, restructures large parts of the aggregation graph. Context: #100250. The motivating pattern is a new root operation reading the existing endpoint outputs after the emit, like the route bundle stats in #101540.A: count all root uppers as one upper in
optimize_taskoptimize_taskraises a task's aggregation number when(uppers - 1) * followersis too large. A root aggregates its whole subgraph, and raising the number never removes an upper edge to a root, yet root uppers were still counted.So every additional root over an existing subgraph made large parts of it look like they needed optimization. The resulting raises cascaded into a rebalance of the subgraph without reducing upper edges. The old code already counted a
root_uppersvariable but never used it, and it also counted root followers.The fix:
Behavior with at most one root upper is unchanged.
B: parentless calls no longer make persistent callees aggregation roots
connect_childmade every child connected without a parent task (e.g. called from aturbo_tasks::runclosure) an aggregation root, so each one aggregated its entire, possibly large and existing, subgraph.Making the callee a root is only needed to shield a persistent graph from a transient upper, and a parentless connect has no upper. Persistent tasks that need to be roots are already made roots at creation: strongly consistent reads and collectibles reads are only allowed on
rootfunctions.What changes and what doesn't:
Benchmarks
Machine: 32 vCPU (16 cores × 2 SMT), 64 GB RAM. Pristine canary
2be83539vs this PR, both built locally as release next-swc.TURBO_ENGINE_IGNORE_DIRTY=1.taskset -c 0,2,…,30); 32 CPUs is0-31.%M, which is the largest single process'sru_maxrss.bench/large-app-router(5,102 routes, from bench: add generated large App Router benchmark for Turbopack scaling #101502, not part of this PR) andbench/nested-deps-app-router-many-pages.Microbenchmark (new
turbo_tasks_backend_existing_outputscriterion bench, 32 threads):runclosure calling 50 existing endpointsnext build: no significant change on canary's own build, as expected. Canary rarely layers roots over existing graphs or calls existing tasks fromrun. The VM was noisy in the second block, hence the large stddev on large-app-router.Peak memory. Cold-build deltas change sign between blocks and are within stddev. The large-app-router warm builds showed a small positive delta (+0.36% at 16 CPUs, +0.51% at 32 CPUs, about +20–30 MiB) in both blocks. I re-checked that with a controlled protocol:
There the PR was not higher: canary 5.024 ± 0.025 GiB (n=6), PR 5.009 ± 0.031 GiB (n=5; one run hit a full disk and is excluded). Builds with only A or only B were also not higher: 5.018 ± 0.032 and 5.003 ± 0.035 GiB.
The persisted cache size differs by +2–4 MB (0.1%). I couldn't attribute the warm delta in the standard protocol to the change, but I'm disclosing it here rather than claiming a proven memory non-regression.
Raw data (wall time and raw peak RSS in KiB)
next buildwall (s)Controlled warm protocol (large-app-router, 32 CPUs), raw peak RSS in KiB:
ENOSPC)On the #101540 pattern (stats over 5,102 existing endpoint outputs from a new root op), an earlier exploratory run with A enabled through an env toggle reduced that step from 4.18 ± 0.63 s to 3.54 ± 0.18 s (n=3, 32 CPUs). B removes the much larger cliff of the
tt.runvariant of that pattern: 50 endpoints in a synthetic repro went from 64 s to 0.1 ms.Verification
aggregation_update.rs:connect_child.rs:tests/existing_outputs_from_new_roots.rs:cargo nextest run -p turbo-tasks-backend: 456 passed, also with--features verify_aggregation_graph.cargo test -p turbo-tasks-backend --test existing_outputs_from_new_roots.