Skip to content

Commit 2d1ecdf

Browse files
authored
sync: stat each file once per scan, not twice (#66)
`scan_folder` called `child.metadata()` twice for every file it keeps. `DirEntry::metadata` is a fresh `lstat` on each call, so the second one bought nothing: let (mtime_secs, mtime_nanos) = mtime_of(&child); // lstat let disk = child.metadata().ok(); // lstat again `mtime_of_metadata` already existed and takes a `&Metadata`, so the single-call shape was in the file the whole time. `mtime_of` was a `DirEntry` wrapper around it that forced the extra call. It has no other caller and is removed. MEASURED, AND IT IS A SMALL WIN. Do not read it as a large one. The watched tree on Silber holds 17,063 files. One `lstat` pass over all of them costs 36 ms wall on this machine, warm, best of three. The bus entry ran 12.2 scans a minute over a 419 second window, so the redundant call costs about 0.43 s of CPU a minute, or 0.72 percent of one core. Treat 0.72 percent as an UPPER bound. I measured the syscall from Python, whose per-call interpreter overhead is included in the 2.08 us per file and would not be present in this loop. WHAT THIS DOES NOT FIX, SO NOBODY READS IT AS THE ANSWER. The dominant idle cost on Silber is that one changed file triggers a full walk of the whole tree, about 21 times a minute. Halving the stats per file makes the walk cheaper. It does not stop the walk from happening. Agent: Silber.fabric
1 parent 5ae4abd commit 2d1ecdf

1 file changed

Lines changed: 9 additions & 8 deletions

File tree

src/sync/engine.rs

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1342,8 +1342,16 @@ fn scan_folder(
13421342
if !entry.includes(&norm) {
13431343
continue;
13441344
}
1345-
let (mtime_secs, mtime_nanos) = mtime_of(&child);
1345+
// One stat per file, not two. `DirEntry::metadata` is a fresh
1346+
// lstat on every call, and this loop used to call it twice: once
1347+
// inside `mtime_of` and again on the next line. `mtime_of_metadata`
1348+
// already takes the result, so the single-call shape was in the
1349+
// file the whole time.
13461350
let disk = child.metadata().ok();
1351+
let (mtime_secs, mtime_nanos) = disk
1352+
.as_ref()
1353+
.map(mtime_of_metadata)
1354+
.unwrap_or((0, 0));
13471355
let size = disk.as_ref().map(|meta| meta.len()).unwrap_or(u64::MAX);
13481356
let executable = disk.as_ref().is_some_and(is_executable);
13491357
// Reuse the recorded hash when size and both mtime components are
@@ -1832,13 +1840,6 @@ fn mtime_of_metadata(meta: &std::fs::Metadata) -> (i64, u32) {
18321840
}
18331841
}
18341842

1835-
fn mtime_of(entry: &std::fs::DirEntry) -> (i64, u32) {
1836-
let Ok(meta) = entry.metadata() else {
1837-
return (0, 0);
1838-
};
1839-
mtime_of_metadata(&meta)
1840-
}
1841-
18421843
/// Resolve the tombstone sweep window from `FABRIC_TOMBSTONE_SWEEP_DAYS`.
18431844
///
18441845
/// Unset means the sweep is OFF. That is deliberate: forgetting a tombstone is

0 commit comments

Comments
 (0)