uutils / coreutils

Cross-platform Rust rewrite of the GNU coreutils
https://uutils.github.io/
MIT License
17.8k stars 1.28k forks source link

du: Reuse existing metadata instead of calling path.is_dir() again #6840

Open jesseschalken opened 2 weeks ago

jesseschalken commented 2 weeks ago

Path::is_dir does another call to stat but we can reuse the metadata we already have.

Also Path::is_dir traverses symbolic links which is only right if -L was passed.

~1.13x speedup on my test directory.

Before

> cargo build --release
> measure-command { .\target\release\coreutils.exe du --apparent-size "test folder" }
TotalSeconds      : 11.0101422

After

> cargo build --release
> measure-command { .\target\release\coreutils.exe du --apparent-size "test folder" }
TotalSeconds      : 9.7109226
sylvestre commented 2 weeks ago

nice could you please try to benchmark with hyperfine ? and document your work in src/uu/du/BENCHMARKING.md ?

see other commands for example

sylvestre commented 2 weeks ago

and i wonder if we could automate this check to make sure we don't regress :)

github-actions[bot] commented 1 week ago

GNU testsuite comparison:

Skipping an intermittent issue tests/rm/rm1 (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)
sylvestre commented 1 week ago

ping ?