-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Lazily download packages #2406
Merged
Merged
Lazily download packages #2406
Conversation
This file contains 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
This commit moves the SourceMap structure into the PackageSet structure, and simultaneously massively cuts down on the API surface area of PackageSet. It's intended that eventually a PackageSet will be a lazily loaded set of packages so we don't have to download everything all at once, and this is the commit in preparation of that.
Eventually we may not have the entire set of packages resident in memory, so refactor the implementation to validate the `links` attribute in a demand driven way rather than all at once up front.
Like with the previous refactor, remove the need to list all packages in a package set as we only need to lazily do this for the actual packages being compiled. Whenever a build script is requested to be executed is when we actually go and try to see if an override was in play.
Future calls to `get_package` may end up actually downloading a package, so we want to defer this as late as possible to ensure that we don't download anything.
If we download lazily, that's excessively wasteful!
This isn't used anywhere
Nothing currently implements the ability to more efficiently download a set of packages at any one point in time, and the download/get distinction isn't really used at all. We can always refactor later, but currently there's no benefit, nor can it really be seen what the possible benefit is, so let's just merge these two methods into one and have them operate on one id at a time.
This function will lazily download the package specified and fill a cell with that package. Currently this is always infallible because the `PackageSet` is initialized with all packages, but eventually this will not be true.
The propagate all the `try!`-s needed upwards
r? @brson |
r? @huonw (rust_highfive has picked a reviewer for you, use r? to override) |
Instead delay all downloads until just before they're needed. This should ensure that we don't download any more packages than we really need to.
alexcrichton
force-pushed
the
download-less
branch
from
February 26, 2016 19:25
b63312b
to
f994592
Compare
@bors r+ |
📌 Commit f994592 has been approved by |
bors
added a commit
that referenced
this pull request
Feb 29, 2016
Currently Cargo will download an entire resolution graph all at once when in fact most packages may not be relevant to a compilation. For example target-specific dependencies and dev-dependencies are unconditionally downloaded regardless of whether they're actually needed or not. This commit alters the internals of Cargo to avoid downloading everything immediately and just switches to lazily downloading packages. This involved adding a new `LazyCell` primitive (similar to the one in use on crates.io) and also propagates `CargoResult` in a few more locations. Overall this ended up being a pretty large refactoring so the commits are separated in bite-sized chunks as much as possible with the end goal being this PR itself. Closes #2394
☀️ Test successful - cargo-linux-32, cargo-linux-64, cargo-mac-32, cargo-mac-64, cargo-win-gnu-32, cargo-win-gnu-64, cargo-win-msvc-32, cargo-win-msvc-64 |
This was referenced Feb 29, 2016
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.
Currently Cargo will download an entire resolution graph all at once when in fact most packages may not be relevant to a compilation. For example target-specific dependencies and dev-dependencies are unconditionally downloaded regardless of whether they're actually needed or not.
This commit alters the internals of Cargo to avoid downloading everything immediately and just switches to lazily downloading packages. This involved adding a new
LazyCell
primitive (similar to the one in use on crates.io) and also propagatesCargoResult
in a few more locations.Overall this ended up being a pretty large refactoring so the commits are separated in bite-sized chunks as much as possible with the end goal being this PR itself.
Closes #2394