Skip to content

fix(store): speed up cold installs - #267

Merged
jdx merged 7 commits into
mainfrom
codex/bench-results-refresh
Apr 24, 2026
Merged

jdx merged 7 commits into
mainfrom
codex/bench-results-refresh

Conversation

@jdx

@jdx jdx commented Apr 24, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Fix the cold-cache install bottlenecks in aube's CAS import and package materialization paths, keep CAS entries readable after temp-file persist, and refresh the benchmark matrix from a hermetic Verdaccio + 500mbit run.

The first root cause was aube-store calling sync_all() for every individual content-addressed file during tarball import. The benchmark fixture imports roughly 33k CAS files on a cold install, so per-file fsync dominated wall time. The store now writes non-empty CAS entries through a sibling temp file and persists them with no-clobber semantics instead of fsyncing every file.

A second regression from the beta.2-era cold path was per-package materialization sentinels in aube-linker: each package wrote .aube-initialized through atomic_write, which fsyncs. That added roughly one fsync per materialized package and pushed GVS cold installs back behind pnpm. The linker now removes those sentinels, since callers already avoid invoking materialize_into for existing package dirs and GVS materialization writes into fresh temp dirs before rename. The patch sidecar write also avoids fsync; if it is lost or corrupt, the next install conservatively relinks patched entries.

Follow-up fixes preserve Unix CAS file readability at 0644 and reject cached package indices when sampled/all store-file metadata is missing or size-mismatched, so missing/truncated CAS content is re-fetched instead of silently reused. This also probes auto link strategy against the project root rather than a not-yet-created node_modules, allowing hardlink/reflink detection to succeed before first install.

Benchmark Matrix

The old cold ci / warm ci names were misleading because those rows used aube's global virtual store. The benchmark now splits them into:

  • Fresh install (warm cache, GVS) and Fresh install (cold cache, GVS) for the local default path.
  • CI install (warm cache, GVS disabled) and CI install (cold cache, GVS disabled) with aube run under CI=1.

Latest hermetic mise run bench:bump results:

Scenario aube bun pnpm npm yarn
Fresh install, warm cache, GVS 0.162s 0.429s 1.015s 2.765s 2.456s
Fresh install, cold cache, GVS 1.346s 0.931s 1.577s 4.195s 6.589s
CI install, warm cache, GVS disabled 0.238s 0.438s 1.028s 2.789s 2.509s
CI install, cold cache, GVS disabled 1.270s 0.925s 1.585s 4.197s 6.672s
npm install && npm run test 0.009s 0.025s 0.442s 0.602s 0.344s
Add dependency 0.199s 0.422s 1.343s 2.908s 2.532s

Focused beta.2/current check on the same machine showed the regression and fix clearly:

Build GVS cold aube
v1.0.0-beta.2 1.102s
current before linker sentinel fix 2.325s
current after linker sentinel fix 1.375s

The landing/README copy continues to use the first row, Fresh install (warm cache, GVS), as the headline comparison.

Validation

  • flock /tmp/aube-bench.lock mise run bench:bump
  • focused beta.2/current cold GVS comparison under hermetic Verdaccio + 500mbit
  • cargo test -p aube-store --release
  • cargo test -p aube-linker --release
  • cargo clippy -p aube-store --all-targets -- -D warnings
  • cargo clippy -p aube-linker --all-targets -- -D warnings
  • cargo clippy -p aube --all-targets -- -D warnings
  • cargo fmt --check
  • bash -n benchmarks/bench.sh
  • node --check benchmarks/generate-results.js
  • git diff --check
  • pre-commit hooks: Rust fmt/clippy, shfmt, shellcheck

Note

Medium Risk
Touches core install storage/linking paths (CAS writes, index validation, linker materialization), so mistakes could cause corrupted/inefficient installs even though changes are performance-focused and guarded by new tests.

Overview
Cold-install performance is improved by changing CAS writes in aube-store to use a temp file + persist_noclobber instead of per-file fsync, and by removing the .aube-initialized sentinel write/read from aube-linker materialization.

Store correctness checks are tightened: cached package indexes are now rejected when sampled/all CAS entries are missing or size-mismatched, and Unix CAS entries are persisted as 0644 (with new tests covering these cases). The install link-strategy auto-probe now checks across the store dir and project root (not node_modules) so it works before first install.

Benchmarks/docs are refreshed: the benchmark matrix splits “fresh install (GVS)” vs “CI install (GVS disabled via CI=1)”, and README/docs/results JSON are updated accordingly.

Reviewed by Cursor Bugbot for commit 1bd2c44. Bugbot is set up for automated code reviews on this repo. Configure here.

@jdx
jdx force-pushed the codex/bench-results-refresh branch from 9ea1b50 to 04287af Compare April 24, 2026 14:03
@greptile-apps

greptile-apps Bot commented Apr 24, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes two cold-install performance bottlenecks: removing per-file sync_all() on CAS imports (replaced by temp-file + persist_noclobber) and removing per-package .aube-initialized sentinel writes from the linker. It also fixes the auto-link-strategy probe to work before node_modules exists, adds size-based CAS cache invalidation, preserves 0644 permissions on imported files, and refreshes the benchmark matrix with clearer GVS vs CI-mode scenario labels.

Confidence Score: 4/5

