ls and other tools run more statx syscalls then needed
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 24.1k
- Forks
- 2k
- Avg merge
- 1d 5h
- Merged PRs (30d)
- 365
Description
This counts for more places and for more tools. This is about a part of ls.
alt_access_indicator (src/uu/ls/src/display.rs:375) calls item.security_context(config) for every entry in long format, regardless of whether -Z was given — it needs the length to decide between ., + and a space. That routes into get_security_context (src/uu/ls/src/ls.rs:1610), which begins with:
if must_dereference && let Err(err) = get_metadata_with_deref_opt(path, must_dereference) {
That is a fresh dereferencing stat, separate from the one PathData already cached, taken purely to detect a dangling symlink so the exit code can be set to 1. It fires per entry whenever must_dereference is true, and it fires whether or not SELinux is present or -Z was asked for.
Evidence
500 symlinks in one directory, counting statx and newfstatat:
ls -l |
ls -lL |
|
|---|---|---|
| uutils | 511 | 1011 |
| GNU | 504 | 504 |
-L doubles uutils' stat count — exactly one extra per entry — while GNU pays nothing for it. Without -L, must_dereference is false for readdir entries and the extra call does not happen, which is why plain ls -l sits at GNU + 7.
The security_context field is a OnceCell, so the several other call sites in display.rs (376, 420, 901, 1005, 1152, 1352, 1424) each hit the cache. It is one extra stat per entry, not several.
Fix shape
The dangling-link detection needs an answer PathData already has. is_dangling_link() (ls.rs:955) is exactly this question and is already computed from the cached metadata. Use that instead of a fresh get_metadata_with_deref_opt call.
The trap: the existing call reports the error through show!(LsError::IOErrorContext(..)) and sets exit code 1, and it does so with the specific io::Error from that stat. Reusing the cached result means the error has to be preserved when the cached stat fails, rather than re-issued. PathData::metadata() currently discards the error after reporting it.
Second, smaller point in the same function's neighbourhood: PathData::new (ls.rs:869) performs an eager, uncached, discarded p_buf.metadata() inside the Dereference::DirArgs arm just to decide must_dereference. That is per command-line argument rather than per entry, so it is a much smaller cost, but it is the same shape and could be folded into the same fix.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with alt_access_indicator in src/uu/ls/src/display.rs and get_security_context, is_dangling_link, PathData::metadata, and PathData::new in src/uu/ls/src/ls.rs. Trace the cached metadata and error handling before running the existing ls tests and the reported symlink syscall comparison. Done means avoiding the redundant per-entry stat while preserving dangling-link detection, reported errors, and exit status.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- cli, performance
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 72/100