Avoid cloning parsed TOML manifest in ManifestErrorContext

[Kobzol]

What does this PR try to resolve?

I was profiling the build performance of bors, and noticed that running Cargo on a no-op build takes >300ms, which seemed like a lot. I found a few places that could be micro-optimized, this one is the one that produced the largest performance win with the fewest code changes.

I noticed that ManifestErrorContext is being unconditionally created for each compiled unit (whether fresh or not), even though in the vast majority of cases, there are no manifest errors to be enriched. The error context clones a bunch of things. While taking a look at memory profiles using Valgrind's dhat, I noticed that a lot of time and memory allocations were spent on cloning the TOML document, which happened inside the error context.

This PR changes the representation of the TOML manifest from Rc to Arc, so that it can be directly used in ManifestErrorContext without cloning the whole thing.

This produced a ~6% wall-time win on the Cargo invocation on bors:

Benchmark 1: /projects/personal/rust/cargo/target/release/cargo test --no-run
Time (mean ± σ):     317.8 ms ±   1.0 ms    [User: 204.1 ms, System: 117.6 ms]
Range (min … max):   316.5 ms … 319.7 ms    10 runs

Benchmark 2: ./cargo-baseline test --no-run
Time (mean ± σ):     336.4 ms ±   1.3 ms    [User: 218.7 ms, System: 121.4 ms]
Range (min … max):   334.8 ms … 339.5 ms    10 runs

Summary
/projects/personal/rust/cargo/target/release/cargo test --no-run ran
1.06 ± 0.01 times faster than ./cargo-baseline test --no-run

I compiled Cargo with lto = "thin" to emulate the configuration we use in the Cargo that is distributed through Rustup.

How to test and review this PR?

I suppose try to run and benchmark Cargo on a project with hundreds of deps before and after this change :)

[rustbot]

r? @weihanglo

rustbot has assigned @weihanglo.
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

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: @ehuss, @epage, @weihanglo
• @ehuss, @epage, @weihanglo expanded to ehuss, epage, weihanglo
• Random selection from ehuss, epage, weihanglo

[epage]

Thanks!

This is a newer, experimental code path so we'll see if it sticks around but worth improving in the mean time.

I suspect we'll eventually move more Rcs to Arcs over time.

View changes since this review

[Kobzol]

Makes sense. Another big part of allocations and performance is parsing the TOML manifests, which is of course a "known bottleneck" :) I wonder if there is some opportunity for zero-copy references into the original input, either for the TOML manifest itself, or at least for data derived from it, e.g. summaries.

[epage]

I've wanted to improve things there.
If you have any ideas, would love to hear them. Typically, I have run into one or more of

• managing lifetimes
• barriers with serde
• keeping cargo-util-schemas usable by others
• specialized, invasive toml changes
• design trade offs in toml