Clean up find_library_crate
#93608
Clean up find_library_crate
#93608
Conversation
This code and comment appear to be out of date. `CrateLocator::find_library_crate` is the only caller of this function and it handles rlib vs dylib overlap itself (see `CrateLocator::extract_lib`) after inspecting all the files present, so it doesn't need to see them in any particular order.
It's returned from `FileSearch::search` but it's only used to print some debug info.
It has only a single callsite, and having all the code in one place will make it possible to optimize the search.
Currently, it can be `None` if the conversion from `OsString` fails, in which case all searches will skip over the `SearchPathFile`. The commit changes things so that the `SearchPathFile` just doesn't get created in the first place. Same behaviour, but slightly simpler code.
rustbot
added
the
T-compiler
|
The impact of the performance differs in the following cases:
This is because the number of files present in the relevant directories differs in these three case. Let's see what happens for the CI configuration, which I think is a stage 2 compiler. @bors try @rust-timer queue |
|
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
rustbot
added
the
S-waiting-on-perf
|
⌛ Trying commit 1746e0e364d5b012fe05bec1b3fec1249bbe45f4 with merge 1a649e00cff62ad3f6fdfd5ebece9c1269e9c743... |
|
☀️ Try build successful - checks-actions |
|
Queued 1a649e00cff62ad3f6fdfd5ebece9c1269e9c743 with parent 1be5c8f, future comparison URL. |
|
Finished benchmarking commit (1a649e00cff62ad3f6fdfd5ebece9c1269e9c743): comparison url. Summary: This benchmark run did not return any relevant results. 11 results were found to be statistically significant but too small to be relevant. If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR led to changes in compiler perf. @bors rollup=never |
rustbot
added
S-waiting-on-review
|
The first commits are good cleanups, but I'm somewhat skeptical about the last commit - it makes things more complex, but there's not so much benefits? Or you were benchmarking webrender-wrench specifically? |
petrochenkov
added
S-waiting-on-author
|
@petrochenkov: as per my comment above, the benefit depends on exactly which compiler you're running and what else you've built. I did this because I wanted to understand why I sometimes got different results running the stage 1 compiler when I seemingly hadn't changed anything -- it's because if I've also built the stage 2 compiler there are more files in these directories. I agree that the CI perf results are underwhelming and not worth the added complication. I'll likely drop the final commit before requesting review. |
By introducing prefix and suffix variables for all file types, and renaming some variables.
nnethercote
force-pushed
the
speed-up-find_library_crate
branch
from
1746e0e
to
2826586
Compare
|
I have removed the binary search optimization, and added a couple more clean-up patches. @bors rollup=always |
petrochenkov
added
S-waiting-on-review
|
@bors r+ |
|
📌 Commit 2826586 has been approved by |
bors
added
S-waiting-on-bors
…askrgr Rollup of 7 pull requests Successful merges: - rust-lang#90132 (Stabilize `-Z instrument-coverage` as `-C instrument-coverage`) - rust-lang#91589 (impl `Arc::unwrap_or_clone`) - rust-lang#93495 (kmc-solid: Fix off-by-one error in `SystemTime::now`) - rust-lang#93576 (Emit more valid HTML from rustdoc) - rust-lang#93608 (Clean up `find_library_crate`) - rust-lang#93612 (doc: use U+2212 for minus sign in integer MIN/MAX text) - rust-lang#93615 (Fix `isize` optimization in `StableHasher` for big-endian architectures) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
Some clean-ups.
r? @petrochenkov