Rollup of 4 pull requests #140708
Merged
Rollup of 4 pull requests #140708
Conversation
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
This also removes some manipulation of the function signature span that only made sense in the context of merging non-adjacent spans.
Because we no longer merge non-adjacent spans, there is no need to use buckets to prevent merging across hole spans.
Update examples to no longer avoid iterating arrays for rust-lang#84513
Update iterator.rs to use arrays by value Update examples to no longer avoid iterating arrays for rust-lang#84513
coverage: Only merge adjacent coverage spans For a long time, coverage instrumentation has automatically “merged” spans with the same control-flow into a smaller number of larger spans, even when the spans being merged are not overlapping or adjacent. This causes any source text between the original spans to be included in the merged span, which is then associated with an execution count when shown in coverage reports. That approach causes a number of problems: - The intervening source text can contain all sorts of things that shouldn't really be marked as executable code (e.g. nested items, parts of macro invocations, long comments). In some cases we have complicated workarounds (e.g. bucketing to avoid merging spans across nested items), but in other cases there isn't much we can do. - Merging can have aesthetically weird effects, such as including unbalanced parentheses, because the merging process doesn't really understand what it's doing at a source code level. - It generally leads to an accumulation of piled-on heuristics and special cases that give decent-looking results, but are fiendishly difficult to modify or replace. Therefore, this PR aims to abolish the merging of non-adjacent coverage spans. The big tradeoff here is that the resulting coverage metadata (embedded in the instrumented binary) tends to become larger, because the overall number of distinct spans has increased. That's unfortunate, but I see it as the inevitable cost of cleaning up the messes and inaccuracies that were caused by the old approach. And the resulting spans do tend to be more accurate to the program's actual control-flow. --- The `.coverage` snapshot changes give an indication of how this PR will affect user-visible coverage reports. In many cases the changes to reporting are minor or even nonexistent, despite substantial changes to the metadata (as indicated by `.cov-map` snapshots). --- try-job: aarch64-gnu
Rename `graph::implementation::Graph` to `LinkedGraph` One of the more confusing parts of the `rustc_data_structures::graph` module is this mysteriously-named “Graph” type, which turns out to be an older standalone graph implementation that predates the traits used by the rest of the graph module. This graph type is still used in a couple of places (for reporting certain lifetime errors, and by certain debugging/test-only checks of the query dependency graph), but hasn't had much attention in years. This PR renames that old graph type from `implementation::Graph` to `linked_graph::LinkedGraph` to give it a more distinct identity (and make existing uses easier to find), and adds some notes to gently discourage any further use in new code. No functional change.
…oieni Handle PR not found in post-merge workflow Should hopefully fix errors like [these](rust-lang#140561 (comment)). r? `@marcoieni`
rustbot
added
A-query-system
|
@bors r+ p=4 rollup=never |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions |
|
📌 Perf builds for each rolled up PR:
previous master: 1a95cc6f9d In the case of a perf regression, run the following command for each PR you suspect might be the cause: |
What is this?This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.Comparing 1a95cc6 (parent) -> 27d6200 (this PR) Test differencesShow 452 test diffsStage 1
Additionally, 440 doctest diffs were found. These are ignored, as they are noisy. Job group index Test dashboardRun cargo run --manifest-path src/ci/citool/Cargo.toml -- \
test-dashboard 27d6200a70601f6fcf419bf2f9e37989f3624ca4 --output-dir test-dashboardAnd then open Job duration changes
How to interpret the job duration changes?Job durations can vary a lot, based on the actual runner instance |
|
Finished benchmarking commit (27d6200): comparison URL. Overall result: ❌ regressions - please read the text belowOur benchmarks found a performance regression caused by this PR. Next Steps:
@rustbot label: +perf-regression Instruction countThis is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.
Max RSS (memory usage)Results (primary 0.9%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResults (primary -0.6%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 769.964s -> 769.287s (-0.09%) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Successful merges:
graph::implementation::GraphtoLinkedGraph#140692 (Renamegraph::implementation::GraphtoLinkedGraph)r? @ghost
@rustbot modify labels: rollup
Create a similar rollup