Safe to merge pending resolution of the open std::fs::write crash-safety concern for .aube-applied-patches.json noted in a previous thread.

All core store and linker logic is well-structured and documented. The no-fsync tradeoff is sound and backed by the size-mismatch invalidation. The one outstanding concern (std::fs::write for patch sidecar) was flagged in a prior thread and remains unaddressed in this revision.

crates/aube-linker/src/lib.rs — the patch sidecar write path (around the previously-flagged std::fs::write call) deserves a final look before merge.

Important Files Changed

Filename Overview
crates/aube-store/src/lib.rs Replaces per-file fsync with temp-file/persist_noclobber for non-empty CAS entries; adds size-based metadata validation to cache invalidation; sets 0644 permissions on CAS files; adds two new regression tests. Logic is sound and the no-fsync tradeoff is well-documented.
crates/aube-linker/src/lib.rs Removes the .aube-initialized sentinel read/write from materialize_into and drops dirs_skipped from LinkStats. The PR description explains callers guarantee a fresh target dir, making the sentinel redundant. The previously-flagged std::fs::write for .aube-applied-patches.json remains.
crates/aube/src/commands/install/lifecycle.rs Fixes auto-link-strategy probe to use cwd instead of cwd/node_modules, allowing hardlink/reflink detection to succeed before first install when node_modules doesn't yet exist.
benchmarks/bench.sh Adds two new GVS scenarios (gvs-warm, gvs-cold) and adds CI=1 to aube's ci-warm/ci-cold commands to properly disable the global virtual store in those rows. Non-aube tools are unaffected.
benchmarks/generate-results.js Adds gvs-warm and gvs-cold entries to the benchmark list to match new scenario keys in bench.sh.
benchmarks/results.json Updated benchmark data with new gvs-warm and gvs-cold rows and revised timings. The toolchain version question was addressed in a previous thread.
README.md Headline benchmark copy updated to reflect new scenario naming and revised performance numbers.
docs/benchmarks.md Scenario descriptions updated to match the new GVS/CI split and the explanation of CI=1 disabling the global virtual store.

Reviews (7): Last reviewed commit: "linker: keep patch sidecar writes atomic" | Re-trigger Greptile

Comment thread benchmarks/results.json Resolved
@jdx
jdx force-pushed the codex/bench-results-refresh branch 2 times, most recently from cc657e2 to 440f5f6 Compare April 24, 2026 14:57
@jdx jdx changed the title bench: refresh package manager results store: fix cold CI import bottleneck Apr 24, 2026
@jdx jdx changed the title store: fix cold CI import bottleneck fix(store): avoid fsyncing every cold import file Apr 24, 2026
@jdx
jdx marked this pull request as ready for review April 24, 2026 15:00
Comment thread crates/aube-store/src/lib.rs Resolved
Comment thread crates/aube-store/src/lib.rs Resolved
@jdx
jdx force-pushed the codex/bench-results-refresh branch 2 times, most recently from 27bda9d to 2210be2 Compare April 24, 2026 15:51
@jdx jdx changed the title fix(store): avoid fsyncing every cold import file fix(store): speed up cold installs Apr 24, 2026
@jdx
jdx force-pushed the codex/bench-results-refresh branch from 2210be2 to 23ba689 Compare April 24, 2026 16:29
jdx added a commit that referenced this pull request Apr 24, 2026
Use the new `gvs-warm` and `install-test` rows (added in #267) and
reuse #267's sharper wording ("Warm installs with the global virtual
store", "Repeat test commands"). Drops the generic "Warm CI" /
"Across the fixture set" framing in favor of row-specific language.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 23ba689. Configure here.

Comment thread crates/aube-linker/src/lib.rs Resolved
Comment thread crates/aube-linker/src/lib.rs Outdated
let out = serde_json::to_string(map)
.map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidData, e))?;
aube_util::fs_atomic::atomic_write(&path, out.as_bytes())
std::fs::write(path, out)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 The switch from aube_util::fs_atomic::atomic_write to std::fs::write removes crash safety for .aube-applied-patches.json. std::fs::write truncates the file first, so a SIGKILL or power-loss during the write leaves malformed JSON at the path. On the next install, the parser fails to read it, causing either all patches to be re-applied (double-patching, corrupting installed files) or none (patches silently skipped). The surrounding comment and the original choice of atomic_write exist precisely to prevent this stale-patch scenario.

Suggested change
std::fs::write(path, out)
aube_util::fs_atomic::atomic_write(&path, out.as_bytes())

Fix in Claude Code

@jdx
jdx force-pushed the codex/bench-results-refresh branch from 23ba689 to 1bd2c44 Compare April 24, 2026 17:00
@jdx
jdx merged commit 780af28 into main Apr 24, 2026
18 checks passed
@jdx
jdx deleted the codex/bench-results-refresh branch April 24, 2026 17:14
jdx added a commit that referenced this pull request Apr 24, 2026
Use the new `gvs-warm` and `install-test` rows (added in #267) and
reuse #267's sharper wording ("Warm installs with the global virtual
store", "Repeat test commands"). Drops the generic "Warm CI" /
"Across the fixture set" framing in favor of row-specific language.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@greptile-apps greptile-apps Bot mentioned this pull request Jun 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant