Skip to content

Commit 430fabe

Browse files
committed
Auto merge of #14692 - x-hgg-x:resolver-perf-5, r=Eh2406
fix(resolver): share conflict cache between activation retries ### What does this PR try to resolve? Found in Eh2406/pubgrub-crates-benchmark#6. Currently the resolve is done in a loop, restarting if there are pending dependencies which are not yet fetched, and recreating the conflict cache at each iteration. This means we do a lot of duplicate work, especially if the newly fetched dependencies has zero or few dependencies which doesn't conflict. This also means that the maximum depth of the dependency graph has a big impact on the total resolving time, since adding a dependency not yet seen means we will restart the resolve at the next iteration. Here is the output of the resolve for the master branch using `solana-core = "=1.2.18"` in `Cargo.toml`, printing the duration from the start after each iteration: ``` Updating crates.io index 103.142341ms [=====> ] 1 complete; 0 pending 118.486144ms [========> ] 2 complete; 0 pending 785.389055ms [===========> ] 62 complete; 0 pending 4.916033377s [==============> ] 277 complete; 0 pending 9.13101404s [=================> ] 439 complete; 0 pending 50.083251549s [====================> ] 520 complete; 0 pending 133.401303265s [=======================> ] 561 complete; 0 pending 214.120483345s [==========================> ] 565 complete; 0 pending 294.180677785s [=============================> ] 566 complete; 0 pending Locking 536 packages to latest compatible versions ``` To improve the situation, this PR moves the conflict cache outside the activation loop so it can be reused for all iterations. This is possible since when using an online registry, dependencies which are not yet fetched are not added to the candidate summary: https://github.com/rust-lang/cargo/blob/82c489f1c612096c2dffc48a4091d21af4b8e046/src/cargo/core/resolver/dep_cache.rs#L248-L266 On the other hand, if a dependency query returns `Poll::Ready`, then all compatible summaries are returned, so we cannot have a partial view where only some compatible summaries would be returned. This means we cannot add a conflict in the cache which would be incompatible with future fetches, and therefore the conflict cache can be shared. Here is the output of the resolve with this PR: ``` Updating crates.io index 98.239126ms [=====> ] 1 complete; 0 pending 127.528982ms [========> ] 2 complete; 0 pending 821.601257ms [===========> ] 62 complete; 0 pending 4.67917132s [==============> ] 277 complete; 0 pending 5.933230172s [=================> ] 431 complete; 0 pending 14.74321904s [====================> ] 508 complete; 0 pending 24.607428893s [=======================> ] 546 complete; 0 pending 24.700610469s [==========================> ] 550 complete; 0 pending 24.757651875s [=============================> ] 551 complete; 0 pending Locking 536 packages to latest compatible versions ``` Besides the huge performance savings between iterations, sharing the conflict cache has the side effect of eliminating dead paths earlier in the resolve after a restart. We can see that the duration of all iterations using this PR is less than the duration of the last iteration on master, and that it needs to resolve less dependencies overall.
2 parents 3c4c0a2 + 46ea11a commit 430fabe

File tree

1 file changed

+14
-7
lines changed

1 file changed

+14
-7
lines changed

src/cargo/core/resolver/mod.rs

+14-7
Original file line numberDiff line numberDiff line change
@@ -137,10 +137,20 @@ pub fn resolve(
137137
_ => None,
138138
};
139139
let mut registry = RegistryQueryer::new(registry, replacements, version_prefs);
140+
141+
// Global cache of the reasons for each time we backtrack.
142+
let mut past_conflicting_activations = conflict_cache::ConflictCache::new();
143+
140144
let resolver_ctx = loop {
141145
let resolver_ctx = ResolverContext::new();
142-
let resolver_ctx =
143-
activate_deps_loop(resolver_ctx, &mut registry, summaries, first_version, gctx)?;
146+
let resolver_ctx = activate_deps_loop(
147+
resolver_ctx,
148+
&mut registry,
149+
summaries,
150+
first_version,
151+
gctx,
152+
&mut past_conflicting_activations,
153+
)?;
144154
if registry.reset_pending() {
145155
break resolver_ctx;
146156
} else {
@@ -194,14 +204,11 @@ fn activate_deps_loop(
194204
summaries: &[(Summary, ResolveOpts)],
195205
first_version: Option<VersionOrdering>,
196206
gctx: Option<&GlobalContext>,
207+
past_conflicting_activations: &mut conflict_cache::ConflictCache,
197208
) -> CargoResult<ResolverContext> {
198209
let mut backtrack_stack = Vec::new();
199210
let mut remaining_deps = RemainingDeps::new();
200211

201-
// `past_conflicting_activations` is a cache of the reasons for each time we
202-
// backtrack.
203-
let mut past_conflicting_activations = conflict_cache::ConflictCache::new();
204-
205212
// Activate all the initial summaries to kick off some work.
206213
for (summary, opts) in summaries {
207214
debug!("initial activation: {}", summary.package_id());
@@ -313,7 +320,7 @@ fn activate_deps_loop(
313320
if let Some(c) = generalize_conflicting(
314321
&resolver_ctx,
315322
registry,
316-
&mut past_conflicting_activations,
323+
past_conflicting_activations,
317324
&parent,
318325
&dep,
319326
&conflicting_activations,

0 commit comments

Comments
 (0)