Skip to content

vfs: inline READDIRPLUS attributes the tree can size cheaply - #10

Merged
Andrew Gazelka (andrewgazelka) merged 1 commit into
ix-patchedfrom
vfs-readdirplus-attributes
Aug 1, 2026
Merged

vfs: inline READDIRPLUS attributes the tree can size cheaply#10
Andrew Gazelka (andrewgazelka) merged 1 commit into
ix-patchedfrom
vfs-readdirplus-attributes

Conversation

@andrewgazelka

@andrewgazelka Andrew Gazelka (andrewgazelka) commented Aug 1, 2026

Copy link
Copy Markdown
Member

READDIRPLUS returned no attributes for any entry, so a client spent one LOOKUP per entry it cared about. This fills them in where the tree can size an entry cheaply.

The cost being paid today

On a cold find -type f over a real tree of 16,894 files, 4,302 directories and 21,196 entries, the walk issued 19,242 LOOKUPs at 1.026 per directory entry, 70.6% of every RPC it made. That is the readdir-driven walk a build actually performs, not a synthetic pure-stat sweep.

What the lookups were, which is not what it first looked like

They were not redundant. On the original tree, 18,718 lookups against 18,721 entries is 0.9998 per entry — exactly one per distinct path component, none repeated. The directory vnode cache was already perfect and there was no duplication to remove.

An earlier framing of "1.27 lookups per stat" came from dividing by stats rather than by entries; the excess over 1.0 was simply the directory components, each resolved once. Recording that because it changes what the fix is: the only way to remove these lookups was to stop needing them, which is what inlining attributes does.

Why this is not a trade

A LOOKUP reply already computes exactly these attributes. Measured with proc_pid_rusage across a cold sweep, an entire LOOKUP RPC — XDR decode, protocol, handler, size(), file_size(), XDR encode — costs 0.616 us of server CPU. Filling a READDIRPLUS entry therefore costs strictly less than that.

Under 0.6 us of local work against a 63.9 us round trip. About 100x favourable per entry.

Ceiling, stated as a ceiling

Before measuring, the predicted ceiling was 27,244 RPCs down to roughly 8,000, taking about 1.23 s off a 2.81 s cold walk, about 1.8x. Not "faster" — that is the most the change can be worth.

Measured

Same repo, same revision, two mountpoints, one binary each, cold walk on each. Control 23 RPCs / 20 s idle.

cold find -type f before after
total RPCs 31083 9931
Lookup 22477 1320
RdirPlus 4305 4310
Access 4301 4301
wall 1.44 s 1.14 s

Repeat on a fresh cold mount: 9937 RPCs, Lookup 1320 exactly.

Total RPCs 3.13x fewer. LOOKUPs 17.0x fewer, 94.1% eliminated. Lookups per entry 1.060 -> 0.062.

Wall clock understates it: the patched server was a debug build and still beat an optimized one. The ratio is the number to read, not the time.

What tree these numbers are from. Both arms mounted ~/.config/nix at revision 0fc2d7cd0c22 — the nix-config repo, 16,894 files and 4,302 directories. Earlier drafts of this description called it "the ix tree", which was wrong: on this machine ix is a derived view served from a separate ix-view ref, and a mount taken with -r main gets nix-config. The A/B is unaffected, because both the patched and unpatched runs mounted the same source and the file and directory counts were verified equal between them, so every ratio here stands. Only the label was wrong, and it is corrected rather than quietly dropped because someone will otherwise re-derive these numbers against the wrong repo.

A first attempt at this A/B was discarded — the repo had grown from 14,720 to 16,894 files between mounting the baseline and building the patched binary, so it was comparing two different trees.

The client uses them, on macOS

The macOS NFSv3 client uses inline attributes and does not re-LOOKUP. Measured, and it was not a given. Not tested on Linux, so this is macOS-verified rather than verified.

The new floor, and why this stops here

Of the 9,934 RPCs remaining, 8,611 (87%) are RdirPlus + Access, which is 2.0 RPCs per directory across 4,302 directories — NFSv3 directory traversal, not something a server change removes.

Eliminating every one of the 1,320 residual LOOKUPs would buy a further 1.15x. I did not determine what those 1,320 are; they are not symlinks, of which there are only 29. I stopped because of that ceiling, not because the question was answered. Named here as a bounded loose thread so nobody has to rediscover it.

