Add opt-in subtree-keyed caching to the worker DirectoryCache#2541
Open
erneestoc wants to merge 2 commits into
Open
Add opt-in subtree-keyed caching to the worker DirectoryCache#2541erneestoc wants to merge 2 commits into
erneestoc wants to merge 2 commits into
Conversation
The directory cache is keyed only on the root Directory digest, so one changed file out of thousands forces a full re-materialization (measured 1400ms of tree walk + per-file hardlinks + proto decodes) even though a warm full-root hit is 69ms and nearly every blob is already in the fast tier. REAPI Merkle nodes are content-addressed, so unchanged subtrees are byte-identical across roots and can be reused wholesale. With `experimental_subtree_caching` enabled (default: false), `create_subdirectory` routes each subdirectory through the same core get-or-create flow root materializations use, keyed by the subtree's own Directory digest: a hit is one `hardlink_directory_tree` call; a miss constructs the subtree once into its own cache entry (single-flighted through the shared `construction_locks` map; deadlock-free since flights only wait on strict Merkle descendants) and recursion caches nested subtrees too. New `subtree_hits`/`subtree_misses`/`evictions` counters in `CacheStats` surface reuse and thrash. To keep eviction meaningful, an entry's recorded size covers only bytes not owned by a descendant entry, so the map's size total approximates unique materialized bytes instead of counting each file once per ancestor level. Because subtree caching turns several previously rare root-path hazards into routine events, the shared flow is also hardened (these fixes apply to root-only caching as well): - Unified core: `get_or_create_entry` + `try_materialize_from_cache` + `construct_and_materialize` now carry ONE copy of the hit/miss logic for both root and subtree materializations. - RAII refcount pins: `CachedDirectoryMetadata.ref_count` is an `Arc<AtomicUsize>` released by `EntryPin::drop`, so futures cancelled mid-await (e.g. `buffer_unordered` dropping siblings on the first error) can no longer leak pins and leave entries permanently unevictable. `last_access` is an atomic too, letting the hit path run under the cache READ lock instead of convoying on the write lock. - Temp+rename publish: constructions build into a unique `cache_root/.tmp-N` and atomically rename to the canonical path, so a failed construction can never poison its (stable, shared) digest with a partial tree; failures clean up inline and a `ScratchGuard` sweeps temps of cancelled futures. - Tombstone eviction: victims are renamed to `cache_root/.del-N` under the same write-lock critical section that removed them from the map (rename is metadata-cheap; deletion stays off-lock in the background), so background deletion can never race a re-construction publishing to the canonical path. `DirectoryCache::new` sweeps pre-existing `cache_root` content the same way (the map starts empty, so anything on disk is orphaned and would poison re-constructions). - Dead-entry invalidation + clean retry: when hardlinking from an entry fails, the entry is invalidated (guarded by refcount-handle identity so a freshly rebuilt entry is never nuked) and the partially populated destination is removed before reconstruction — previously the retry hit "destination already exists"/AlreadyExists and the dead map entry kept failing its digest forever. - `hardlink_directory_tree` now chmods created directories to 0o755 (unix), keeping the documented invariant under restrictive umasks. Tests cover one-file-changed churn reuse (exact hit/miss accounting plus byte verification), the default-off regression guard, cross-root subtree sharing, eviction safety under concurrent construction, construction- failure retry (no digest poisoning, no temp leftovers), pin drain after cancelled sibling futures, damaged-entry recovery, and non-depth- multiplied size accounting. Observability: DirectoryCache implements MetricsComponent (clonefile_hits, hardlink_hits, subtree_hits, subtree_misses, evictions, entries, total_size_bytes) and is wired into the worker metrics tree via a new directory_cache field on the running-actions-manager Metrics component (Weak ref, absent when the cache is disabled). entries/total_size_bytes are lock-free atomic mirrors maintained at insert/evict/invalidate. A rate-limited (1/min) tracing::info! "DirectoryCache summary" line from the hit/miss path covers deployments without a metrics pipeline. Config: the cache-wide fetch semaphore size (previously the hardcoded CONSTRUCT_DIRECTORY_MAX_FETCHES=64 from TraceMachina#2526) is now DirectoryCacheConfig::max_concurrent_fetches (serde default 64 — behavior-preserving; validated > 0 at construction with InvalidArgument). Low values fragment coalesced read batches, so deployments using experimental_read_batching on the underlying grpc store can raise it substantially (e.g. 512). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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
The worker
DirectoryCacheis keyed only on the rootDirectorydigest, so any input change is a full miss: one changed file out of 4400 re-materializes the entire tree. Measured on a churn-shaped workload (the dominant case for incremental builds): the one-file-changed reconstruction costs 1855 ms of almost purely local work — only 4 slow-store fetches; the rest is tree-walking, per-file hardlinks, and proto decodes for 4399 unchanged files.This adds opt-in subtree caching: every subdirectory is additionally cached under its own
Directorydigest, andcreate_subdirectoryconsults the cache before constructing. REAPI's Merkle design makes this sound by construction —Directorynodes are content-addressed, so an unchanged subtree has an identical digest across roots (and across different actions' input trees). With the flag on, the one-file-changed case reuses all unchanged subtrees via whole-subtreehardlink_directory_tree(APFSclonefileon macOS) and constructs only the changed spine.Default off: existing behavior byte-for-byte (guarded by a regression test asserting zero subtree cache activity and correct results with the flag unset).
Measurements
4400-file tree (20 mids x 10 leaves x 22 files), latency-injected slow tier,
A'= same tree with exactly one file changed:The win scales with tree size and churn locality; cross-root sharing means common subtrees (SDKs, framework bundles) are reused across different actions' input roots, not just successive versions of one root.
Caveat on absolute local timings: captured under a test harness whose
capturing TRACE subscriber adds measurable overhead — treat the table as
same-harness A/B. Production evidence: with this flag + read coalescing on
macOS workers (4-build sweep, true cold), per-exec setup time -67% avg
cold and -48% avg incremental — the incremental number is the one this
feature targets.
Scope and companion changes
This PR makes repeat/churn materialization cheap (warm reuse). The
cold construct cost itself is addressed independently: on macOS, most
of it was per-blob
F_FULLFSYNCinFilesystemStore, fixed by the flushcoalescing /
sync_policychanges (cold 4,400-file construct ~14.5s ->~3s in the same benchmark). All of these are orthogonal and compose;
subtree hits (~0.1-0.5s) remain far below even the improved cold path.
Design
construction_locks— a root-levelget_or_createand a subtree construction of the same digest share one flight and one entry. Cache paths use the existingcache_root/<digest>scheme, so a subtree entry can later serve a root-level request and vice versa.Directorycannot contain an ancestor — that would be a hash cycle).ref_count: 1) with the same digest-derived size accounting as root entries, hardlinked into the parent, then released — the existingref_count == 0eviction filter covers it, mirroringconstruct_and_materialize. Hardlink failures fall back to reconstruction (hit path) or direct in-place construction (post-construction path), mirroring the root-level fallbacks.subtree_hits/subtree_missescounters alongside the existingclonefile_hits/hardlink_hits.max_size_bytesonce per entry.Testing
(mids-1)+(leaves-1)subtree hits and exactly 2 misses (changed mid + changed leaf); full-tree byte verification including the changed file.max_entries: 3vs 4 concurrent multi-entry trees on a multi-threaded runtime, all byte-verified.bazel test //nativelink-worker:integration //nativelink-config:unit_testgreen (clippy + rustfmt aspects);cargo check --testsandcargo fmt --checkclean.macOS flush-coalescing (Coalesce FilesystemStore durability flushes on macOS #2539) PRs: zero conflicts, all 40 bazel tests
across config/store/worker green on the combined branch.
This change is