uutils / uutils/coreutils

ls and other tools run more statx syscalls then needed

Open
#14,343 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

U - ls
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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.