From 687239c7f05dc59cf1d4701c44f922dfafc57d4f Mon Sep 17 00:00:00 2001 From: konstin Date: Wed, 28 Feb 2024 11:21:58 +0100 Subject: [PATCH] Warn when trying many versions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In pathological cases, it's hard to follow why the resolution is slow. This is an attempt to give the user an indication what is slow (boto3) and how to fix this (add a manual boto3 constraint). This also helps to determine if this is a pathological case or a server that doesn't support range requests. The values i picked are somewhat arbitrary. ``` $ uv pip install bio_embeddings ⠸ pytest==8.0.2 warning: Downloading metadata for more than 50 different versions of boto3. Consider adding a stricter version constraint for this package to speed up resolution. warning: Downloading metadata for more than 50 different versions of botocore. Consider adding a stricter version constraint for this package to speed up resolution. ⠼ boto3==1.26.85 ``` `warn_user!` interacts badly with the `Reporter`, do we have a way to pause-and-restart the restart the reporter when showing our own messages, similar to tqdm or `IndicatifLayer`? --- Cargo.lock | 1 + crates/uv-distribution/Cargo.toml | 5 ++- .../src/distribution_database.rs | 42 +++++++++++++++++++ 3 files changed, 46 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 24ad8d511..c2fc7f27c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4420,6 +4420,7 @@ dependencies = [ "uv-git", "uv-normalize", "uv-traits", + "uv-warnings", "zip", ] diff --git a/crates/uv-distribution/Cargo.toml b/crates/uv-distribution/Cargo.toml index d81cac3dc..859f2379f 100644 --- a/crates/uv-distribution/Cargo.toml +++ b/crates/uv-distribution/Cargo.toml @@ -18,7 +18,7 @@ distribution-filename = { path = "../distribution-filename", features = ["serde" distribution-types = { path = "../distribution-types" } install-wheel-rs = { path = "../install-wheel-rs" } pep440_rs = { path = "../pep440-rs" } -pep508_rs = { path = "../pep508-rs" } +pep508_rs = { path = "../pep508-rs" } platform-tags = { path = "../platform-tags" } uv-cache = { path = "../uv-cache" } uv-client = { path = "../uv-client" } @@ -27,6 +27,7 @@ uv-fs = { path = "../uv-fs", features = ["tokio"] } uv-git = { path = "../uv-git", features = ["vendored-openssl"] } uv-normalize = { path = "../uv-normalize" } uv-traits = { path = "../uv-traits" } +uv-warnings = { path = "../uv-warnings" } pypi-types = { path = "../pypi-types" } anyhow = { workspace = true } @@ -36,7 +37,7 @@ nanoid = { workspace = true } reqwest = { workspace = true } rmp-serde = { workspace = true } rustc-hash = { workspace = true } -serde = { workspace = true , features = ["derive"] } +serde = { workspace = true, features = ["derive"] } tempfile = { workspace = true } thiserror = { workspace = true } tokio = { workspace = true } diff --git a/crates/uv-distribution/src/distribution_database.rs b/crates/uv-distribution/src/distribution_database.rs index b79bb3526..c9eadd718 100644 --- a/crates/uv-distribution/src/distribution_database.rs +++ b/crates/uv-distribution/src/distribution_database.rs @@ -4,6 +4,8 @@ use std::path::Path; use std::sync::Arc; use futures::{FutureExt, TryStreamExt}; +use rustc_hash::FxHashMap; +use tokio::sync::Mutex; use tokio_util::compat::FuturesAsyncReadCompatExt; use tracing::{info_span, instrument, Instrument}; use url::Url; @@ -17,13 +19,20 @@ use uv_cache::{Cache, CacheBucket, Timestamp, WheelCache}; use uv_client::{CacheControl, CachedClientError, Connectivity, RegistryClient}; use uv_fs::metadata_if_exists; use uv_git::GitSource; +use uv_normalize::PackageName; use uv_traits::{BuildContext, NoBinary, NoBuild}; +use uv_warnings::warn_user; use crate::download::{BuiltWheel, UnzippedWheel}; use crate::locks::Locks; use crate::reporter::Facade; use crate::{DiskWheel, Error, LocalWheel, Reporter, SourceDistCachedBuilder}; +/// Warn when more than this many versions were requested for a single package. +const METADATA_REQUEST_WARN_THRESHOLD: usize = 50; +/// Warn when more than this many source dist build were requested for a single package. +const BUILD_REQUEST_WARN_THRESHOLD: usize = 5; + /// A cached high-level interface to convert distributions (a requirement resolved to a location) /// to a wheel or wheel metadata. /// @@ -41,6 +50,8 @@ pub struct DistributionDatabase<'a, Context: BuildContext + Send + Sync> { reporter: Option>, locks: Arc, client: &'a RegistryClient, + metadata_request_counts: Mutex>, + build_request_counts: Mutex>, build_context: &'a Context, builder: SourceDistCachedBuilder<'a, Context>, } @@ -57,6 +68,8 @@ impl<'a, Context: BuildContext + Send + Sync> DistributionDatabase<'a, Context> reporter: None, locks: Arc::new(Locks::default()), client, + metadata_request_counts: Mutex::new(FxHashMap::default()), + build_request_counts: Mutex::new(FxHashMap::default()), build_context, builder: SourceDistCachedBuilder::new(build_context, client, tags), } @@ -358,6 +371,20 @@ impl<'a, Context: BuildContext + Send + Sync> DistributionDatabase<'a, Context> ) -> Result<(Metadata21, Option), Error> { match dist { Dist::Built(built_dist) => { + let count: usize = *self + .metadata_request_counts + .lock() + .await + .entry(built_dist.name().clone()) + .and_modify(|count| *count += 1) + .or_insert(1); + if count == METADATA_REQUEST_WARN_THRESHOLD { + warn_user!( + "Downloading metadata for more than {METADATA_REQUEST_WARN_THRESHOLD} different versions of {}. \ + Consider adding a stricter version constraint for this package to speed up resolution.", + built_dist.name() + ); + } Ok((self.client.wheel_metadata(built_dist).boxed().await?, None)) } Dist::Source(source_dist) => { @@ -371,6 +398,21 @@ impl<'a, Context: BuildContext + Send + Sync> DistributionDatabase<'a, Context> return Err(Error::NoBuild); } + let count: usize = *self + .build_request_counts + .lock() + .await + .entry(source_dist.name().clone()) + .and_modify(|count| *count += 1) + .or_insert(1); + if count == BUILD_REQUEST_WARN_THRESHOLD { + warn_user!( + "Building more than {METADATA_REQUEST_WARN_THRESHOLD} source distribution versions of {}. \ + Consider adding a stricter version constraint for this package to speed up resolution.", + source_dist.name() + ); + } + let lock = self.locks.acquire(dist).await; let _guard = lock.lock().await;