The honest cost

A caller that lists a directory and looks at nothing — no stat, no recursion, names only — now receives a sized fattr3 per entry it did not ask for. That is precisely what the original comment was protecting, and it is a real cost.

Measured on the largest directory in the tree, 955 entries, listing only. The extra cost is 0.134 us of server CPU per entry, and 2 additional READDIRPLUS round trips for one cold listing (3 -> 5) because sized entries are larger and fit fewer per reply. Together about 256 us for a cold 955-entry listing. Since each avoided LOOKUP is worth 63.9 us, the change breaks even if the caller looks at 4 of those 955 entries, and ls -l looks at all of them.

The recursive listing case does not pay this at all — it gets faster. A walk that never stats went 29,873 RPCs to 8,683, because omitting attributes leaves d_type unset: the client reports DT_UNKNOWN for every entry, so a portable walker must probe each one and generates the LOOKUP anyway. Verified directly — before: UNKNOWN=58 DIR=0 REG=0; after: UNKNOWN=0 DIR=30 REG=28. So today's default optimises the rarer case, and the server also burns less CPU after the change on that walk (10,121 us vs 12,445 us) because 21,195 whole RPCs stop arriving.

Scope: which workloads gain, and which do not

This helps traversal. It does very little for a package manager, and that is worth stating up front because the package-manager case is the one that prompted the filesystem work.

Another measurement of this same change found it worthless on bun install: getattr 3,397,989 -> 3,374,005 and total operations 5,430,450 -> 5,394,137, both about -0.7% and inside noise. That result is correct. So is the 3.13x above. They are different workloads, and the boundary between them is measurable.

Both runs below use the same instrument, the server's own JJ_VFS_STATS op census, against the same mount.

find -type f bun install
readdir 4310 4325
lookup 1320 119899
lookups per listing 0.31 27.7

bun install does 91x more path resolution per directory it lists. It issues 20.6 lookups for every file it creates — and a listing-derived lookup can happen at most once per entry, so anything above 1 per file has to be resolution of paths bun constructed rather than paths it discovered. That is a package manager replaying a lockfile: it already knows every path it wants.

Note it does not skip readdir entirely — 4,325 of them, comparable to the walker. It lists directories and then resolves tens of thousands of paths that were never in a listing.

The ceiling this puts on the change, for that workload. Attributes can only ever save a lookup for an entry that appeared in a listing. bun created 5,828 files and 569 directories, so at most 6,397 entries were ever listable — 5.3% of its lookups and 0.80% of its 797,696 total operations. The independently observed -0.67% sits right at that predicted ceiling. The two measurements agree; one predicts the other.

One trap in that census, for whoever reads it next. The JJ_VFS_STATS getattr counter records internal calls, not client RPCs: it read 494,589 while NFS GETATTR RPCs over the same run were 37,169, because the server calls getattr internally to fill replies for lookup, create and the rest. It is sound for comparing two runs of the same workload, which is what it is for, but it is not a count of what the client asked for, and comparing it against nfsstat output will produce a false conclusion.

So: large win for anything that lists a tree and then looks at what it found — find, fts(3), ripgrep, fd, git, ninja, and a Nix source walk. Approximately nothing for a package manager replaying a lockfile. Traversal is most of what a build does outside the package manager, which is why this is still worth landing, but it should not be advertised as speeding up bun install.

What changed

DirIterator inlines fattr3 for directories, whose size is a constant, and for files, whose length comes from the git object header.

Two cases keep None, each named with its own comment because they are where the old reasoning still holds: a conflicted path, which has no content until its sides are materialized, and a symlink, whose length needs its target fetched. The symlink case is a second live instance of the rule this PR retires, found while implementing it. NFSv3 makes the field optional per entry, so each costs one LOOKUP and nothing else.

The comment was rewritten rather than deleted. It records what the old trade was, why it was right when written, and what changed underneath it, so nobody re-adds the old behaviour and nobody reads the old decision as carelessness.

FattrContext is split out of NfsTree so the iterator can size entries without borrowing the tree, and DirIterator::next_entry lets plain READDIR skip the attribute work entirely — no effect on macOS, which only ever issues READDIRPLUS, but Linux clients do issue plain READDIR.

