coverage: Include the highest counter ID seen in .cov-map dumps#131476
Merged
bors merged 1 commit intorust-lang:masterfrom Oct 11, 2024
Merged
coverage: Include the highest counter ID seen in .cov-map dumps#131476bors merged 1 commit intorust-lang:masterfrom
.cov-map dumps#131476bors merged 1 commit intorust-lang:masterfrom
Conversation
Zalathar
added
the
A-code-coverage
Collaborator
|
rustbot has assigned @Mark-Simulacrum. Use |
rustbot
added
the
S-waiting-on-review
Member
Author
|
One of the motivations behind this is that it's really easy to make changes that make the coverage mappings “simpler” (by using fewer expressions), at the expense of making them use more physical counters. That's not necessarily a good tradeoff, and without this extra summary it's hard to notice the downside. |
Zalathar
added
the
T-compiler
jieyouxu
approved these changes
Oct 11, 2024
Member
jieyouxu
left a comment
jieyouxu
left a comment
There was a problem hiding this comment.
AFAICT this seems reasonable. Thanks!
Member
|
@bors r+ rollup |
Collaborator
bors
added
S-waiting-on-bors
Zalathar
added a commit
to Zalathar/rust
that referenced
this pull request
Oct 11, 2024
coverage: Include the highest counter ID seen in `.cov-map` dumps When making changes that have a large impact on coverage counter creation, this makes it easier to see whether the number of physical counters has changed. (The highest counter ID seen in coverage maps is not necessarily the same as the number of physical counters actually used by the instrumented code, but it's the best approximation we can get from looking only at the coverage maps, and it should be reasonably accurate in most cases.) Extracted from rust-lang#131398, since I'm still considering whether to make those changes as-is, whereas this PR is useful and good on its own.
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Oct 11, 2024
…iaskrgr Rollup of 5 pull requests Successful merges: - rust-lang#131464 (Update wasm-component-ld to 0.5.10) - rust-lang#131498 (Consider outermost const-anon in `non_local_def` lint) - rust-lang#131512 (Fixing rustDoc for LayoutError.) - rust-lang#131529 (rustdoc-json-types: fix typo in comment) - rust-lang#131531 (move dummy commit logic into x86_64-gnu-llvm-18) Failed merges: - rust-lang#131476 (coverage: Include the highest counter ID seen in `.cov-map` dumps) r? `@ghost` `@rustbot` modify labels: rollup
When making changes that have a large impact on coverage counter creation, this makes it easier to see whether the number of physical counters has changed. (The highest counter ID seen in coverage maps is not necessarily the same as the number of physical counters actually used by the instrumented code, but it's the best approximation we can get from looking only at the coverage maps, and it should be reasonably accurate in most cases.)
Zalathar
force-pushed
the
highest-counter
branch
from
a9fe2f7 to
599f95e
Compare
Member
Author
Collaborator
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Oct 11, 2024
…iaskrgr Rollup of 6 pull requests Successful merges: - rust-lang#131464 (Update wasm-component-ld to 0.5.10) - rust-lang#131476 (coverage: Include the highest counter ID seen in `.cov-map` dumps) - rust-lang#131497 (Add myself to bootstrap review rotation) - rust-lang#131498 (Consider outermost const-anon in `non_local_def` lint) - rust-lang#131512 (Fixing rustDoc for LayoutError.) - rust-lang#131529 (rustdoc-json-types: fix typo in comment) r? `@ghost` `@rustbot` modify labels: rollup
rust-timer
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Oct 11, 2024
Rollup merge of rust-lang#131476 - Zalathar:highest-counter, r=jieyouxu coverage: Include the highest counter ID seen in `.cov-map` dumps When making changes that have a large impact on coverage counter creation, this makes it easier to see whether the number of physical counters has changed. (The highest counter ID seen in coverage maps is not necessarily the same as the number of physical counters actually used by the instrumented code, but it's the best approximation we can get from looking only at the coverage maps, and it should be reasonably accurate in most cases.) Extracted from rust-lang#131398, since I'm still considering whether to make those changes as-is, whereas this PR is useful and good on its own.
Contributor
|
@bors r- still shows up in the queue |
bors
added
S-waiting-on-author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When making changes that have a large impact on coverage counter creation, this makes it easier to see whether the number of physical counters has changed.
(The highest counter ID seen in coverage maps is not necessarily the same as the number of physical counters actually used by the instrumented code, but it's the best approximation we can get from looking only at the coverage maps, and it should be reasonably accurate in most cases.)
Extracted from #131398, since I'm still considering whether to make those changes as-is, whereas this PR is useful and good on its own.