`cargo fix` checks packages serially, even if there is nothing to fix

[epage]

Problem

Running cargo fix (and worse, cargo clippy --fix on https://github.com/crate-ci/typos is very slow. For clippy, its over 2m.

Proposed Solution

Packages are checked serially. Seems like we could speculatively check them in parallel in case nothing is affected.

Notes

See also
https://rust-lang.zulipchat.com/#narrow/channel/246057-t-cargo/topic/.E2.9C.94.20Performance.20of.20.60cargo.20fix.60

[ehuss]

I suspect this will be somewhat challenging to do. It may be possible to run the "collect suggestions" in parallel. But if any thread needs to actually apply a suggestion, it will need to acquire an exclusive lock, apply the suggestions, and importantly invalidate any threads that have already collected some suggestions (because the suggestions may no longer be valid), and force them to run rustc again. That could potentially cause even more calls to rustc.

[epage]

I'm going to roughly break things down into

• Every crate has suggestions
• A couple of crates with suggestions
• No suggestions

We should probably consider which cases we consider common and should optimize for as I'm assuming that serialization and invalidation you mentioned would affect these cases differently.

The last two are what I ran into. I'm assuming that is more representative but I can't say for sure.

[epage]

Further refining my idea, it is to have cargo fix to run cargo check --message-format json in a loop until we hit a fixed point.

• Parallel without any special handling
• Easily extends itself to Cargo lints
• Simplified architecture
• Make it easier to add additional features, like interactivity, as we'd be touching cargo fix and not any weird compiler driver or compilation internals

Because a change in a lower level package can affect higher level packages, we could stop once we receive suggestions for a package, apply them, and run the next loop again. Our rebuild detection should keep this loop relatively fast. We could even kill the child process once we have sufficient information to avoid extra work.

iirc that should also extend to build targets, and not just packages, but we can't quite distinguish build targets. This adds a layer of risk to the idea.

Another area of risk is source files shared across build-targets or packages. The first is more common than the second but both can be relatively rare.

This will put per-call overhead of Cargo into a loop, slowing it down, particularly

• Loading of workspace
• Dependency resolution
• Parsing of manifest for dependencies

Speeding these up will also help Cargo more generally and particularly

• Cargo script
• Dynamic completions
• Large workspaces

We'll have to see how things go before we decide whether performance is blocking for merging or not.

The great thing about this design is we can prototype it as a third-party subcommand before switching the built-in command to it.

[epage]

This idea is optimized for very few corrections to be made. We start to slow down if we have to loop over the same package multiple times. Its unclear what the break even point is between the current design and my proposal.

To make this more clear, I'll quote from @ehuss

Let's say there is a workspace with relationship A->B->C.

1. Loop: Compile and fixes crate A.
2. Loop: Compile and fixes crate B.
3. Loop: Compile and fixes crate C.

If there are fixes in those crates, and it instead processed them as a whole, it would be:

1. Loop:
   1. Compile A, B, C.
   2. Fix A
2. Loop:
   1. Compile B, C.
   2. Fix B
3. Loop:
   1. Compile C.
   2. Fix C

I would expect that to be potentially very expensive (where B and C could need to be rebuilt dozens of times).

[epage]

A third-party subcommand is being developed to try this out. See https://github.com/crate-ci/cargo-fixit

[epage]

cargo fixit (https://crates.io/crates/cargo-fixit) is now in a usable state and I'd recommend people try it out and provide feedback, especially if you identify a case that is slower than cargo fix.

[epage]

@ehuss I'm trying to remember the various corner cases cargo fix has to deal with.

There is "fixes in a dependency can affect a package", so we can't fix a package until all dependencies are fixed. I'm assuming that could be modified to "until all linked dependencies for a build target are fixed", meaning that proc-macros and build scripts don't matter as well as dev-dependencies for lib/bins.

I think there was also "multiple build units include the same source file". This is especially bad when fixes are being handled in separate processes because there is no knowledge of what anything else is doing but that goes away if the fixes are being managed in the same process with a whole-build-tree view of fixes. Could we just go ahead and rely on our "multiple fixes for the same file" logic in that case or are there other corner cases to deal with? If we can find a good balance here, we could possibly fix multiple packages in parallel or even multiple build units within a package (just not the lib in parallel to bins/examples/tests/benches).