-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
cargo fetch can't be parallelized on nightly anymore #6930
Comments
This is sort of intentional in the sense that previously Cargo attempted to do fine-grained locking but it became both a performance issue and a correctness burden on Cargo for a use case that very very rarely came up in practice. I believe #6880 is indeed the cause for this though. |
CARGO_HOME is where Cargo stores its downloaded artefacts. See https://doc.rust-lang.org/cargo/reference/environment-variables.html: CARGO_HOME — Cargo maintains a local cache of the registry index and of git checkouts of crates. By default these are stored under $HOME/.cargo (%USERPROFILE%\.cargo on Windows), but this variable overrides the location of this directory. Once a crate is cached it is not removed by the clean command. For more details refer to the guide. We currently make it point to $(HOST_DIR)/share/cargo, but this has a number of drawbacks: (1) It is not shared between Buildroot builds. Each Buildroot build will re-download the crates index, and the crates themselves, unless of course the final vendored tarball is already there. (2) With BR2_PER_PACKAGE_DIRECTORIES=y, it is even worse: CARGO_HOME is not even shared between packages, as $(HOST_DIR)/share/cargo is per package. So each package in the build that needs vendoring of Cargo crates will download the crates index and the crates in its own CARGO_HOME location. To solve this, this commit moves CARGO_HOME into $(DL_DIR), so that it is shared between builds and packages. Even though not the best/most authoritative source, rust-lang/cargo#6930 indicates that there is a lock when accessing CARGO_HOME, because a user even complains that this lock has even become more coarse-grained than it used to be (which for us is fine, it just means that two Cargo fetch operations from two different packages will be serialized, not a big deal). Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com> Tested-by: Moritz Bitsch <moritz@h6t.eu> [yann.morin.1998@free.fr: rename directory: s/\.cargo/br-cargo-home/] Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
Commit 8450b76 (package/pkg-cargo: move CARGO_HOME into DL_DIR) allowed for a shared cargo cache of crates. Internally, cargo is supposed to lock themselves when accessing that cache, and that commit even had some research in that area, pointing at [0] for complaints about too-coarse the lock, so it was deemed safe to have a shared cargo home. However, in practice, the locking as implemented by cargo, fails to properly protect the concurrent accesses to the crates cache, with random failures that manifest themselves like so: Blocking waiting for file lock on package cache Blocking waiting for file lock on package cache Downloading crates ... error: failed to sync Caused by: failed to download packages Caused by: failed to download `autocfg v1.1.0` Caused by: unable to get packages from source Caused by: failed to unpack package `autocfg v1.1.0` Caused by: failed to unpack entry at `autocfg-1.1.0/src/tests.rs` Caused by: No such file or directory (os error 2) while canonicalizing [...] with the last few errors sometime being: Caused by: failed to parse manifest at `[...]/aho-corasick-0.7.18/Cargo.toml` Caused by: can't find library `aho_corasick`, rename file to `src/lib.rs` or specify lib.path So, as we do not systematically use our own cargo build (we can use a pre-built one with host-rust-bin), we can't patch cargo (even if we knew what to do!). Instead, we implement a lock ourselves, by wrapping the call to "cargo vendor" with a flock(1) on cargo home. Note: the download wrapper is already flock-ed, but it is a per-package lock, so it does not prevent different packages from being downloaded in parallel; if those packages need cargo vendoring, that will not be protected by the flock on the dl wrapper. So we really do need a flock on cargo home. [0] rust-lang/cargo#6930 Fixes: 8450b76 Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com> Cc: Moritz Bitsch <moritz@h6t.eu> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Considering the trade off involved here and the small scope of what was slowed down (while instead other cases were sped up), I'm not seeing us reverting to the old locking scheme. I'm also not seeing a viable alternative. With that in mind, I'm going to go ahead and close this. |
Problem
It used to be possible to run multiple
cargo fetch
in parallel inside the same cargo home, but it's not possible anymore on the latest nightly: the following fetch invocations just wait for the package cache lock until the other fetches finish.This is bad for Crater since it slows down fetching deps to 1/8.
Steps
bash repro.sh stable
for the correct behaviorbash repro.sh nightly
for the wrong behaviorPossible Solution(s)
After talking to @Eh2406 the issue seems to be caused by #6880.
cc @alexcrichton
The text was updated successfully, but these errors were encountered: