turbo-tasks-backend: parent_count reference counting + garbage collection#95976
Draft
lukesandberg wants to merge 1 commit into
Draft
turbo-tasks-backend: parent_count reference counting + garbage collection#95976lukesandberg wants to merge 1 commit into
lukesandberg wants to merge 1 commit into
Conversation
Contributor
Author
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
This was referenced Jul 20, 2026
Contributor
Stats from current PR🟢 2 improvements
📊 All Metrics📖 Metrics GlossaryDev Server Metrics:
Build Metrics:
Change Thresholds:
⚡ Dev Server
📦 Dev Server (Webpack) (Legacy)📦 Dev Server (Webpack)
⚡ Production Builds
📦 Production Builds (Webpack) (Legacy)📦 Production Builds (Webpack)
📦 Bundle SizesBundle Sizes⚡ TurbopackClient Main Bundles
Server Middleware
Build DetailsBuild Manifests
📦 WebpackClient Main Bundles
Polyfills
Pages
Server Edge SSR
Middleware
Build DetailsBuild Manifests
Build Cache
🔄 Shared (bundler-independent)Runtimes
📝 Changed Files (29 files)Files with changes:
View diffsapp-page-exp..ntime.dev.jsfailed to diffapp-page-exp..time.prod.jsfailed to diffapp-page-tur..ntime.dev.jsfailed to diffapp-page-tur..time.prod.jsfailed to diffapp-page-tur..ntime.dev.jsfailed to diffapp-page-tur..time.prod.jsfailed to diffapp-page.runtime.dev.jsfailed to diffapp-page.runtime.prod.jsfailed to diffapp-route-ex..ntime.dev.jsDiff too large to display app-route-ex..time.prod.jsDiff too large to display app-route-tu..ntime.dev.jsDiff too large to display app-route-tu..time.prod.jsDiff too large to display app-route-tu..ntime.dev.jsDiff too large to display app-route-tu..time.prod.jsDiff too large to display app-route.runtime.dev.jsDiff too large to display app-route.ru..time.prod.jsDiff too large to display pages-api-tu..ntime.dev.jsDiff too large to display pages-api-tu..time.prod.jsDiff too large to display pages-api.runtime.dev.jsDiff too large to display pages-api.ru..time.prod.jsDiff too large to display pages-turbo...ntime.dev.jsDiff too large to display pages-turbo...time.prod.jsDiff too large to display pages.runtime.dev.jsDiff too large to display pages.runtime.prod.jsDiff too large to display server.runtime.prod.jsDiff too large to display use-cache-pr..ntime.dev.jsDiff too large to display use-cache-pr..ntime.dev.jsDiff too large to display use-cache-pr..ntime.dev.jsDiff too large to display use-cache-pr..ntime.dev.jsDiff too large to display 📎 Tarball URLCommit: f844d18 |
Contributor
Tests PassedCommit: f844d18 |
lukesandberg
commented
Jul 20, 2026
| /// garbage-collection pass (tearing down tasks whose persistent parent count reached 0). Opt-in | ||
| /// until it has been proven on trusted apps; the eventual default-on flip gets its own escape | ||
| /// hatch. | ||
| static GC_ENABLED: LazyLock<bool> = LazyLock::new(|| { |
Contributor
Author
There was a problem hiding this comment.
nit: move into gc_enabled
lukesandberg
commented
Jul 20, 2026
| /// | ||
| /// (A single-variant enum today; kept as an enum so the self-feeding pool's job type has room to | ||
| /// grow — e.g. if the per-task teardown is later split to fan a huge task's edges across workers.) | ||
| enum GcJob { |
Contributor
Author
There was a problem hiding this comment.
simplify, this doesn't need to be an enum
lukesandberg
commented
Jul 20, 2026
Comment on lines
+51
to
+59
| /// A unit of garbage-collection work fed through the self-feeding parallel pool in | ||
| /// [`TurboTasksBackend::gc_collect`]: collecting one task. Parallelism is *across* tasks — the pool | ||
| /// runs many `Collect` jobs on different workers — while each task's own teardown (its | ||
| /// `CleanupOldEdges` run) is sequential. A `Collect` discovers more `Collect` jobs (each child the | ||
| /// cleanup drives to `parent_count == 0` and that is itself collectible), which flow straight back | ||
| /// into the pool until it drains. | ||
| /// | ||
| /// (A single-variant enum today; kept as an enum so the self-feeding pool's job type has room to | ||
| /// grow — e.g. if the per-task teardown is later split to fan a huge task's edges across workers.) |
Contributor
Author
There was a problem hiding this comment.
comment could just be
A Request to delete a given task. Once deleted Cleans up old edges and enqueues new jobs as needed
lukesandberg
commented
Jul 20, 2026
Comment on lines
+133
to
+141
| // TODO(perf): recycle the task ids of collected tasks. `persisted_task_id_factory` | ||
| // (`IdFactoryWithReuse`) can hand out freed ids, and the persisted `next_free_task_id` | ||
| // high-water mark only grows today, so the id space grows unboundedly across churn even | ||
| // though the task set stays flat. Reuse must happen only AFTER the `save_snapshot` that | ||
| // tombstoned the id has committed (a crash before commit leaves the task on disk — reusing | ||
| // its id would alias it), and must be guarded against resurrection: between removal and | ||
| // commit a `get_or_create_task` for the same type could re-mint the id, and the id must not | ||
| // be handed out while any live `OperationVc`/`DetachedVc` still references it. Feed the | ||
| // recycled ids into `persisted_task_id_factory` so the high-water mark can stop growing. |
Contributor
Author
There was a problem hiding this comment.
we should accumulate deleted tasks in a RoaringBitmap that we
- persist
- use to make accessing deleted tasks a hard error
lukesandberg
commented
Jul 20, 2026
| } | ||
|
|
||
| // Written once per collected task (not per child/dep), so the atomic is not a hot path. | ||
| let collected = AtomicUsize::new(0); |
Contributor
Author
There was a problem hiding this comment.
we should consider other stats
edges deleted
roots detected
lukesandberg
force-pushed
the
lukesandberg/gc-03-parent-count-collection
branch
from
July 20, 2026 22:43
a6f5c07 to
5d4aadd
Compare
lukesandberg
force-pushed
the
lukesandberg/gc-02-tombstone-plumbing
branch
from
July 20, 2026 22:43
b80ac91 to
14080b5
Compare
lukesandberg
commented
Jul 20, 2026
| /// Runs one [`GcJob`], possibly spawning follow-up jobs into the same pool. See | ||
| /// [`Self::gc_collect`] for the concurrency argument. Each job builds its own GC | ||
| /// [`ExecuteContext`]. | ||
| fn gc_run_job( |
Contributor
Author
There was a problem hiding this comment.
just inline this into the scope_self_feeding closure?
lukesandberg
commented
Jul 20, 2026
Comment on lines
+185
to
+193
| // Capture ALL of this task's edges as `OutdatedEdge`s, then hand them to the same | ||
| // `CleanupOldEdges` operation a re-executing task uses. This is what makes GC | ||
| // teardown correct: alongside dropping each child's `parent_count` and scrubbing | ||
| // the forward-dep reverse edges, it PROPAGATES THE AGGREGATION | ||
| // REBALANCE (removes this task from its children's `upper` sets via | ||
| // `InnerOfUppersLostFollowers`). Without that, collected children | ||
| // would be left with a dangling upper edge and never | ||
| // become collectible themselves. The op opens `ctx.task(task_id)` to remove the | ||
| // child edges, so it must run while `task_id` is still resident. |
Contributor
Author
There was a problem hiding this comment.
simplify the comment
just collect all edges and delete them before tagging the task itself as deleted
lukesandberg
commented
Jul 20, 2026
Comment on lines
+221
to
+225
| // The edges are gone and the children are rebalanced. Instead of removing the task | ||
| // from the map now (which would let a later `ctx.task` on it — e.g. a sibling's | ||
| // `CleanupOldEdges` forward-dep scrub — resurrect it from disk as a zombie), mark | ||
| // it soft-deleted and keep it resident. The next snapshot tombstones its on-disk | ||
| // copy; a later step hard-deletes it from memory once the tombstone has committed. |
Contributor
Author
There was a problem hiding this comment.
simplify comment
lukesandberg
commented
Jul 20, 2026
| !child.is_transient(), | ||
| "gc: a transient task should never have a persistent parent_count to zero" | ||
| ); | ||
| if ctx.task(child, TaskDataCategory::Meta).is_gc_collectible() { |
Contributor
Author
There was a problem hiding this comment.
CleanupOldEdges must have fetched All and the next job will do the same... not sure if it matters but should we do the same here?
lukesandberg
commented
Jul 20, 2026
Comment on lines
+212
to
+219
| drop(task); | ||
|
|
||
| CleanupOldEdgesOperation::run( | ||
| task_id, | ||
| old_edges, | ||
| AggregationUpdateQueue::new(), | ||
| &mut ctx, | ||
| ); |
Contributor
Author
There was a problem hiding this comment.
does the relative order of task_deleted and CleanupOldEdges actually matter?
if we mark deleted first, we could save a lock acquisition
lukesandberg
commented
Jul 20, 2026
Comment on lines
+249
to
+281
| /// Marks a collected task **soft-deleted**: it stays resident (in the map and `task_cache`) but | ||
| /// is flagged for deletion and forced into the next snapshot's modified scan. The task's edges | ||
| /// and the aggregation rebalance were already handled by the `CleanupOldEdges` run in | ||
| /// [`Self::gc_run_job`] before this is called. | ||
| /// | ||
| /// Keeping the task resident is the whole point: during the pass another job's `ctx.task` on it | ||
| /// (e.g. a sibling's forward-dep scrub inside `CleanupOldEdges`) finds a real entry rather than | ||
| /// restoring a zombie from disk. The next snapshot's `process` closure sees the `deleted` flag, | ||
| /// tombstones the on-disk copy (task meta/data + `TaskCache` bucket) instead of persisting, and | ||
| /// clears the modified flags; a later step (`evict_after_snapshot`, or the drain snapshot at | ||
| /// shutdown) hard-deletes it from memory once the tombstone has committed. If the task is | ||
| /// resurrected by a connect before then, the marker is cleared and it is made dirty. Must be | ||
| /// called while holding the GC phase. | ||
| fn gc_mark_deleted(&self, task_id: TaskId, ctx: &mut impl ExecuteContext<'_>) { | ||
| let mut task = ctx.task(task_id, TaskDataCategory::Meta); | ||
| task.set_deleted(true); | ||
| // Force the task into the next snapshot's per-shard modified scan so `process` can | ||
| // tombstone it. This can't be an assert-it's-already-modified: a collected task need NOT | ||
| // have been modified this cycle. The count-to-0 cascade DOES dirty meta (`parent_count` is | ||
| // a tracked meta field) and so does an aggregation-graph disconnect, but two collectible | ||
| // states leave meta clean — | ||
| // 1. a parentless (`parent_count == 0`) task kept alive only by a pin, then unpinned: | ||
| // `transient_ref_count` is a *transient* field, so dropping it to 0 tracks nothing; | ||
| // and | ||
| // 2. a task persisted parentless in a prior session, restored this cycle (restore sets no | ||
| // modified flags) and collected with no intervening mutation. | ||
| // In those cases this call is what carries the tombstone into the snapshot (it flips | ||
| // `has_modifications`, so the modified scan runs). When meta is already dirty (the common | ||
| // cascade case) it's a cheap no-op. Runs before `start_snapshot`, so it takes the | ||
| // not-in-snapshot-mode path and bumps the shard modified count. The returned `TrackOutcome` | ||
| // is only for undo; GC's mark is permanent, so discard it. | ||
| let _ = task.track_modification(SpecificTaskDataCategory::Meta, "gc_deleted"); | ||
| } |
Contributor
Author
There was a problem hiding this comment.
all the comments here can be massively simplified
soft deletion is necessary because of racing edge deletions and track_modification is required to ensure that snapshotting sees it
actually i wonder if deleted should be a meta category if only to trigger implicit track_modification behavior
it would never actually be persisted which is slightly amusing but it would simplify reasoning
lukesandberg
commented
Jul 20, 2026
Comment on lines
+287
to
+323
| // Once the backend is stopping, GC bookkeeping is irrelevant (the whole task map is torn | ||
| // down in `stop()`), so pin/unpin become no-ops. This also makes them safe against handles | ||
| // finalized during shutdown — e.g. a `DetachedVc` handed to JS across NAPI is dropped | ||
| // (which unpins) during Node worker teardown, *after* `stop()` has dropped the map. | ||
| if self.stopping.load(Ordering::Acquire) { | ||
| return; | ||
| } | ||
| // Build a normal (operation-guarded) execute context so pin cannot interleave with a GC | ||
| // pass: it runs strictly before or strictly after a collection, never concurrently with | ||
| // `is_gc_collectible` / task removal. This is deadlock-free because neither pin caller | ||
| // holds a guard already — `prevent_gc` runs in the unguarded user-future region, | ||
| // and `DetachedVc` pins from a NAPI thread — and the GC pass itself never calls | ||
| // pin/unpin. | ||
| let mut ctx = self.execute_context(turbo_tasks); | ||
| // A pin is an in-session reference from outside the tracked graph (an explicit | ||
| // `prevent_gc`, or a detached handle like `DetachedVc` holding the task's `OperationVc` | ||
| // across NAPI). It is counted the same way a transient parent's edge is: bump | ||
| // `transient_ref_count`. While that count is > 0 the task is uncollectible (see | ||
| // `is_gc_collectible`) and unevictable (see `evictability`, so the transient count can't be | ||
| // lost). Counting (rather than a bool flag) makes nested/cloned pins correct — each pin is | ||
| // balanced by its own unpin. | ||
| // | ||
| // Use the non-inserting `resident_task`: a pin always targets a live reference, so the task | ||
| // must be resident. A missing entry means the caller pinned an already-collected task (a | ||
| // "zombie `OperationVc`") — a bug we surface via debug_assert rather than paper over by | ||
| // resurrecting a blank entry (which would leave an orphaned zombie in the map). Going | ||
| // through the guard routes the count through the same `update_and_get_*` mutation the | ||
| // `AdjustTransientRefCount` queue job uses. | ||
| let existed = ctx.resident_task(task); | ||
| debug_assert!( | ||
| existed.is_some(), | ||
| "pin_task_for_gc: task {task} has no resident entry (pinned an already-collected \ | ||
| task?)" | ||
| ); | ||
| if let Some(mut guard) = existed { | ||
| guard.update_and_get_transient_ref_count(1); | ||
| } |
Contributor
Author
There was a problem hiding this comment.
comments are too volumninous
lukesandberg
commented
Jul 20, 2026
| // `ReadWriteOnShutdown` drain path drops the whole map wholesale (no per-cycle eviction | ||
| // needed), and `ReadOnly` never persists/GCs. In debug builds, refuse the unsafe combo by | ||
| // forcing GC off with a warning; release builds trust the caller's configuration. | ||
| let mut gc_enabled = gc::gc_enabled(); |
Contributor
Author
There was a problem hiding this comment.
should the gc_enabled function/lazylock just be moved here?
lukesandberg
force-pushed
the
lukesandberg/gc-02-tombstone-plumbing
branch
from
July 20, 2026 23:18
14080b5 to
5dbf6f4
Compare
lukesandberg
force-pushed
the
lukesandberg/gc-03-parent-count-collection
branch
from
July 20, 2026 23:18
5d4aadd to
d370353
Compare
lukesandberg
commented
Jul 21, 2026
Comment on lines
+336
to
+339
| /// A snapshot run after a [`Self::gc_for_testing`] pass commits that pass's tombstones | ||
| /// automatically: GC leaves collected tasks resident with their `deleted` flag set, and the | ||
| /// snapshot derives the on-disk tombstones from that flag (no tombstones need threading in). | ||
| /// |
Contributor
Author
There was a problem hiding this comment.
pointless comment, drop it
lukesandberg
commented
Jul 21, 2026
Comment on lines
+1024
to
+1038
| // Garbage-collection pass runs immediately before the snapshot, so we can compute keys to | ||
| // tombstone in the database and apply it during the snapshot. Opt-in via TURBO_ENGINE_GC. | ||
| // Run the GC pass and transition to the snapshot **atomically**, without ever releasing | ||
| // operation exclusion in between. `begin_gc` drains all operations; `gc_collect` tears down | ||
| // collectible tasks and cascades `parent_count` decrements to their children; then | ||
| // `GcPhase::into_snapshot` hands the exclusion straight to the snapshot phase (swapping the | ||
| // GC request bit for the snapshot bit under one lock). Because no operation can run between | ||
| // the cascade and the snapshot, a task the cascade decremented cannot be resurrected before | ||
| // it is persisted — so the decremented children counts and the snapshot are consistent, and | ||
| // GC-collected tasks' tombstones ride this same commit. | ||
| // | ||
| // Without GC we just begin the snapshot directly. `deletes` are the GC tombstones for this | ||
| // commit: in the production (GC-enabled) path they are produced right here under the *same | ||
| // continuous exclusion* as the snapshot (the atomic `into_snapshot` hand-off), so nothing | ||
| // could have resurrected a collected task. |
Contributor
Author
There was a problem hiding this comment.
simplify the comment
lukesandberg
commented
Jul 21, 2026
Comment on lines
+1077
to
+1078
| // No tasks modified since the last snapshot (GC-collected tasks count as modified) — | ||
| // drop the guard (which calls end_snapshot) and skip the expensive O(N) scan. |
lukesandberg
commented
Jul 21, 2026
Comment on lines
+1287
to
+1294
| // A GC-soft-deleted task is not persisted; instead its on-disk copy (task meta/data + | ||
| // its `TaskCache` entry) is tombstoned in this same commit. We represent that as a | ||
| // `SnapshotItem::Delete` so it rides the streaming shard iterator that `save_snapshot` | ||
| // consumes (rather than a side-channel that would have to be fully populated before the | ||
| // put loop runs). GC marks such tasks modified so the per-shard scan visits them here, | ||
| // and the iterator clears their modified flags, so a still-`deleted` task not | ||
| // hard-deleted this cycle won't be re-tombstoned next snapshot. Only persistent tasks | ||
| // are collected, so a persistent task type (the `TaskCache` key) is always present. |
Contributor
Author
There was a problem hiding this comment.
for tasks that are new and delete we should skip persisting them altogether, there is nothing to tombstone
lukesandberg
commented
Jul 21, 2026
Comment on lines
+2183
to
+2184
| // Note: dirtying any unfinished new children is folded into `connect_children`'s | ||
| // parent_count pass (a single guard per child), so there is no separate dirty pass here. |
lukesandberg
commented
Jul 21, 2026
Comment on lines
+306
to
+313
| /// Whether this increase is for a task being connected as a genuinely-new child (the | ||
| /// direct-connect chokepoint in `ConnectChildOperation::run`). When set, the handler also | ||
| /// resurrects the task if GC had soft-deleted it — riding the child guard it takes anyway, | ||
| /// so the common (not-deleted) case is a free Meta flag read. `false` for the aggregation | ||
| /// propagation producers of this job, which never target a `deleted` task (a collected | ||
| /// task has no `upper`/`followers` to be propagated to). | ||
| #[bincode(skip, default = "Default::default")] | ||
| resurrect: bool, |
Contributor
Author
There was a problem hiding this comment.
i wonder if we need this bit? couldn't we just always ressurrect on activation, it would be redundant sometimes, but checkined a deleted flag on a guard we already have is fine
lukesandberg
commented
Jul 21, 2026
| // only when actually deleted does `resurrect_deleted` consume it, re-acquire `All` to | ||
| // double-check and atomically clear+re-dirty, and hand back a fresh guard. | ||
| if resurrect && task.deleted() { | ||
| task = crate::backend::operation::connect_child::resurrect_deleted( |
Contributor
Author
There was a problem hiding this comment.
import the function
lukesandberg
commented
Jul 21, 2026
| // real edge removal. Gating on the `remove_children` return makes | ||
| // this replay-safe: a resumed CleanupOldEdges re-removes the same | ||
| // edges, and edges already gone (returning false) are skipped. | ||
| let mut removed_persistent_children: SmallVec<[TaskId; 4]> = |
Contributor
Author
There was a problem hiding this comment.
we probably don't need the explicit type declaration
lukesandberg
commented
Jul 21, 2026
Comment on lines
+111
to
+118
| // Each removed persistent child loses a parent. A persistent parent | ||
| // drops the child's durable `parent_count` (queued so it is | ||
| // crash-consistent); a transient parent drops the session-only | ||
| // `transient_ref_count` (skipped during serialization, never | ||
| // replayed). Both ride the queue via the same job family, so the | ||
| // handler — not this site — adjusts the right counter and takes the | ||
| // child guards (avoiding a second guard while `task` is still | ||
| // held). |
Contributor
Author
There was a problem hiding this comment.
simplify comment
lukesandberg
commented
Jul 21, 2026
Comment on lines
+207
to
+213
| // Under the GC phase, a collected task stays resident | ||
| // (soft-deleted) so this scrub | ||
| // finds a live entry; a non-resident target would mean | ||
| // `ctx.task` is about to resurrect an already-collected task from | ||
| // disk (the bug soft-deletion fixes). `gc_target_resident` is | ||
| // `None` outside GC, where | ||
| // restoring a missing target is legitimate. |
Contributor
Author
There was a problem hiding this comment.
simplify comment, we should also generalize this check since it generic for gc corrupts state
lukesandberg
commented
Jul 21, 2026
Comment on lines
+17
to
+35
| /// Revive a task the caller observed as GC-soft-deleted (via `guard.deleted()` on a guard it | ||
| /// already holds during the connect handshake). The caller passes that guard **by value**; this | ||
| /// consumes it, performs the revival, and returns a freshly re-acquired guard of the same | ||
| /// `category`, so the call site is simply `guard = resurrect_deleted(guard, ..)` with the | ||
| /// drop/re-acquire encapsulated here. | ||
| /// | ||
| /// The passed guard is dropped and an `All` guard re-acquired (needed for the `immutable()` read), | ||
| /// under which `deleted` is **re-checked** — double-checked locking: between the caller's cheap | ||
| /// peek and this re-acquire, a concurrent connect of the same task could have already revived it, | ||
| /// in which case there is nothing to do. When still deleted, the clear and the re-dirty happen | ||
| /// **under that single `All` guard without an intervening drop**, so no operation can observe the | ||
| /// intermediate `!deleted && !dirty` state (a task that looks live but still holds the stale/empty | ||
| /// edges GC scrubbed). A **mutable** task is cleared and made dirty so it re-executes and rebuilds | ||
| /// those edges; an **immutable** task cannot be made dirty (invariant in `make_task_dirty`) and | ||
| /// does not need to be — its output is deterministic and its edges self-contained — so clearing the | ||
| /// marker alone suffices, leaving it immediately valid for any concurrent observer. | ||
| /// | ||
| /// GC collection is the single producer of the `deleted` flag and runs under an exclusion that | ||
| /// stops all operations, so once cleared here the flag cannot be re-set concurrently. |
Contributor
Author
There was a problem hiding this comment.
simplify comment
lukesandberg
commented
Jul 21, 2026
Comment on lines
+86
to
+94
| // Resurrection of a GC-soft-deleted child (clearing the `deleted` marker + re-dirtying it) | ||
| // is folded into the child guard the connect handshake takes below — the activeness path | ||
| // does it in `increase_active_count`, the non-activeness path inline — so the common | ||
| // (not-deleted) case costs only a Meta flag read on a guard we hold anyway, with no extra | ||
| // acquisition. A `deleted` child is always in NEITHER the parent's `children` nor its | ||
| // `new_children` when first connected (GC only collects `parent_count == 0` tasks, so it is | ||
| // in no live parent's children; and a prior connect in this execution would have already | ||
| // staged+resurrected it), so the dedup early-returns below never skip a needed | ||
| // resurrection. |
lukesandberg
commented
Jul 21, 2026
Comment on lines
+147
to
+152
| // Peek for GC soft-deletion on the guard we already hold — common path: one Meta flag | ||
| // read, no extra lock. Rare (was deleted): `resurrect_deleted` consumes this guard, | ||
| // re-acquires `All` to double-check and atomically clear+re-dirty, and hands back a | ||
| // fresh Meta guard for the schedule decision below. Resurrection thus completes before | ||
| // scheduling, matching the original resurrect-first order. | ||
| if child_task.deleted() { |
Contributor
Author
There was a problem hiding this comment.
now that ressurrect_deleted takes a guard
it should really include the deleted check insternally and can just be child_task = resurrect_delete and it can sometimes be a no-op
lukesandberg
commented
Jul 21, 2026
Comment on lines
+61
to
+96
| // Single pass over the newly-connected children doing two things per child under one guard, | ||
| // so we don't lock each child twice (once to bump its ref count, once to dirty it): | ||
| // | ||
| // 1. Maintain the child-side parent reference count for **persistent** children: a | ||
| // persistent parent bumps the durable `parent_count`, a transient parent the | ||
| // session-only `transient_ref_count` (not persisted; transient parents re-establish | ||
| // their edges by re-executing on restart). Transient children are never collected, so | ||
| // they take no count. | ||
| // | ||
| // CRITICAL: this is a **direct** adjustment, NOT a queued `AdjustParentCount` job, and | ||
| // it MUST run before `queue.execute` below (the first `operation_suspend_point` | ||
| // this function reaches). GC reads `parent_count` to decide collectibility and | ||
| // only observes state at suspend points (`begin_gc` drains operations to a | ||
| // suspended/quiescent state). If the `+1` rode the `AggregationUpdateQueue` | ||
| // instead, that queue's per-iteration suspend point could suspend with the `+1` | ||
| // still pending — leaving a window where the `children` edge is committed | ||
| // (`extend_children` in the caller) but `parent_count` is not yet incremented. A | ||
| // GC pass landing in that window would read the under-counted `parent_count == | ||
| // 0`, judge the (genuinely reachable) child collectible, and delete it. There is | ||
| // no `operation_suspend_point` between the caller's `extend_children` and here | ||
| // (`make_task_dirty_internal` only enqueues; the queue runs later), so no snapshot/GC | ||
| // can observe the edge without the count. In the parallelized path this runs on | ||
| // a child context (which never suspends — it holds no operation guard), inside | ||
| // the `scope_and_block` barrier that completes within the parent operation, so | ||
| // the property holds there too. (The `-1` on edge removal in `CleanupOldEdges` | ||
| // has the opposite, benign failure mode — a pending `-1` makes GC | ||
| // *under*-collect for one pass, self-correcting — so it stays on the durable | ||
| // queue there.) | ||
| // | ||
| // 2. Make any child that has not produced output yet dirty, so it gets scheduled and | ||
| // computes. This was formerly a separate `for_each_task_all` pass over the same children | ||
| // (`task_execution_completed_unfinished_children_dirty`) before the connect; folding it | ||
| // here reuses this guard and, on the parallelized path, chunks it with the rest. It only | ||
| // runs on the successful connect (not the stale/cancel early-returns in | ||
| // `task_execution_completed_connect`), which is consistent with those paths undoing the | ||
| // children's temporarily-increased active count rather than keeping them. |
Contributor
Author
There was a problem hiding this comment.
simplify comment
we don't need to discuss the various alternatives that were explored
lukesandberg
commented
Jul 21, 2026
| task_id, | ||
| true, | ||
| #[cfg(feature = "task_dirty_cause")] | ||
| crate::backend::operation::invalidate::TaskDirtyCause::Unknown, |
Contributor
Author
There was a problem hiding this comment.
Create a new TaskDirtyCause Resurrected
lukesandberg
commented
Jul 21, 2026
Comment on lines
+203
to
+207
| /// GC-only: ids whose persistent `parent_count` reached 0 during this context's operations | ||
| /// (recorded by `note_gc_parent_count_zeroed`). `None` for normal operation contexts (the | ||
| /// recording hook is a no-op there); `Some` only for the GC context, drained by the collector | ||
| /// via [`Self::take_gc_zeroed`] to discover cascade-collectible tasks. | ||
| gc_zeroed: Option<Vec<TaskId>>, |
Contributor
Author
There was a problem hiding this comment.
we should consider having a dedicated trait implementation for the GC context instead of these optional fields
lukesandberg
commented
Jul 21, 2026
Comment on lines
+427
to
+436
| /// Atomically transitions from the GC phase directly into a snapshot phase **without ever | ||
| /// releasing operation exclusion**. Under the state lock it clears the GC request bit and sets | ||
| /// the snapshot request bit in one critical section, so no operation can start in between (an | ||
| /// operation blocks while *either* bit is set). This closes the race where a mutation could | ||
| /// resurrect a just-collected task in the gap between the GC pass and the snapshot: with an | ||
| /// atomic hand-off there is no such gap, so the GC cascade's `parent_count` decrements and the | ||
| /// snapshot see a consistent graph. | ||
| /// | ||
| /// Because operations were already drained to zero by `begin_gc`, no further draining is | ||
| /// needed. |
Contributor
Author
There was a problem hiding this comment.
simplfiy comment
lukesandberg
force-pushed
the
lukesandberg/gc-02-tombstone-plumbing
branch
from
July 21, 2026 16:08
5dbf6f4 to
73dc675
Compare
lukesandberg
force-pushed
the
lukesandberg/gc-03-parent-count-collection
branch
from
July 21, 2026 16:08
d370353 to
223e74f
Compare
lukesandberg
force-pushed
the
lukesandberg/gc-02-tombstone-plumbing
branch
from
July 21, 2026 16:29
73dc675 to
b5ba401
Compare
lukesandberg
force-pushed
the
lukesandberg/gc-03-parent-count-collection
branch
from
July 21, 2026 16:29
223e74f to
8bc08cb
Compare
lukesandberg
force-pushed
the
lukesandberg/gc-02-tombstone-plumbing
branch
from
July 21, 2026 16:41
b5ba401 to
011b444
Compare
lukesandberg
force-pushed
the
lukesandberg/gc-03-parent-count-collection
branch
from
July 21, 2026 16:41
8bc08cb to
388dd36
Compare
lukesandberg
force-pushed
the
lukesandberg/gc-02-tombstone-plumbing
branch
from
July 21, 2026 16:46
011b444 to
d903753
Compare
lukesandberg
force-pushed
the
lukesandberg/gc-03-parent-count-collection
branch
2 times, most recently
from
July 21, 2026 17:02
2839c72 to
b9f23e7
Compare
lukesandberg
force-pushed
the
lukesandberg/gc-02-tombstone-plumbing
branch
from
July 21, 2026 17:02
d903753 to
37de3fe
Compare
lukesandberg
force-pushed
the
lukesandberg/gc-03-parent-count-collection
branch
2 times, most recently
from
July 21, 2026 20:07
0b657eb to
b9a39db
Compare
…tion Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
lukesandberg
changed the base branch from
lukesandberg/gc-02-tombstone-plumbing
to
graphite-base/95976
July 22, 2026 05:05
lukesandberg
force-pushed
the
lukesandberg/gc-03-parent-count-collection
branch
from
July 22, 2026 05:05
b9a39db to
f844d18
Compare
lukesandberg
changed the base branch from
graphite-base/95976
to
lukesandberg/gc-025-task-access
July 22, 2026 05:05
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
1 participant
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.
What
The core of the persistent-engine garbage collector: a durable per-task
parent_count(number of persistent parents that list the task as a child), maintained incrementally, plus the pass that collects tasks whose count reaches 0.Reference counting
parent_countandtransient_ref_countas inline meta fields.CleanupOldEdges).operation_suspend_pointbetween committing thechildrenedge and bumping the count, or a GC/snapshot could observe a reachable child asparent_count == 0and delete it. The −1 stays queued (its window is benign — a pending −1 only under-collects for one pass).Collection
backend/gc.rs: the pass. A self-feeding parallel job pool (scope_self_feeding) re-validates eachparent_count == 0candidate, tears it down viaCleanupOldEdges(dropping children's counts, scrubbing reverse-dep edges, rebalancing the aggregation graph), soft-deletes it (kept resident, tombstoned at the next snapshot), and cascades into children driven to 0.snapshot_coordinator:begin_gc/begin_snapshotexclusion + an atomic GC→snapshot hand-off that closes a resurrection race.prevent_gc()as a GC pin (transient_ref_count); pin/unpin are exclusion-safe and no-op once the backend is stopping.save_snapshotis parallelized/inlined.Why
This replaces scan-based GC for the persistent engine: a count can only drop to 0 while the parent is in memory, so collectibility is detectable at that moment with no DB scan.
Safety
Opt-in via
TURBO_ENGINE_GC(default off). No behavior change unless enabled. The subtle invariants (suspend-free count maintenance, the GC→snapshot hand-off, soft-deletion + resurrection) are the review focus.Tests
gc_stress(re-rooting stays flat, wide-fanout collection),gc_resurrection(forward-dep scrub / reconnect), and theparent_countcounting + collection suite.Stack
Depends on the tombstone-plumbing and scope PRs below it.