-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
ls: Fix excessive ls -al syscalls for ACL metadata #8660
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…s/coreutils into reduce_syscalls_acls
GNU testsuite comparison:
|
GNU testsuite comparison:
|
GNU testsuite comparison:
|
GNU testsuite comparison:
|
GNU testsuite comparison:
|
excellent, happy to merge this when it is ready :) |
CodSpeed Performance ReportMerging #8660 will degrade performances by 2.18%Comparing 🎉 Hooray!
|
Benchmark | BASE |
HEAD |
Change | |
---|---|---|---|---|
❌ | ls_recursive_long_all_deep_tree[(100, 4)] |
3.1 ms | 3.2 ms | -2.18% |
Footnotes
-
73 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
GNU testsuite comparison:
|
This comment was marked as outdated.
This comment was marked as outdated.
I guess it has a positive impact on performance, did you run benchmarking (hyperfine) on this ? thanks |
Right now any performance differences are modest (worth 1 or 2 ms in hyperfine?). I have a feeling this will have a big impact when I/others merge a few more PRs. After spending some time with it, my guess is that, oddly, any performance difference is not down to syscalls. These seem to happen pretty fast. Much more likely it is silly allocation behavior. Like create a bunch of small Strings instead of creating one String, etc. I have another PR where we were previously doing an insert and a remove on a HashMap for each directory we entered, needlessly. Stuff like that. |
GNU testsuite comparison:
|
GNU testsuite comparison:
|
This reverts commit 8adf358.
GNU testsuite comparison:
|
…s/coreutils into reduce_syscalls_acls
GNU testsuite comparison:
|
GNU testsuite comparison:
|
GNU testsuite comparison:
|
This reverts commit 37b6b96.
GNU testsuite comparison:
|
GNU testsuite comparison:
|
@sylvestre these failing checks seem to have nothing to do with this PR. Should I close and resubmit? |
Reduces syscalls by searching for the specific ACL xattr key instead of requesting a list of all keys and values.
Fixes: #8350