Tests

Two added, both watched to fail before being trusted:

  • forcing name_attributes back to None fails test_nfs_readdirplus_inlines_cheaply_sized_attributes
  • sizing conflicts inside the listing fails test_nfs_readdirplus_omits_attributes_for_a_conflict

Full jj-vfs suite 27/27 green. cargo clippy -p jj-vfs --all-features --all-targets clean.

The first push of this branch failed check (rustfmt), and the reason is worth
recording rather than quietly amending away. I verified with cargo test and
cargo clippy and reported that as passing, but the repo's gate also runs
cargo +nightly fmt --all -- --check, which I had not run. Two defects: a stray
blank line in nfs.rs left by splitting an impl block, and a hand-wrapped string
in test_nfs.rs that fits inside max_width = 100 and so is unwrapped by
format_strings = true. A pass on some of the checks is not a pass on the
checks. Fixed with cargo +nightly fmt --all, confirmed only those two files
changed, and re-ran tests and clippy afterwards rather than assuming a
formatting change was inert.

Not a mount option

Ruled out before writing any code. rdirplus is both the documented default and present on the live mounts, and the client already uses it exclusively — lifetime counters show Readdir=0 against RdirPlus=248189. mount_nfs(8) even names find . -type f as the case it should help. The client was asking correctly; the server was declining.

Rebased onto #9

#9 landed as 34ed0a3e3 and touched the same entry builder, so this was rebased onto it rather than the other way round. #9 renamed NfsTree's snapshot field to tree: Arc<OverlayTree>, moved the mode bits onto the tree, replaced the mount-wide mtime with per-entry timestamps, and added a next_cookie to DirIterator — this change is adapted to all of that, and FattrContext now carries the tree because both mode_bits and getattr live there.

The conflict was resolved by taking the new base verbatim and re-applying the change, rather than hand-editing conflict markers, so what is here is a clean re-port rather than a merge artifact. Every check was re-run on the rebased artifact — including breaking both guards again to confirm they still fire — rather than assuming the earlier verification carried.

Nothing in the 101 added lines is platform-conditional: no cfg(, no libc::, no errno comparison. Checked against a denominator, since those patterns do appear elsewhere in the crate (sys.rs, snapshot.rs, fuse.rs).

Background and the full measurement log: ENG-11746.

@andrewgazelka
Andrew Gazelka (andrewgazelka) changed the base branch from vfs-writable-overlay to ix-patched August 1, 2026 02:37
@andrewgazelka
Andrew Gazelka (andrewgazelka) marked this pull request as ready for review August 1, 2026 02:37
READDIRPLUS returned no attributes for any entry, so a client spent one
LOOKUP per entry it cared about. On a cold walk of the ix tree that was
22,477 LOOKUPs out of 31,083 RPCs, 72% of the whole walk.

The comment justifying it was correct when written and its premise has
since expired. Sizing a file used to mean reading it, so filling these
in would have turned listing a directory into reading every file in it.
`Backend::file_size` now answers from the git object header without
inflating the object, and `getattr` already takes that path.

Inline attributes for directories and files, which are cheap to size.
Keep them out for conflicted paths, whose size still needs the sides
materialized, and for symlinks, whose length still needs the target.
NFSv3 makes the field optional per entry, so those two cost a LOOKUP
each and nothing else.

Split the fattr3 constants into FattrContext so the iterator can size
entries without borrowing the tree, and give DirIterator a next_entry
that skips the attribute work, so plain READDIR pays nothing for a
field it would discard.

Measured against the same tree and revision, cold `find -type f`:
31,083 RPCs to 9,931, a 3.13x cut, with LOOKUPs down 17x from 22,477
to 1,320. A listing-only walk that never stats goes 29,873 to 8,683,
because without inline attributes the client reports DT_UNKNOWN and a
portable walker has to probe every entry anyway.
@andrewgazelka
Andrew Gazelka (andrewgazelka) merged commit d06587c into ix-patched Aug 1, 2026
34 checks passed
@andrewgazelka
Andrew Gazelka (andrewgazelka) deleted the vfs-readdirplus-attributes branch August 1, 2026 07:09
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