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

`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.

Record (mtime_ns, len) alongside the digest and settle the common case -- nothing
changed since the last run -- with one `stat`. The digest remains 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 on a 51,513-file application, A/B against main in one hyperfine run:

  main             4.945s +/- 0.118
  this branch      3.450s +/- 0.117    1.43x faster

  user time        7.534s -> 6.877s
  system time     12.927s -> 8.495s    (-34%)

An earlier batch on a busier machine measured the same pair at 1.30x. Both are
valid single-batch comparisons; the ratio moves with load because the phases this
does not touch are a larger share when the machine is quiet. The system-time drop
is the stable signal, and it is the mechanism: this phase is syscall-bound, not
CPU-bound. 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 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 cannot tell us, so the stat is discarded and the digest
carries the entry. This needed no new branches at the call sites -- `None`
already meant "no usable stat" -- and both ways of being wrong fail safe:

- Coarse filesystem: nothing is trusted, the fast path never engages. Correct,
  just not faster.
- Fine filesystem, mtime landing exactly on a second boundary: a 1-in-10^9
  coincidence costing one extra hash. Measured across 20,003 files of a real
  Rails application: zero occurrences.

Cost of the check itself: 1.00x +/- 0.02 against the same branch without it.

It narrows rather than closes the window, and the type's docs say so. A
millisecond-granularity filesystem is trusted, so two same-length writes inside
one millisecond would still be missed -- six orders of magnitude tighter, and
needing machine-speed edits to reach. Also documented: mtimes that are copied
rather than set by writing (rsync -t, tar -p, cp -p) can carry a timestamp from
elsewhere; every mtime-driven cache shares that hole, which is why they all
document `touch` as the way to force a rebuild.

## packwerk compatibility

Verified against packwerk 3.3.0, and the concern turned out to be misplaced:

- Its `Cache::CacheContents.deserialize` uses plain hash access and never
  enumerates keys, so an unknown key is invisible to it. Ran its logic against a
  packwerk-format entry carrying `source_stat`: reads fine.
- The tools do not share a directory. packwerk reads `tmp/cache/packwerk/<md5>`;
  pks writes `tmp/cache/packwerk/zeitwerk/<md5>`.
- The formats were never interchangeable. Feeding packwerk what pks writes today
  raises `NoMethodError: undefined method 'map' for nil`. That predates this
  change, so `test_compatible_with_packwerk` does not test what its name claims;
  it round-trips pks's own format. Left alone, but it is not a guarantee.

Regardless, `source_stat` is `#[serde(default, skip_serializing_if)]`, so an
entry without one still deserializes and is still honored via the digest.

## Failure modes closed by construction

- `EmptyCacheEntry` holds `Option<String>` rather than an empty string meaning
  "not computed", private behind `digest()`. `write` errors instead of persisting
  a placeholder, which would have produced an entry that never matches -- making
  that file permanently uncacheable and silently slow.
- The in-place repair warns on failure rather than 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 is a stat worth recording. Without the guard,
  a filesystem yielding `None` every run would never match and would rewrite the
  entire cache every time.

## Tests

tests/cache_stat_fastpath_test.rs, ten cases. Note that before this change *no
test in the repo exercised a warm cache at all* -- every fixture ships
`cache: false` -- so these paths were untested rather than under-tested.

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; a
stale stat with a matching digest is repaired in place; a malformed `source_stat`
(five shapes) degrades to the digest without panicking.

Other commands: `pks update` on a warm cache -- the highest-consequence path,
since update *writes* package_todo.yml and a stale entry persists a wrong answer
rather than printing one -- and the experimental parser, which uses a different
cache subdirectory but shares this implementation.

Each was verified to fail rather than assumed to pass: injecting a bug that makes
the cache always hit fails 7 of the 10, and the granularity and repair guards
were separately confirmed to fail with their own checks removed.

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

Verified: `check` and `check --no-cache` produce identical output on the 51k-file
application, as do this branch and main. Across the 30 fixture apps with a
packwerk.yml, 29 are byte-identical; the 30th is app_with_monkey_patches, which
trips the pre-existing nondeterministic duplicate-constant panic in both binaries.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Author: perryqh

src/packs/caching/mod.rs

@@ -1,5 +1,7 @@
 use std::path::{Path, PathBuf};
 
+use serde::{Deserialize, Serialize};
+
 use super::{file_utils::file_content_digest, ProcessedFile};
 pub(crate) mod cache;
 pub(crate) mod noop_cache;
