Auto merge of #10830 - arlosi:parallel_yank, r=Eh2406

Make `is_yanked` return `Poll<>` The `is_yanked` check performed by `cargo install` and `cargo package` was running sequentially (calling `block_until_ready` after every check). This change makes `is_yanked` return `Poll<>` and runs the check in parallel, which gives better performance for `cargo install --locked` and `cargo package` when using a sparse registry. fixes #10821 r? `@ehuss`
rust-lang · Jul 7, 2022 · bc28260 · bc28260
2 parents 9c4bae6 + 72ed97b
commit bc28260
Show file tree

Hide file tree

Showing 9 changed files with 83 additions and 39 deletions.
diff --git a/src/cargo/core/source/mod.rs b/src/cargo/core/source/mod.rs
@@ -103,7 +103,7 @@ pub trait Source {
 
     /// Query if a package is yanked. Only registry sources can mark packages
     /// as yanked. This ignores the yanked whitelist.
-    fn is_yanked(&mut self, _pkg: PackageId) -> CargoResult<bool>;
+    fn is_yanked(&mut self, _pkg: PackageId) -> Poll<CargoResult<bool>>;
 
     /// Block until all outstanding Poll::Pending requests are `Poll::Ready`.
     ///
@@ -190,7 +190,7 @@ impl<'a, T: Source + ?Sized + 'a> Source for Box<T> {
         (**self).add_to_yanked_whitelist(pkgs);
     }
 
-    fn is_yanked(&mut self, pkg: PackageId) -> CargoResult<bool> {
+    fn is_yanked(&mut self, pkg: PackageId) -> Poll<CargoResult<bool>> {
         (**self).is_yanked(pkg)
     }
 
@@ -260,7 +260,7 @@ impl<'a, T: Source + ?Sized + 'a> Source for &'a mut T {
         (**self).add_to_yanked_whitelist(pkgs);
     }
 
-    fn is_yanked(&mut self, pkg: PackageId) -> CargoResult<bool> {
+    fn is_yanked(&mut self, pkg: PackageId) -> Poll<CargoResult<bool>> {
         (**self).is_yanked(pkg)
     }
 

diff --git a/src/cargo/ops/cargo_install.rs b/src/cargo/ops/cargo_install.rs
@@ -1,6 +1,7 @@
 use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet};
 use std::path::{Path, PathBuf};
 use std::sync::Arc;
+use std::task::Poll;
 use std::{env, fs};
 
 use crate::core::compiler::{CompileKind, DefaultExecutor, Executor, Freshness, UnitOutput};
@@ -530,22 +531,40 @@ impl<'cfg, 'a> InstallablePackage<'cfg, 'a> {
         // duplicate "Updating", but since `source` is taken by value, then it
         // wouldn't be available for `compile_ws`.
         let (pkg_set, resolve) = ops::resolve_ws(&self.ws)?;
-        let mut sources = pkg_set.sources_mut();
 
         // Checking the yanked status involves taking a look at the registry and
         // maybe updating files, so be sure to lock it here.
         let _lock = self.ws.config().acquire_package_cache_lock()?;
 
-        for pkg_id in resolve.iter() {
-            if let Some(source) = sources.get_mut(pkg_id.source_id()) {
-                if source.is_yanked(pkg_id)? {
-                    self.ws.config().shell().warn(format!(
-                        "package `{}` in Cargo.lock is yanked in registry `{}`, \
-                         consider running without --locked",
-                        pkg_id,
-                        pkg_id.source_id().display_registry_name()
-                    ))?;
+        let mut sources = pkg_set.sources_mut();
+        let mut pending: Vec<PackageId> = resolve.iter().collect();
+        let mut results = Vec::new();
+        for (_id, source) in sources.sources_mut() {
+            source.invalidate_cache();
+        }
+        while !pending.is_empty() {
+            pending.retain(|pkg_id| {
+                if let Some(source) = sources.get_mut(pkg_id.source_id()) {
+                    match source.is_yanked(*pkg_id) {
+                        Poll::Ready(result) => results.push((*pkg_id, result)),
+                        Poll::Pending => return true,
+                    }
                 }
+                false
+            });
+            for (_id, source) in sources.sources_mut() {
+                source.block_until_ready()?;
+            }
+        }
+
+        for (pkg_id, is_yanked) in results {
+            if is_yanked? {
+                self.ws.config().shell().warn(format!(
+                    "package `{}` in Cargo.lock is yanked in registry `{}`, \
+                     consider running without --locked",
+                    pkg_id,
+                    pkg_id.source_id().display_registry_name()
+                ))?;
             }
         }
 

diff --git a/src/cargo/ops/cargo_package.rs b/src/cargo/ops/cargo_package.rs
@@ -5,6 +5,7 @@ use std::io::SeekFrom;
 use std::path::{Path, PathBuf};
 use std::rc::Rc;
 use std::sync::Arc;
+use std::task::Poll;
 
 use crate::core::compiler::{BuildConfig, CompileMode, DefaultExecutor, Executor};
 use crate::core::resolver::CliFeatures;
@@ -722,16 +723,34 @@ fn check_yanked(config: &Config, pkg_set: &PackageSet<'_>, resolve: &Resolve) ->
     let _lock = config.acquire_package_cache_lock()?;
 
     let mut sources = pkg_set.sources_mut();
-    for pkg_id in resolve.iter() {
-        if let Some(source) = sources.get_mut(pkg_id.source_id()) {
-            if source.is_yanked(pkg_id)? {
-                config.shell().warn(format!(
-                    "package `{}` in Cargo.lock is yanked in registry `{}`, \
-                     consider updating to a version that is not yanked",
-                    pkg_id,
-                    pkg_id.source_id().display_registry_name()
-                ))?;
+    let mut pending: Vec<PackageId> = resolve.iter().collect();
+    let mut results = Vec::new();
+    for (_id, source) in sources.sources_mut() {
+        source.invalidate_cache();
+    }
+    while !pending.is_empty() {
+        pending.retain(|pkg_id| {
+            if let Some(source) = sources.get_mut(pkg_id.source_id()) {
+                match source.is_yanked(*pkg_id) {
+                    Poll::Ready(result) => results.push((*pkg_id, result)),
+                    Poll::Pending => return true,
+                }
             }
+            false
+        });
+        for (_id, source) in sources.sources_mut() {
+            source.block_until_ready()?;
+        }
+    }
+
+    for (pkg_id, is_yanked) in results {
+        if is_yanked? {
+            config.shell().warn(format!(
+                "package `{}` in Cargo.lock is yanked in registry `{}`, \
+                 consider updating to a version that is not yanked",
+                pkg_id,
+                pkg_id.source_id().display_registry_name()
+            ))?;
         }
     }
     Ok(())

diff --git a/src/cargo/ops/common_for_install_and_uninstall.rs b/src/cargo/ops/common_for_install_and_uninstall.rs
@@ -553,8 +553,20 @@ where
         None => {
             let is_yanked: bool = if dep.version_req().is_exact() {
                 let version: String = dep.version_req().to_string();
-                PackageId::new(dep.package_name(), &version[1..], source.source_id())
-                    .map_or(false, |pkg_id| source.is_yanked(pkg_id).unwrap_or(false))
+                if let Ok(pkg_id) =
+                    PackageId::new(dep.package_name(), &version[1..], source.source_id())
+                {
+                    source.invalidate_cache();
+                    loop {
+                        match source.is_yanked(pkg_id) {
+                            Poll::Ready(Ok(is_yanked)) => break is_yanked,
+                            Poll::Ready(Err(_)) => break false,
+                            Poll::Pending => source.block_until_ready()?,
+                        }
+                    }
+                } else {
+                    false
+                }
             } else {
                 false
             };

diff --git a/src/cargo/sources/directory.rs b/src/cargo/sources/directory.rs
@@ -219,8 +219,8 @@ impl<'cfg> Source for DirectorySource<'cfg> {
 
     fn add_to_yanked_whitelist(&mut self, _pkgs: &[PackageId]) {}
 
-    fn is_yanked(&mut self, _pkg: PackageId) -> CargoResult<bool> {
-        Ok(false)
+    fn is_yanked(&mut self, _pkg: PackageId) -> Poll<CargoResult<bool>> {
+        Poll::Ready(Ok(false))
     }
 
     fn invalidate_cache(&mut self) {

diff --git a/src/cargo/sources/git/source.rs b/src/cargo/sources/git/source.rs
@@ -234,8 +234,8 @@ impl<'cfg> Source for GitSource<'cfg> {
 
     fn add_to_yanked_whitelist(&mut self, _pkgs: &[PackageId]) {}
 
-    fn is_yanked(&mut self, _pkg: PackageId) -> CargoResult<bool> {
-        Ok(false)
+    fn is_yanked(&mut self, _pkg: PackageId) -> Poll<CargoResult<bool>> {
+        Poll::Ready(Ok(false))
     }
 
     fn invalidate_cache(&mut self) {}

diff --git a/src/cargo/sources/path.rs b/src/cargo/sources/path.rs
@@ -562,8 +562,8 @@ impl<'cfg> Source for PathSource<'cfg> {
 
     fn add_to_yanked_whitelist(&mut self, _pkgs: &[PackageId]) {}
 
-    fn is_yanked(&mut self, _pkg: PackageId) -> CargoResult<bool> {
-        Ok(false)
+    fn is_yanked(&mut self, _pkg: PackageId) -> Poll<CargoResult<bool>> {
+        Poll::Ready(Ok(false))
     }
 
     fn block_until_ready(&mut self) -> CargoResult<()> {

diff --git a/src/cargo/sources/registry/mod.rs b/src/cargo/sources/registry/mod.rs
@@ -801,14 +801,8 @@ impl<'cfg> Source for RegistrySource<'cfg> {
         self.yanked_whitelist.extend(pkgs);
     }
 
-    fn is_yanked(&mut self, pkg: PackageId) -> CargoResult<bool> {
-        self.invalidate_cache();
-        loop {
-            match self.index.is_yanked(pkg, &mut *self.ops)? {
-                Poll::Ready(yanked) => return Ok(yanked),
-                Poll::Pending => self.block_until_ready()?,
-            }
-        }
+    fn is_yanked(&mut self, pkg: PackageId) -> Poll<CargoResult<bool>> {
+        self.index.is_yanked(pkg, &mut *self.ops)
     }
 
     fn block_until_ready(&mut self) -> CargoResult<()> {

diff --git a/src/cargo/sources/replaced.rs b/src/cargo/sources/replaced.rs
@@ -134,7 +134,7 @@ impl<'cfg> Source for ReplacedSource<'cfg> {
         self.inner.add_to_yanked_whitelist(&pkgs);
     }
 
-    fn is_yanked(&mut self, pkg: PackageId) -> CargoResult<bool> {
+    fn is_yanked(&mut self, pkg: PackageId) -> Poll<CargoResult<bool>> {
         self.inner.is_yanked(pkg)
     }