perf(walk): improvements to the walker hot path#575
Open
arcuru wants to merge 6 commits into
Open
Conversation
Assisted-by: Claude Opus 4.7 (code generation, refactoring, code review)
Syscall reduction verified via strace on a 3-dir / 5-file test tree: statx count drops from 11 to 8, exactly one per directory saved. Assisted-by: Claude Opus 4.7 (code review, refactoring)
Assisted-by: Claude Opus 4.7 (code review, refactoring)
Also thread metadata through the ignore_file -> build_node boundary so filter-active walks (-x / -M / -A / -y) stat each entry at most once. Correctness verified by diffing output against the pre-change binary with default flags, -i, -e, -v, -X, -M, -x. Assisted-by: Claude Opus 4.7 (code review, refactoring)
Assisted-by: Claude Opus 4.7 (code review, refactoring)
Under `-f` the file's stat is mostly thrown away — `node_from_tuple` sets size to 1 and the time fields are only read when -M/-A/-y are on. The only field still needed downstream is `inode_device` for `clean_inodes` dedup, and both halves are syscall-free: `inode` from `DirEntry::ino()` (getdents64), `dev` from the parent directory's cached tuple. Gated to `-f` without `-L` and without metadata-needing filters, so output is byte-identical to the previous stat path. Unix-only; Windows DirEntry has no cheap d_ino analogue. Assisted-by: Claude Opus 4.7 (code review, refactoring)
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.
More changes as a stack on top of my previous #574. Ideally this would be stacked commits but right now this will show the prev commit inside this PR...
Happy to split/rebase as requested, though I am also fine if you pull/modify yourself.
Five small reductions on the per-entry walker hot path. None of them change observable behavior they just remove redundant work. They compound for noticeable wins on tree-heavy and wide-directory walks.
Per-dir stat cache.
walk_dir()was statting each directory for itsis_dir()check, thenbuild_node()statted it again to populate the Node. Cache the parsed metadata tuple inPendingDirviaOnceLock. Onestatxsaved per directory.Cache
entry.path()/entry.file_type(). Both were called 2-3× per entry, andentry.path()allocates a freshPathBufeach call. Compute once inprocess_entryand thread through. Also gateis_ignored_pathon a cheap empty-check so walks without--ignore-directoryskip the HashSet probe entirely.Skip
ignore_fileon default walks. Precomputehas_any_filterwhen constructingWalkData. In the hot loop, three-way branch: fullignore_filewhen filters are active, inlined dot-file check when only--ignore-hiddenis on, otherwise skip. When filters are active, also coalesce the duplicateget_metadatacalls insideignore_file, replacepath.is_file()stats with the cachedfile_type, hoistfs::canonicalizeout of theignore_directoriesloop, and thread the prefetched tuple intobuild_nodeso each entry is statted at most once on filter-active walks.FxHash for inode dedup.
clean_inodeshashes 25M(inode, dev)pairs on my test/nix/store. SipHash's DoS resistance is pointless for primitive keys from our own syscalls. Addsrustc-hash = "2", swapsHashSetforFxHashSet. This can be inlined to avoid the dep by defining a simple hasher.Skip per-file
statxin--filecountmode. Under-f, every file's size is overwritten with 1, so the per-filestatxis removable. The only field the dedup still needs is(inode, dev): inode comes fromDirEntry::ino()(filled bygetdents64), dev from the parent directory's cached tuple. Gated to-fwithout-Land without metadata-needing filters, so output is byte-identical to the previous stat path. Unix-only; WindowsDirEntryhas no cheapd_inoanalogue.Benchmarks
Cumulative for this batch of changes. "Before" is inclusive of #574.
host: 24 CPU; hyperfine
--warmup 2 --runs 10for synthetics,--warmup 1 --runs 3-5for large targets-e "\.rs$"regex filter on dust src-xdust src-fbalanced-fwide_flat-f/nix/storeDefault-walk synthetics (
balanced,wide_flat,~) are tighter on this run. Those workloads are already close to the syscall floor; the per-entry CPU savings show up in user time more than wall time. I also sampled the system times for these runs, and they are more dramatic where they apply:-e-75% sys,-x-41% sys,-f /nix/store-61% sys. strace confirms-f wide_flatgoes from ~100kstatxcalls to ~10.Further Work
One more potential perf change stacked on top of this:
AT_STATX_DONT_SYNCon Linux. Biggest win on network filesystems (~3.8× on an 11 TB NFS mount in my testing); helps cold-cache local in smaller amounts. dust's defaultstd::fs::MetadatausesAT_STATX_SYNC_AS_STAT, which for a read-only walker is stricter coherence than we need.That is a small behavior change though, which may or may not be acceptable.