fix(resolver): share conflict cache between activation retries

[x-hgg-x]

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:

cargo/src/cargo/core/resolver/dep_cache.rs

Lines 248 to 266 in 82c489f

	let mut deps = deps
	.into_iter()
	.filter_map(|(dep, features)| match self.query(&dep, first_version) {
	Poll::Ready(Ok(candidates)) => Some(Ok((dep, candidates, features))),
	Poll::Pending => {
	all_ready = false;
	// we can ignore Pending deps, resolve will be repeatedly called
	// until there are none to ignore
	None
	}
	Poll::Ready(Err(e)) => Some(Err(e).with_context(|| {
	format!(
	"failed to get `{}` as a dependency of {}",
	dep.package_name(),
	describe_path_in_context(cx, &candidate.package_id()),
	)
	})),
	})
	.collect::<CargoResult<Vec<DepInfo>>>()?;

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.

[rustbot]

r? @ehuss

rustbot has assigned @ehuss.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[x-hgg-x]

r? Eh2406

[epage]

To double check my understanding, the slowdown is only relevant if you don't have a lockfile because wee prefetch deps in a lockfile?

[x-hgg-x]

To double check my understanding, the slowdown is only relevant if you don't have a lockfile because wee prefetch deps in a lockfile?

Yes, when a lockfile is present the resolve takes only a few milliseconds.

[Eh2406]

This is a delightfully small diff. The argument for correctness is kind of suttle. But now that we have talked it over N times, I am convinced it is correct.

@bors r+

[bors]

📌 Commit 46ea11a has been approved by Eh2406

It is now in the queue for this repository.

[bors]

⌛ Testing commit 46ea11a with merge 430fabe...

[bors]

☀️ Test successful - checks-actions
Approved by: Eh2406
Pushing 430fabe to master...