cargo icon indicating copy to clipboard operation
cargo copied to clipboard

Native cargo coverage support

Open nagisa opened this issue 7 months ago • 3 comments

Problem

I was looking at a performance issue caused by env RUSTFLAGS="-Cinstrument-coverage" in our codebase and have determined that the major part of the degradation (~10x slowdown) was because dependencies were instrumented. In one particular case, a hot & tight AVX-intrinsic heavy code has been instrumented as well. I suspect that this was leading to AVX registers being spilled and reloaded between each intrinsic to invoke a call to the coverage runtime library(-ies.)

The best fix for me would have been to somehow exclude the dependencies external to the cargo workspace from being passed this flag (collecting coverage for external dependencies isn’t particularly useful in the first place), but I was not able to find any way to do so, before Jakub Beránek suggested to write a wrapper script that would be applied to my cargo invocations with RUSTC_WORKSPACE_WRAPPER.

The solution works, of course, but it isn’t particularly discoverable. The obvious way to achieve -Cinstrument-coverage continues to be via RUSTFLAGS and in most cases it will do something a little different from what a good default would be (to only instrument your own code.)

Proposed Solution

I believe adding a profile setting to enable or disable addition of -Cinstrument-coverage flag would be a pretty good option here. It would allow to specifically instrument the local crates only (via profile.*.packages."*".instrument-coverage=false) while also retaining a full ability to selectively or fully instrument all the other crates as well using the same mechanism.

The one negative to this proposed solution is that the obvious setting of

[profile.dev]
instrument-coverage = true

is still going to instrument all the crates, including the dependencies.

[profile.dev]
instrument-coverage = true
[profile.dev.packages."*"]
instrument-coverage = false

Notes

Relevant discussion on Zulip

cc @Ekleog cc @taiki-e (as a developer of llvm-cov)

nagisa avatar Nov 23 '23 17:11 nagisa

See also https://github.com/rust-lang/rfcs/pull/3287

epage avatar Nov 24 '23 01:11 epage

I've generalized the title so we can focus the conversation on generally solving coverage, rather than having piece-meal conversations in different issues for different solutions.

epage avatar Dec 05 '23 15:12 epage

On a few occasions I've been asked to track down coverage bugs that cause either an ICE in the compiler or a fatal error in llvm-cov, both of which make coverage reports unusable until a fix or workaround is found.

In almost all cases, the code that triggered the bug was in some third-party dependency, and not in the code that the user actually cared about getting coverage reports for.

Coverage instrumentation also causes general artifact bloat, in terms of both instructions (counter increments and inhibited optimizations) and metadata (coverage mappings embedded in binaries), such that it's preferable to instrument only those crates that the user cares about.

So I would be happy to see some mechanism for having Cargo set -Cinstrument-coverage on a per-crate basis, either via a more general per-crate rustflags feature, or by a specific instrument-coverage setting within profiles.

(The main advantage of a specific setting would be that it's much easier to specify/implement/review/stabilize, compared to the more general rustflags capability.)

Zalathar avatar Dec 20 '23 07:12 Zalathar