Skip reading source files on a warm cache hit via mtime + length

[perryqh]

1.43× faster on pks check, measured against current main in a single hyperfine run.

What it does

process_files_with_cache is the largest phase of pks check, and half of it is work we can avoid: for every file we open it, read it in full, and MD5 it, purely to compare a digest against the cache entry.

This records (mtime_ns, len) alongside the digest and settles the common case — nothing changed since the last run — with one stat.

The digest stays the authority. If no stat is recorded, or the stat moved, we fall back to reading and hashing exactly as before. An entry whose contents match but whose stat moved (a git checkout, a touch) is repaired in place so the next run takes the fast path.

Measured

51,513-file application, warm cache, both binaries in one hyperfine run:

	main	this branch
wall clock	4.945 s ± 0.118	3.450 s ± 0.117	1.43× faster
user time	7.534 s	6.877 s	−8.7%
system time	12.927 s	8.495 s	−34%

Phase breakdown:

phase	main	this branch
process_files_with_cache	3.817 s	1.862 s
directory walk	1.387 s	1.408 s
autoload-path glob	1.043 s	1.021 s
run checkers	0.565 s	0.578 s

An earlier batch on a busier machine measured the same pair at 1.30×. Both are valid single-batch A/B runs; the ratio moves with load because the phases this doesn't touch are a larger share of the total when the machine is quiet.

The system-time drop is the stable signal, and it's the mechanism: this phase is syscall-bound, not CPU-bound. It removes 51,513 file opens and full reads per run. An earlier attempt to speed the same phase up by parsing JSON faster (from_reader → from_slice) changed nothing measurable, which is what pointed at the syscalls.

Coarse filesystems are detected, not assumed away

Trusting (mtime, len) is only sound where the filesystem timestamps finely enough to notice a write. At one-second granularity — some Docker bind mounts on macOS, NFS, SMB, FAT — a same-length edit inside the same second keeps both fields, and a stat-only check would serve the stale entry.

Rather than probe the filesystem or assume a platform, SourceStat::of reads the value it already has: a non-zero sub-second component proves the filesystem tracks sub-second time, so an edit at any other instant would have moved the mtime. A zero component means it can't tell us, so the stat is discarded and the digest carries the entry.

fine.rb      mtime_ns=1787343439414352315  sub-second=414352315  -> trust
coarse.rb    mtime_ns=1787000000000000000  sub-second=        0  -> use digest

No new branches were needed at the call sites — None already meant "no usable stat, use the digest". Both ways of being wrong fail safe:

	outcome
Coarse filesystem	nothing trusted → fast path never engages. Correct, just not faster.
Fine filesystem, mtime exactly on a second boundary	1-in-10⁹ → one extra hash. Measured across 20,003 real files: zero occurrences.

Cost of the check: 1.00 ± 0.02 against the same branch without it — unmeasurable.

I did consider git's "racily clean" rule (record when the entry was written; re-hash anything whose mtime falls in a granularity window of it). It doesn't transfer cleanly — git's index timestamp comes from the same filesystem as the files, whereas our write time would come from a fine-grained clock and the mtime from a coarse one, comparing incommensurable units. The sub-second check is cheaper and self-calibrating.

The hole that remains

The check proves the filesystem would have moved the mtime. It can't prove nobody moved it back. Tools that preserve timestamps — rsync -t, tar -p, cp -p, unzip, some backup and container-image flows — can install different content carrying an mtime from elsewhere; if both that mtime and the length match what was cached, the fast path serves a stale entry.

Reaching it needs the replacement to match in mtime and byte length, which in practice means restoring a near-identical copy of what was already there. It also isn't specific to this design — make, ccache and every other mtime-driven cache share it, which is why they all document touch as the way to force a rebuild. Closing it would mean always hashing, i.e. giving up the entire win.

Documented on SourceStat rather than left implicit, along with the escape hatches that already exist: --no-cache and pks delete-cache.

packwerk cache compatibility

This adds a key to the cache JSON, so I treated "does this break packwerk?" as blocking. It doesn't. Checked against packwerk 3.3.0:

fed to packwerk 3.3.0's deserializer	result
packwerk's own format	✅
packwerk's format + source_stat	✅
what pks writes today	❌ NoMethodError

1. Its deserializer ignores unknown keys. Cache::CacheContents.deserialize uses plain hash access and never enumerates keys — there's no T::Struct.from_hash, which would have been strict.
2. They don't share a directory. packwerk reads tmp/cache/packwerk/<md5>; pks writes tmp/cache/packwerk/zeitwerk/<md5>. On the app I measured: 51,708 files under zeitwerk/, and the only top-level file is constant_resolver.json, also pks's.
3. The formats were never interchangeable — row 3 above. pks nests unresolved_references under processed_file and names the fields differently. That predates this PR.

So test_compatible_with_packwerk doesn't test compatibility with packwerk; it round-trips pks's own format. Left alone — renaming it is a separate change — but it shouldn't be read as a guarantee.

Belt and braces regardless: source_stat is #[serde(default, skip_serializing_if = "Option::is_none")], so an entry without it still deserializes and is still honored via the digest.

Note

