Optimize path components iteration on platforms that don't have prefixes by the8472 · Pull Request #148084 · rust-lang/rust (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation8 Commits3 Checks11 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
OLD
path::bench_path_components_iter 140.20ns/iter +/- 6.08
path::bench_path_file_name 57.62ns/iter +/- 1.62
NEW
path::bench_path_components_iter 126.74ns/iter +/- 1.46
path::bench_path_file_name 47.48ns/iter +/- 2.49
rustbot added S-waiting-on-review
Status: Awaiting review from the assignee but also interested parties.
Relevant to the library team, which will review and decide on the PR/issue.
labels
rustbot has assigned @Mark-Simulacrum.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.
Use r? to explicitly pick a reviewer
Nice! I was thinking about this when profiling rust-lang/glob#181, because it was clear that getting just the filename is unnecessary slow just becase .components() has to be called.
This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.
Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.
it was clear that getting just the filename is unnecessary slow just becase .components() has to be called.
added a benchmark for getting filenames
📌 Commit d263ab3 has been approved by Mark-Simulacrum
It is now in the queue for this repository.
bors added S-waiting-on-bors
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
and removed S-waiting-on-review
Status: Awaiting review from the assignee but also interested parties.
labels
bors added a commit that referenced this pull request
Rollup of 22 pull requests
Successful merges:
- #128666 (Add
overflow_checksintrinsic) - #146305 (Add correct suggestion for multi-references for self type in method)
- #147179 ([DebugInfo] Fix container types failing to find template args)
- #147743 (Show packed field alignment in mir_transform_unaligned_packed_ref)
- #148079 (Rename
downcast_[ref|mut]_unchecked->downcast_unchecked_[ref|mut]) - #148084 (Optimize path components iteration on platforms that don't have prefixes)
- #148126 (Fix rust stdlib build failing for VxWorks)
- #148204 (Modify contributor email entries in .mailmap)
- #148279 (rustc_builtin_macros: rename bench parameter to avoid collisions with user-defined function names)
- #148333 (constify result unwrap unchecked)
- #148539 (Add Allocator proxy impls for Box, Rc, and Arc)
- #148601 (
invalid_atomic_ordering: also lintupdate&try_update) - #148612 (Add note for identifier with attempted hygiene violation)
- #148613 (Switch hexagon targets to rust-lld)
- #148619 (Enable std locking functions on AIX)
- #148644 ([bootstrap] Make
--openoption work withdoc src/tools/error_index_generator) - #148649 (don't completely reset
HeadUsages) - #148673 (Remove a remnant of
dyn*from the parser) - #148675 (Remove eslint-js from npm dependencies)
- #148680 (Recover
[T: N]as[T; N]) - #148688 (Remove unused argument
featuresfromeval_config_entry) - #148711 (Use the current lint note id when parsing
cfg!())
r? @ghost
@rustbot modify labels: rollup
rust-timer added a commit that referenced this pull request
Rollup merge of #148084 - the8472:prefixless-path-components-iter, r=Mark-Simulacrum
Optimize path components iteration on platforms that don't have prefixes
OLD
path::bench_path_components_iter 140.20ns/iter +/- 6.08
path::bench_path_file_name 57.62ns/iter +/- 1.62
NEW
path::bench_path_components_iter 126.74ns/iter +/- 1.46
path::bench_path_file_name 47.48ns/iter +/- 2.49github-actions bot pushed a commit to rust-lang/stdarch that referenced this pull request
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request
makai410 pushed a commit to makai410/rustc_public that referenced this pull request
makai410 pushed a commit to makai410/rust that referenced this pull request
…-iter, r=Mark-Simulacrum
Optimize path components iteration on platforms that don't have prefixes
OLD
path::bench_path_components_iter 140.20ns/iter +/- 6.08
path::bench_path_file_name 57.62ns/iter +/- 1.62
NEW
path::bench_path_components_iter 126.74ns/iter +/- 1.46
path::bench_path_file_name 47.48ns/iter +/- 2.49makai410 pushed a commit to makai410/rust that referenced this pull request
This PR appears to have triggered the perf regressions observed in rollup at #148721 (comment).
rust-lang/glob#181 (comment) shows some improvements at runtime.
Binary sizes are down.
One of the compile-time regressions is in cargo which works a lot with paths, so that's not very surprising.
Linker wall-time for cargo-opt-full more than doubling is surprising to me though.
makai410 pushed a commit to makai410/rustc_public that referenced this pull request
github-actions bot pushed a commit to model-checking/verify-rust-std that referenced this pull request
…-iter, r=Mark-Simulacrum
Optimize path components iteration on platforms that don't have prefixes
OLD
path::bench_path_components_iter 140.20ns/iter +/- 6.08
path::bench_path_file_name 57.62ns/iter +/- 1.62
NEW
path::bench_path_components_iter 126.74ns/iter +/- 1.46
path::bench_path_file_name 47.48ns/iter +/- 2.49github-actions bot pushed a commit to model-checking/verify-rust-std that referenced this pull request
Labels
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Relevant to the library team, which will review and decide on the PR/issue.