Skip to content

Commit

Permalink
Log fork timings (#4618)
Browse files Browse the repository at this point in the history
This includes a functional change, we now skip the forked state pop/push
if we didn't fork.

From transformers:

```
DEBUG Pre-fork split universal took 0.036s
DEBUG Split python_version >= '3.10' and python_version >= '3.10' and platform_system == 'Darwin' and python_version >= '3.11' and python_version >= '3.12' and python_version >= '3.6' and platform_system == 'Linux' and platform_machine == 'aarch64' took 0.048s
DEBUG Split python_version <= '3.9' and platform_system == 'Darwin' and platform_machine == 'arm64' and python_version >= '3.7' and python_version >= '3.8' and python_version >= '3.9' took 0.038s
```

The messages could use simplification from
#4536

We can consider nested spans in the future but this works nicely for
now.
  • Loading branch information
konstin committed Jun 28, 2024
1 parent 363f3f7 commit 2b63dfd
Showing 1 changed file with 23 additions and 3 deletions.
26 changes: 23 additions & 3 deletions crates/uv-resolver/src/resolver/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use std::collections::{BTreeMap, VecDeque};
use std::fmt::{Display, Formatter};
use std::ops::Bound;
use std::sync::Arc;
use std::time::Instant;
use std::{iter, thread};

use dashmap::DashMap;
Expand Down Expand Up @@ -336,6 +337,7 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
}

'FORK: while let Some(mut state) = forked_states.pop() {
let start = Instant::now();
loop {
// Run unit propagation.
state
Expand Down Expand Up @@ -364,6 +366,11 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
if enabled!(Level::DEBUG) {
prefetcher.log_tried_versions();
}
debug!(
"Split {} took {:.3}s",
state.markers,
start.elapsed().as_secs_f32()
);
resolutions.push(state.into_resolution());
continue 'FORK;
};
Expand Down Expand Up @@ -539,7 +546,7 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
version.clone(),
UnavailableReason::Version(reason),
));
forked_states.push(state);
continue;
}
ForkedDependencies::Unforked(dependencies) => {
state.add_package_version_dependencies(
Expand All @@ -560,7 +567,7 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
let url = package.name().and_then(|name| state.fork_urls.get(name));
self.visit_package(package, url, &request_sink)?;
}
forked_states.push(state);
continue;
}
ForkedDependencies::Forked {
forks,
Expand All @@ -580,6 +587,7 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
// as it needs to be. We basically move the state
// into `forked_states`, and then only clone it if
// there is at least one more fork to visit.
let markers = state.markers.clone();
let mut cur_state = Some(state);
let forks_len = forks.len();
for (i, fork) in forks.into_iter().enumerate() {
Expand Down Expand Up @@ -612,9 +620,21 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
}
forked_states.push(forked_state);
}
if markers.is_universal() {
debug!(
"Pre-fork split universal took {:.3}s",
start.elapsed().as_secs_f32()
);
} else {
debug!(
"Pre-fork split {} took {:.3}s",
markers,
start.elapsed().as_secs_f32()
);
}
continue 'FORK;
}
}
continue 'FORK;
}
// `dep_incompats` are already in `incompatibilities` so we know there are not satisfied
// terms and can add the decision directly.
Expand Down

0 comments on commit 2b63dfd

Please sign in to comment.