Pre-existing interaction, not caused by this PR: packwerk's bust_cache! does FileUtils.rm_rf(tmp/cache/packwerk), which deletes pks's zeitwerk/ subdirectory too. Running packwerk after a packwerk.yml or inflections change forces a full pks cold rebuild. Performance only, not correctness.

Failure modes closed by construction

• A cache entry cannot exist without a digest. CacheLookup holds what is knowable without opening the file (cache path, stat); read_contents consumes it to produce the EmptyCacheEntry that write takes. So write needs no digest check — the only way to hold its argument is to have read the file. An entry carrying a placeholder digest would never match, making that file permanently uncacheable and silently slow; there is no state in which one can be built.
• The in-place repair warns on failure instead of discarding the error. The result stays correct either way, but a persistent failure (unwritable cache dir, full disk) would otherwise leave every run re-hashing with no clue why.
• That repair only fires when there's a stat worth recording. Without the guard, a filesystem yielding None every run would never match and would rewrite the entire cache every time — turning a read-mostly cache into a full rewrite of itself.

Tests

tests/cache_stat_fastpath_test.rs, ten cases.

Before this branch, no test in the repo exercised a warm cache at all — every fixture ships cache: false. These paths weren't under-tested, they were untested.

The fast path: stats are recorded · warm output matches cold · an edit invalidates · a same-length edit invalidates (the case a length-only check would miss) · a whole-second mtime is not trusted.

Fallback and repair: a stat-less packwerk-style entry is honored then upgraded in place · a stale stat with a matching digest is repaired · a malformed source_stat (five shapes: not an object, wrong type, missing field, null, negative) degrades to the digest without panicking.

Other commands: pks update on a warm cache — the highest-consequence path, since update writes package_todo.yml, so a stale entry persists a wrong answer rather than printing one · the experimental parser, which uses tmp/cache/packwerk/experimental but shares this implementation, so a regression confined there would be invisible.

Uses common::Fixture from #57 rather than a local copy helper.

Each test was verified to fail

A test never seen failing isn't yet evidence of anything. Injecting a bug that makes the cache always hit fails 7 of the 10:

test test_update_is_correct_on_a_warm_cache ... FAILED
test test_edited_file_invalidates_cache ... FAILED
test test_stale_stat_with_matching_digest_is_repaired ... FAILED
test test_experimental_parser_uses_the_fast_path_correctly ... FAILED
test test_entry_without_stat_is_still_valid ... FAILED
test test_same_length_edit_invalidates_cache ... FAILED
test test_whole_second_mtime_is_not_trusted ... FAILED

The granularity guard and the repair path were separately confirmed to fail with their own checks removed.

Review follow-ups

• write's digest precondition was a runtime check. Now structural — see "Failure modes closed by construction" above. populate_digest/digest() are gone along with the error path, so the suggested test for that path has nothing left to assert.
• The mtime test shelled out to touch. Replaced with File::set_modified (stable since 1.75, toolchain is 1.97.1) — no dev-dependency, no external binary, works on Windows.
• Both mutation checks re-verified after the refactor: removing the sub-second guard still fails test_whole_second_mtime_is_not_trusted; forcing the fast path to always hit still fails the same 7 of 10.

Verification

• cargo test — 268 passing (258 + 10 new)
• cargo clippy --all-targets --all-features and cargo fmt --all -- --check — clean
• cargo doc --no-deps — clean
• On the 51k-file application: check and check --no-cache produce identical output, and so do this branch and main
• Across the 30 fixture apps with a packwerk.yml: 29 byte-identical. The 30th is app_with_monkey_patches, which trips the pre-existing nondeterministic duplicate-constant panic in both binaries — the unmodified binary names a different constant run to run.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds a stat-based “fast path” for per-file cache validation to avoid hashing unchanged files, and pins the expected behaviors with integration tests.

Changes:

• Introduces SourceStat and defers content-digest computation until needed.
• Updates PerFileCache::get to prefer stat equality and fall back to digest validation.
• Adds a comprehensive integration test suite covering invalidation, coarse mtimes, malformed cache entries, and update behavior.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
tests/cache_stat_fastpath_test.rs	New integration tests that exercise the stat fast path and its safety fallbacks.
src/packs/caching/per_file_cache.rs	Implements the stat fast path and cache-entry refresh behavior; persists source_stat.
src/packs/caching/mod.rs	Adds SourceStat and refactors EmptyCacheEntry to lazily compute the content digest.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/packs/caching/per_file_cache.rs:140

• source_stat is deserialized as Option<SourceStat>, so any malformed source_stat value (wrong shape/type) will cause deserialization of the entire CacheEntry to fail and the entry to be treated as corrupt. That forces a full reparse/rehash even when file_contents_digest and processed_file are still valid. If the intent is to “degrade to digest” when source_stat is malformed, consider a tolerant deserialize_with that treats invalid values as None while still loading the rest of the entry.

    /// Absent in entries written by packwerk, or by versions of pks before the
    /// stat fast path existed. `serde(default)` keeps those entries readable;
    /// they simply fall back to comparing the content digest.
    #[serde(default, skip_serializing_if = "Option::is_none")]
    pub source_stat: Option<SourceStat>,