Skip to content

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

Merged
sylvestre merged 1 commit intouutils:mainfrom
jesseschalken:reuse-metadata-instead-of-path-is-dir
Dec 2, 2024
Merged

du: Reuse existing metadata instead of calling path.is_dir() again#6840
sylvestre merged 1 commit intouutils:mainfrom
jesseschalken:reuse-metadata-instead-of-path-is-dir

Conversation

@jesseschalken
Copy link
Contributor

@jesseschalken jesseschalken commented Nov 3, 2024 •
edited
Loading

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

@jesseschalken jesseschalken changed the title Reuse existing metadata instead of calling path.is_dir() again du: Reuse existing metadata instead of calling path.is_dir() again Nov 3, 2024
@jesseschalken jesseschalken marked this pull request as ready for review November 3, 2024 14:21
@sylvestre
Copy link
Contributor

sylvestre commented Nov 3, 2024

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
Copy link
Contributor

sylvestre commented Nov 4, 2024

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

@sylvestre sylvestre mentioned this pull request Nov 6, 2024
Open
@sylvestre sylvestre force-pushed the reuse-metadata-instead-of-path-is-dir branch from 9ec85b5 to 99a5f2c Compare November 11, 2024 11:48
@github-actions
Copy link

github-actions bot commented Nov 11, 2024

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
Copy link
Contributor

sylvestre commented Nov 14, 2024

ping ?

@sylvestre sylvestre force-pushed the reuse-metadata-instead-of-path-is-dir branch from 99a5f2c to 8a69922 Compare November 28, 2024 16:50
@sylvestre
Copy link
Contributor

sylvestre commented Nov 28, 2024

any update? thanks

@sylvestre sylvestre force-pushed the reuse-metadata-instead-of-path-is-dir branch from 8a69922 to 6487347 Compare December 2, 2024 09:01
@github-actions
Copy link

github-actions bot commented Dec 2, 2024

GNU testsuite comparison:

Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)

@sylvestre
Copy link
Contributor

sylvestre commented Dec 2, 2024

i will document it at some point

@sylvestre sylvestre merged commit 763306c into uutils:main Dec 2, 2024
@moonfruit moonfruit mentioned this pull request Jan 20, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jesseschalken @sylvestre