Rollup of 5 pull requests - #161783
Conversation
…08 notes The `for<...>` prefixes in `cmp_fn_sig` are built from the region map returned by `name_all_regions`. That map also contained regions that are bound by an enclosing binder and merely escape through the binder being named, so nested binders in expected/found notes listed lifetimes they don't bind, printing invalid types such as `&mut for<'a> fn(for<'a> fn(&'a ()))` for `&mut for<'a> fn(fn(&'a ()))`. Key the folder's map by the region's binder offset and only return the regions actually bound by the binder being named. The offset in the key also fixes a latent collision between a bound and an escaping region sharing the same bound variable index. The printed text is unaffected: the `name` closure already skips escaping regions when writing to the printer, which is why diagnostic labels were already correct.
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 787af2b (parent) -> 0f33d09 (this PR) Test differencesShow 5547 test diffsStage 1
Stage 2
Additionally, 5542 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 0f33d0912709e847199218ebab88e2311872f364 --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 (0f33d09): comparison URL. Overall result: ❌ regressions - please read:Our benchmarks found a performance regression caused by this PR. Next Steps:
@rustbot label: +perf-regression Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary -0.0%, secondary -4.1%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (secondary 2.3%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeResults (primary -0.1%, secondary 0.1%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 474.247s -> 475.39s (0.24%) |
|
📌 Perf builds for each rolled up PR:
parent commit: 787af2b8c8 In the case of a perf regression, run the following command for each PR you suspect might be the cause: |
|
#161464 was benchmarked before merge, #161774 is just a test. It looks like it could be either of the remaining three, the clippy one changes various things in the standard library, it has a perf run but not on the final diff I believe. The Allocator changes a bunch of trait bounds. The Detailed results show that there are more hits on @rust-timer triage f951449 |
#161684 f951449 Replace
|
| mean | range | count | |
|---|---|---|---|
| Regressions ❌ (primary) |
0.3% | [0.3%, 0.3%] | 1 |
| Regressions ❌ (secondary) |
0.3% | [0.3%, 0.3%] | 2 |
| Improvements ✅ (primary) |
- | - | 0 |
| Improvements ✅ (secondary) |
- | - | 0 |
| All ❌✅ (primary) | 0.3% | [0.3%, 0.3%] | 1 |
Max RSS (memory usage)
Results (primary -0.1%, secondary -3.7%)
A less reliable metric. May be of interest, but not used to determine the overall result above.
| mean | range | count | |
|---|---|---|---|
| Regressions ❌ (primary) |
2.0% | [2.0%, 2.0%] | 1 |
| Regressions ❌ (secondary) |
- | - | 0 |
| Improvements ✅ (primary) |
-2.2% | [-2.2%, -2.2%] | 1 |
| Improvements ✅ (secondary) |
-3.7% | [-3.7%, -3.7%] | 1 |
| All ❌✅ (primary) | -0.1% | [-2.2%, 2.0%] | 2 |
Cycles
Results (primary 3.2%, secondary 1.7%)
A less reliable metric. May be of interest, but not used to determine the overall result above.
| mean | range | count | |
|---|---|---|---|
| Regressions ❌ (primary) |
3.2% | [3.2%, 3.2%] | 1 |
| Regressions ❌ (secondary) |
1.7% | [1.7%, 1.7%] | 1 |
| Improvements ✅ (primary) |
- | - | 0 |
| Improvements ✅ (secondary) |
- | - | 0 |
| All ❌✅ (primary) | 3.2% | [3.2%, 3.2%] | 1 |
Binary size
Results (primary 0.0%, secondary 0.0%)
A less reliable metric. May be of interest, but not used to determine the overall result above.
| mean | range | count | |
|---|---|---|---|
| Regressions ❌ (primary) |
0.0% | [0.0%, 0.0%] | 4 |
| Regressions ❌ (secondary) |
0.0% | [0.0%, 0.0%] | 1 |
| Improvements ✅ (primary) |
- | - | 0 |
| Improvements ✅ (secondary) |
- | - | 0 |
| All ❌✅ (primary) | 0.0% | [0.0%, 0.0%] | 4 |
|
That PR literally just find-and-replaces Edit: Ah this is compile time stuff. Then I have no idea ^^ |
|
More specifically it's I'm not sure what to do about it, but I think this is not really a problem of the original PR. If there's a way to address it, it's arguably in rustdoc code and not in here. I'll ask in @rustbot label: +perf-regression-triaged |
|
Yeah this was needed for soundness, so no way around this until we come up with a more permanent solution for how to do allocators in btree. But why is the regression is serde of all places? |
|
I guess it's because |
|
i would've expected it to only do that stuff in bounds and signatures. There is no way serde is using anything other than the global allocator with |
|
Not sure, we'd have to measure. But you can see in detailed results that there are a lot more query hits for Not sure why lang items (are Allocator traits lang items?) but implied clauses are (I believe) related to the fact that |
|
Probably because
|
|
I just checked, there is not a single result for So it only appears in |
View all comments
Successful merges:
Allocator + ClonewithAllocatorClonein btree #161684 (ReplaceAllocator + ClonewithAllocatorClonein btree)for<...>binders of E0308 notes #159232 (Don't list escaping bound regions in nestedfor<...>binders of E0308 notes)rustc_builtin_macros#161464 (various cleanups ofrustc_builtin_macros)r? @ghost
Create a similar rollup