Loading of a git source is slow due to always checking if the submodule is updated

[epage]

When you have some git dependencies or patches, loading them takes a long time

For the traces from #14238, notice how slow the first resolve is compared to the second. A lot of the time is taken up checking each git dependencies submodules

Note: this slow down is most relevant for tooling like rust-analyzer or rust-defined completions when it may be called in interactive contexts without any compilation happening or "cargo script" where cargo build is performed every time you run your script.

Related: #14395

[epage]

Previously discussed at https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/Redundant.20code.20in.20.60GitSouce.60.3F

[epage]

When we GitDatabase::copy_into a checkout, we skip it if the checkout is "fresh"

cargo/src/cargo/sources/git/utils.rs

Lines 174 to 185 in 9d66d13

	// If the existing checkout exists, and it is fresh, use it.
	// A non-fresh checkout can happen if the checkout operation was
	// interrupted. In that case, the checkout gets deleted and a new
	// clone is created.
	let checkout = match git2::Repository::open(dest)
	.ok()
	.map(|repo| GitCheckout::new(self, rev, repo))
	.filter(|co| co.is_fresh())
	{
	Some(co) => co,
	None => GitCheckout::clone_into(dest, self, rev, gctx)?,
	};

We then unconditionally check for submodules

cargo/src/cargo/sources/git/utils.rs

Line 186 in 9d66d13

checkout.update_submodules(gctx)?;

submoduels should be as immutable as the checkout and we should be able to guard the whole process with the freshness check.

The main risk in doing that is if you switch back and forth between old cargo and Cargo with the submodules covered by the freshness check and the submodule update is interrupted. New cargo will see that the freshness check passxes and assumes the submodule is valid and goes on its way. We could do extra fancy things with the file used for this like in #14395 but it doesn't seem worth it.

[epage]

CC @osiewicz

[osiewicz]

It seems like old cargo will "heal" itself from the corrupted checkout, as it'll remove it and refetch everything. Given that, I would agree that we could just proceed as is? Alternatively, could we bundle submodule freshness check with the checkout freshness check?

[epage]

It seems like old cargo will "heal" itself from the corrupted checkout, as it'll remove it and refetch everything. Given that, I would agree that we could just proceed as is? Alternatively, could we bundle submodule freshness check with the checkout freshness check?

The corner case is

1. With old cargo
2. See lack of freshness file
3. Fetch commit
4. Checkout commit
5. Write freshness file
6. Perform submodule update and get interrupted
7. With new cargo
8. See freshness file and assume commit is fetched, checked out, and submodules are updated, bypassing all other steps

This kind of quick back and forth is likely when working directly with Cargo and an IDE running Cargo in the background.

That said, I think we would be fine with this. This will be limited to a specific set of cargo versions run in a specific order, with an interruption. Over time, the combination of versions will become less likely.

If we did something else about this, some potential mitigations

• For a transition period, always do a submodule update but write the file after it
• Store in the file whether the submodule update happened
• Use a second file this would cause problems with old Cargo's reading the check out iiuc