Flatten all -L search paths into a single list of files#158823
Draft
Kobzol wants to merge 1 commit into
Draft
Conversation
Collaborator
|
r? @mati865 rustbot has assigned @mati865. Use Why was this reviewer chosen?The reviewer was selected based on:
|
Member
Author
|
Sorry, meant to open this as a draft, but missclicked. r? @ghost |
Collaborator
|
The job Click to see the possible cause of the failure (guessed by this bot) |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a pre-emptive optimization for the new Cargo build dir layout.
Before, Cargo usually used to pass rustc a single
-Lpath to a directory containing potentially a large number of.rlibor other dependency paths. Rustc is optimized for this, and does a preprocessing step, where it preloads all files from this directory into a sorted vector, in which it then performs binary search to look for files having a given prefix and postfix.With the new build dir layout, this changes, and Cargo will pass potentially many (even hundreds, if your crate has many dependencies)
-Ldirectories, where each directory will usually have only 1-2 files. This is not ideal for the current rustc search implementation, because the old preprocessing isn't really worth it when the-Ldirectory typically only has a single file, and because rustc will do a lot of work per each-Ldirectory in the lookup phase, including some quadratic (O(n^2)) work.With the
large-workspacestress test from rustc-perf, where the final binary has ~1000 total and ~500 direct dependencies:FilesIndex::query1938314calls toFilesIndex::query. Not ideal :)This PR modifies the logic so that instead of going through all
-Ldirectories one by one when we are looking for a specific dependency, we pre-gather all files from all passed-Ldirectories at the start, and sort them all together. Then, in the lookup phase, instead of performing <number of -L dirs> * 4 (.rmeta, .rlib, etc.) searched, we only perform the 4 searches across all files. This gets rid of the O(n^2) behavior.I thought about pre-filtering the files further, e.g. based on their
kind, but that is not worth it in usual situations, where ~all of the deps will be.rlibs or.rmetas anyway. What might work is pre-filtering based on their actual names (we strip known prefixes from them and then search based on the names alone, in a hashmap or something?), but that can be a follow-up.Below are perf. runs with rustc/cargo using the new build dir layout:
Note that this is not intended to be merged as-is, it is not cleaned up and it is not very relevant until Cargo switches to the new build dir layout anyway. But I wanted to get a vibe-check if you think that something like this might make sense, e.g. from @petrochenkov. The main change is in
locator.rs, along with thesearch_paths2function.