Skip to content

Commit

Permalink
fix: correctly limits the concurrency, by limiting the tokio::spawn
Browse files Browse the repository at this point in the history
…'s in all cases (#215)

In the first implmentation the concurrency could potentially be
unbounded. Because, when resolving sdists a new `DependencyProvider` was
created. This would imply a new semaphore, and because the building is
recursive this could still lead to too many tasks.
  • Loading branch information
tdejager committed Feb 12, 2024
1 parent e657886 commit 9375605
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 39 deletions.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -26,26 +26,23 @@ use std::{
any::Any, borrow::Borrow, cmp::Ordering, collections::HashMap, rc::Rc, str::FromStr, sync::Arc,
};
use thiserror::Error;
use tokio::sync::Semaphore;
use url::Url;

/// This is a [`DependencyProvider`] for PyPI packages
pub(crate) struct PypiDependencyProvider {
pub pool: Rc<Pool<PypiVersionSet, PypiPackageName>>,
pub cached_artifacts: FrozenMap<SolvableId, Vec<Arc<ArtifactInfo>>>,
pub name_to_url: FrozenMap<NormalizedPackageName, String>,
package_db: Arc<PackageDb>,
wheel_builder: Arc<WheelBuilder>,
markers: Arc<MarkerEnvironment>,
compatible_tags: Option<Arc<WheelTags>>,

pub cached_artifacts: FrozenMap<SolvableId, Vec<Arc<ArtifactInfo>>>,

favored_packages: HashMap<NormalizedPackageName, PinnedPackage>,
locked_packages: HashMap<NormalizedPackageName, PinnedPackage>,
pub name_to_url: FrozenMap<NormalizedPackageName, String>,

options: ResolveOptions,
should_cancel_with_value: Mutex<Option<MetadataError>>,
concurrent_tasks: Arc<Semaphore>,
}

impl PypiDependencyProvider {
Expand Down Expand Up @@ -86,7 +83,6 @@ impl PypiDependencyProvider {
name_to_url,
options,
should_cancel_with_value: Default::default(),
concurrent_tasks: Arc::new(Semaphore::new(30)),
})
}

Expand Down Expand Up @@ -220,7 +216,8 @@ impl PypiDependencyProvider {
/// Acquires a lease to be able to spawn a task
/// this is used to limit the amount of concurrent tasks
async fn aquire_lease_to_run(&self) -> tokio::sync::OwnedSemaphorePermit {
self.concurrent_tasks
self.options
.max_concurrent_tasks
.clone()
.acquire_owned()
.await
Expand Down
30 changes: 29 additions & 1 deletion crates/rattler_installs_packages/src/resolve/solve_options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
use crate::python_env::PythonLocation;
use pep508_rs::{Requirement, VersionOrUrl};
use std::str::FromStr;
use std::sync::Arc;
use tokio::sync::Semaphore;

use crate::types::PackageName;

Expand Down Expand Up @@ -188,7 +190,7 @@ pub enum OnWheelBuildFailure {
/// Additional options that may influence the solver. In general passing [`Default::default`] to
/// the [`super::resolve`] function should provide sane defaults, however if you want to fine tune the
/// resolver you can do so via this struct.
#[derive(Default, Clone)]
#[derive(Clone)]
pub struct ResolveOptions {
/// Defines how to handle sdists during resolution. By default sdists will be treated the same
/// as wheels.
Expand All @@ -208,4 +210,30 @@ pub struct ResolveOptions {
/// Defines whether pre-releases are allowed to be selected during resolution. By default
/// pre-releases are not allowed (only if there are no other versions available for a given dependency).
pub pre_release_resolution: PreReleaseResolution,

/// Limits the amount of concurrent tasks when resolving.
pub max_concurrent_tasks: Arc<Semaphore>,
}

impl ResolveOptions {
/// Create a new instance of `ResolveOptions` with the given `max_concurrent_tasks`.
pub fn with_max_concurrent_tasks(max_concurrent_tasks: usize) -> Self {
Self {
max_concurrent_tasks: Arc::new(Semaphore::new(max_concurrent_tasks)),
..Default::default()
}
}
}

impl Default for ResolveOptions {
fn default() -> Self {
Self {
sdist_resolution: SDistResolution::default(),
python_location: PythonLocation::default(),
clean_env: false,
on_wheel_build_failure: OnWheelBuildFailure::default(),
pre_release_resolution: PreReleaseResolution::default(),
max_concurrent_tasks: Arc::new(Semaphore::new(30)),
}
}
}
1 change: 1 addition & 0 deletions crates/rip_bin/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,7 @@ async fn actual_main() -> miette::Result<()> {
clean_env: args.clean_env,
on_wheel_build_failure,
pre_release_resolution,
..Default::default()
};

// Solve the environment
Expand Down

0 comments on commit 9375605

Please sign in to comment.