@@ -10,34 +12,155 @@ pub enum CacheResult {
     Miss(EmptyCacheEntry),
 }
 
+/// Cheap identity for a source file, obtained from one `stat` call.
+///
+/// The content digest remains the authority on whether a cache entry is valid.
+/// This exists only so that the common case -- nothing changed since the last
+/// run -- can be settled without opening and hashing the file.
+///
+/// # Only used where the filesystem timestamps finely enough to be trusted
+///
+/// Treating a matching (mtime, len) as "unchanged" is only sound if every write
+/// moves the mtime, which is a filesystem property rather than a guarantee. On a
+/// filesystem with one-second granularity -- some Docker bind mounts on macOS,
+/// NFS, SMB, FAT -- a file edited to *the same length* within the same second as
+/// it was cached keeps both its mtime and its length, and a stat-only check
+/// would happily serve the stale entry.
+///
+/// Rather than assume, this detects it per file: a filesystem that reports a
+/// non-zero sub-second component is one that tracks sub-second time, so an edit
+/// at any other instant *would* have moved the mtime. When the component is zero
+/// the stat is discarded and the caller falls back to hashing the contents,
+/// which is always correct.
+///
+/// The consequences of being wrong run the safe direction in both cases:
+///
+/// - Coarse filesystem: every mtime is a whole second, every file falls back to
+///   the digest, and the fast path simply does not engage. Correct, no faster.
+/// - Fine filesystem, and a file whose mtime lands exactly on a second boundary:
+///   a 1-in-10^9 coincidence that costs one extra hash for that file. Measured
+///   on a 20,003-file Rails application: **zero** files hit it.
+///
+/// This narrows rather than closes the window. A filesystem with, say,
+/// millisecond granularity reports a non-zero sub-second component and is
+/// trusted, so two same-length writes inside one millisecond would still be
+/// missed. That is six orders of magnitude tighter than the one-second case and
+/// requires machine-speed edits to reach.
+///
+/// # The remaining hole: mtimes that are copied rather than set by writing
+///
+/// The check above establishes that the *filesystem* would have moved the mtime.
+/// It cannot establish that nobody moved it back. Tools that deliberately
+/// preserve timestamps -- `rsync -t`, `tar -p`, `cp -p`, unzip, some
+/// backup/restore and container-image flows -- can install different content
+/// carrying an mtime from somewhere else. If that mtime and the length both
+/// happen to match what was cached, the fast path serves a stale entry.
+///
+/// In practice this needs the replacement to match the cached version in both
+/// mtime and byte length, which usually means restoring a near-identical copy of
+/// what was already there. It is not specific to this design: `make`, `ccache`
+/// and every other mtime-driven cache have the same hole, which is why they all
+/// document `touch` as a way to force a rebuild.
+///
+/// If it ever bites, `--no-cache` is the escape hatch, and `pks delete-cache`
+/// clears the state. A tool-side fix would mean giving up on stat-only
+/// validation and always hashing, which is precisely the cost this exists to
+/// avoid.
+#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq)]
+pub struct SourceStat {
+    /// Nanoseconds since the unix epoch. u64 is good until the year 2554.
+    pub mtime_ns: u64,
+    pub len: u64,
+}
+
+const NANOS_PER_SEC: u64 = 1_000_000_000;
+
+impl SourceStat {
+    /// `None` whenever the stat cannot be trusted as a change detector: the file
+    /// cannot be stat'd, has no mtime, has one before the unix epoch or too far
+    /// in the future to represent, or -- see the type docs -- carries no
+    /// sub-second precision. Every such case falls back to the content digest,
+    /// which is authoritative anyway, and which will produce a sensible error if
+    /// the file is genuinely unreadable.
+    pub fn of(path: &Path) -> Option<SourceStat> {
+        let metadata = std::fs::metadata(path).ok()?;
+        let since_epoch = metadata
+            .modified()
+            .ok()?
+            .duration_since(std::time::UNIX_EPOCH)
+            .ok()?;
+
+        // `try_from` rather than `as`, which would silently wrap a far-future
+        // mtime into a small value that could collide with a real one.
+        let mtime_ns = u64::try_from(since_epoch.as_nanos()).ok()?;
+
+        // No sub-second component means this filesystem cannot tell us about a
+        // change made within the same second. Do not trust it.
+        if mtime_ns % NANOS_PER_SEC == 0 {
+            return None;
+        }
+
+        Some(SourceStat {
+            mtime_ns,
+            len: metadata.len(),
+        })
+    }
+}
+
 #[derive(Debug, Default)]
 pub struct EmptyCacheEntry {
     #[allow(dead_code)]
     pub filepath: PathBuf,
-    pub file_contents_digest: String,
+    /// `None` until [`Self::populate_digest`] computes it. Private so that
+    /// "not computed yet" cannot be mistaken for a digest: writing an entry
+    /// without one would persist a value that never matches, quietly making
+    /// that file uncacheable forever.
+    file_contents_digest: Option<String>,
     #[allow(dead_code)]
     pub file_name_digest: String,
     pub cache_file_path: PathBuf,
+    pub source_stat: Option<SourceStat>,
 }
 
 impl EmptyCacheEntry {
-    pub fn new(
+    /// The parts of a cache entry that can be derived without reading the file's
+    /// contents. Reading + MD5-ing the source is the expensive half, so it is
+    /// deferred until something actually needs the digest.
+    pub fn without_digest(
         cache_directory: &Path,
         filepath: &Path,
     ) -> anyhow::Result<EmptyCacheEntry> {
         let file_digest = md5::compute(filepath.to_str().unwrap());
         let file_name_digest = format!("{:x}", file_digest);
         let cache_file_path = cache_directory.join(&file_name_digest);
 
-        let file_contents_digest = file_content_digest(filepath)?;
-
         Ok(EmptyCacheEntry {
             filepath: filepath.to_owned(),
-            file_contents_digest,
+            file_contents_digest: None,
             cache_file_path,
             file_name_digest,
+            source_stat: SourceStat::of(filepath),
         })
     }
+
+    /// Reads and hashes the file, at most once per entry.
+    pub fn populate_digest(&mut self) -> anyhow::Result<&str> {
+        if self.file_contents_digest.is_none() {
+            self.file_contents_digest =
+                Some(file_content_digest(&self.filepath)?);
+        }
+        Ok(self
+            .file_contents_digest
+            .as_deref()
+            .expect("just populated above"))
+    }
+
+    /// The digest, if it has been computed. `None` means no one has called
+    /// [`Self::populate_digest`] -- see the field comment for why that must not
+    /// be treated as an empty digest.
+    pub fn digest(&self) -> Option<&str> {
+        self.file_contents_digest.as_deref()
+    }
 }
 
 pub fn create_cache_dir_idempotently(cache_dir: &Path) {

src/packs/caching/per_file_cache.rs

@@ -11,41 +11,107 @@ use tracing::warn;
 use super::cache::Cache;
 use super::CacheResult;
 use super::EmptyCacheEntry;
+use super::SourceStat;
 
 pub struct PerFileCache {
     pub cache_dir: PathBuf,
 }
 
 impl Cache for PerFileCache {
     fn get(&self, path: &Path) -> anyhow::Result<CacheResult> {
-        let empty_cache_entry = EmptyCacheEntry::new(&self.cache_dir, path)
-            .context(format!("Failed to create cache entry for {:?}", path))?;
-        let cache_entry = CacheEntry::from_empty(&empty_cache_entry)?;
-        if let Some(cache_entry) = cache_entry {
-            let file_digests_match = cache_entry.file_contents_digest
-                == empty_cache_entry.file_contents_digest;
-
-            if !file_digests_match {
-                Ok(CacheResult::Miss(empty_cache_entry))
-            } else {
-                let processed_file = cache_entry.processed_file;
-                Ok(CacheResult::Processed(processed_file))
+        // Deliberately does not read the source file yet. On a warm cache the
+        // stat below settles the overwhelming majority of files, and reading
+        // every source file to MD5 it was roughly half the cost of this phase.
+        let mut empty_cache_entry =
+            EmptyCacheEntry::without_digest(&self.cache_dir, path).context(
+                format!("Failed to create cache entry for {:?}", path),
+            )?;
+
+        let Some(cache_entry) = CacheEntry::from_empty(&empty_cache_entry)?
+        else {
+            empty_cache_entry.populate_digest()?;
+            return Ok(CacheResult::Miss(empty_cache_entry));
+        };
+
+        // Fast path: the file has the same mtime and length as when we cached
+        // it, so it cannot have changed in any way we care about.
+        //
+        // `is_some()` is not redundant with the equality check and must not be
+        // folded into it. Both sides are `None` whenever no usable stat exists --
+        // on a filesystem too coarse to be trusted, every file every run (see
+        // `SourceStat`) -- and `None == None` is true. Without this, "we have no
+        // idea whether the file changed" would read as "the file is unchanged",
+        // serving stale entries on exactly the filesystems the stat check exists
+        // to protect. Covered by `test_whole_second_mtime_is_not_trusted`.
+        if cache_entry.source_stat.is_some()
+            && cache_entry.source_stat == empty_cache_entry.source_stat
+        {
+            return Ok(CacheResult::Processed(cache_entry.processed_file));
+        }
+
+        // Slow path: no stat recorded (entry predates this feature, or was
+        // written by packwerk), or the stat moved. The content digest is still
+        // the authority, so fall back to it.
+        let digest = empty_cache_entry.populate_digest()?;
+        if cache_entry.file_contents_digest != digest {
+            return Ok(CacheResult::Miss(empty_cache_entry));
+        }
+
+        // Contents are unchanged but the stat differs -- a checkout, a `touch`,
+        // or an entry written before stats were recorded. Refresh the entry so
+        // the next run takes the fast path.
+        //
+        // Only when there is actually a usable stat to record, and it differs
+        // from what is on disk. Without this guard, a filesystem too coarse to
+        // produce a trustworthy stat (see `SourceStat`) would yield `None` on
+        // every run, never match, and rewrite every cache entry every time --
+        // turning a read-mostly cache into a full rewrite of itself.
+        let stat_is_worth_recording = empty_cache_entry.source_stat.is_some()
+            && empty_cache_entry.source_stat != cache_entry.source_stat;
+
+        if stat_is_worth_recording {
+            // A failure here is not fatal: the result we return is still
+            // correct, we just re-hash this file on the next run too. It is
+            // warned about rather than ignored, because a persistent failure
+            // (an unwritable cache dir, a full disk) degrades every subsequent
+            // run and would otherwise be invisible -- the tool would simply be
+            // slow forever with no clue why.
+            if let Err(e) =
+                self.write(&empty_cache_entry, &cache_entry.processed_file)
+            {
+                warn!(
+                    "Failed to refresh cache entry {:?}; it will be re-hashed \
+                     on every run until this succeeds: {}",
+                    empty_cache_entry.cache_file_path, e
+                );
             }
-        } else {
-            Ok(CacheResult::Miss(empty_cache_entry))
         }
+
+        Ok(CacheResult::Processed(cache_entry.processed_file))
     }
 
     fn write(
         &self,
         empty_cache_entry: &EmptyCacheEntry,
         processed_file: &ProcessedFile,
     ) -> anyhow::Result<()> {
-        let file_contents_digest =
-            empty_cache_entry.file_contents_digest.to_owned();
+        // A missing digest means a caller reached `write` without hashing the
+        // file. Erroring is deliberate: persisting a placeholder would produce
+        // an entry that never matches, making the file permanently uncacheable
+        // and silently slow.
+        let file_contents_digest = empty_cache_entry
+            .digest()
+            .with_context(|| {
+                format!(
+                    "Refusing to write a cache entry for {:?} with no content digest",
+                    empty_cache_entry.filepath
+                )
+            })?
+            .to_owned();
 
         let cache_entry = &CacheEntry {
             file_contents_digest,
+            source_stat: empty_cache_entry.source_stat,
             // Ideally we could pass by reference here, but in practice this cost should be paid on few files
             // that have changed and need to be reprocessed.
             processed_file: processed_file.clone(),
@@ -81,6 +147,11 @@ impl Cache for PerFileCache {
 #[derive(Debug, Serialize, Deserialize, PartialEq, Eq)]
 pub struct CacheEntry {
     pub file_contents_digest: String,
+    /// 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>,
     pub processed_file: ProcessedFile,
 }
 
@@ -169,6 +240,8 @@ mod tests {
 
         let expected_serialized = CacheEntry {
             file_contents_digest: "8f9efdcf2caa22fb7b1b4a8274e68d11".to_owned(),
+            // A packwerk-written entry carries no stat; it must still deserialize.
+            source_stat: None,
             processed_file: ProcessedFile {
                 absolute_path: PathBuf::from("/tests/fixtures/simple_app/packs/foo/app/services/bar/foo.rb"),
                 unresolved_references: vec![UnresolvedReference {
@@ -210,7 +283,7 @@ mod tests {
         fs::write(corrupt_file_path, corrupt_contents)
             .context("expected to write corrupt cache file")?;
 
-        let empty_cache_entry = EmptyCacheEntry::new(
+        let empty_cache_entry = EmptyCacheEntry::without_digest(
             &cache_path,
             &PathBuf::from(
                 "tests/fixtures/simple_app/packs/foo/app/services/foo/bar.rb",