-
Notifications
You must be signed in to change notification settings - Fork 150
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge pull request #1719 from pnkfelix/triage-2023-09-13
Triage 2023 09 13
- Loading branch information
Showing
1 changed file
with
164 additions
and
0 deletions.
There are no files selected for viewing
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,164 @@ | ||
# 2023-09-13 Triage Log | ||
|
||
An interesting week. We saw a massive improvement to instruction-counts across | ||
over a hundred benchmarks, thanks to #110050 an improved encoding scheme for the | ||
dependency graphs that underlie incremental-compilation. However, these | ||
instruction-count improvements did not translate to direct cycle time | ||
improvements. We also saw an improvement to our artifact sizes due to #115306. | ||
Beyond that, we had a scattering of small regressions to instruction-counts that | ||
were justified because they were associated with bug fixes. | ||
|
||
Triage done by **@pnkfelix**. | ||
Revision range: [15e52b05..7e0261e7](https://perf.rust-lang.org/?start=15e52b05ca8f63e0da27c808680388717e5b997e&end=7e0261e7ea2085bdc0bc3d0fd6776bf343473858&absolute=false&stat=instructions%3Au) | ||
|
||
**Summary**: | ||
|
||
| (instructions:u) | mean | range | count | | ||
|:----------------------------------:|:-----:|:--------------:|:-----:| | ||
| Regressions ❌ <br /> (primary) | 2.8% | [0.7%, 10.2%] | 11 | | ||
| Regressions ❌ <br /> (secondary) | 1.5% | [0.4%, 7.7%] | 9 | | ||
| Improvements ✅ <br /> (primary) | -1.7% | [-5.9%, -0.2%] | 112 | | ||
| Improvements ✅ <br /> (secondary) | -1.3% | [-2.7%, -0.4%] | 41 | | ||
| All ❌✅ (primary) | -1.3% | [-5.9%, 10.2%] | 123 | | ||
|
||
|
||
3 Regressions, 2 Improvements, 5 Mixed; 2 of them in rollups | ||
84 artifact comparisons made in total | ||
|
||
#### Regressions | ||
|
||
Add `FreezeLock` type and use it to store `Definitions` [#115401](https://github.com/rust-lang/rust/pull/115401) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=c1d80ba9e28a9248158ab09fe593b0724647e642&end=a0c28cd9dc99d9acb015d06f6b27c640adad3550&stat=instructions:u) | ||
|
||
| (instructions:u) | mean | range | count | | ||
|:----------------------------------:|:----:|:------------:|:-----:| | ||
| Regressions ❌ <br /> (primary) | 0.3% | [0.2%, 0.4%] | 11 | | ||
| Regressions ❌ <br /> (secondary) | 0.3% | [0.3%, 0.3%] | 1 | | ||
| Improvements ✅ <br /> (primary) | - | - | 0 | | ||
| Improvements ✅ <br /> (secondary) | - | - | 0 | | ||
| All ❌✅ (primary) | 0.3% | [0.2%, 0.4%] | 11 | | ||
|
||
* The impact here is [hypothesized to be](https://github.com/rust-lang/rust/pull/115401#issuecomment-1709461275) due to serial/parallel trade-off; we benchmark the serial case and observe a small regression, while the parallel case is observing an improvement of roughly the same caliber. | ||
* Marked as triaged | ||
|
||
Rollup of 6 pull requests [#115672](https://github.com/rust-lang/rust/pull/115672) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=cd71a37f320c379df47ff64abd934f3a2da94c26&end=309af3442a1808888e3ceb2eacccbf4140eba1e0&stat=instructions:u) | ||
|
||
| (instructions:u) | mean | range | count | | ||
|:----------------------------------:|:----:|:------------:|:-----:| | ||
| Regressions ❌ <br /> (primary) | 4.2% | [0.8%, 9.8%] | 5 | | ||
| Regressions ❌ <br /> (secondary) | - | - | 0 | | ||
| Improvements ✅ <br /> (primary) | - | - | 0 | | ||
| Improvements ✅ <br /> (secondary) | - | - | 0 | | ||
| All ❌✅ (primary) | 4.2% | [0.8%, 9.8%] | 5 | | ||
|
||
* already marked as triaged | ||
* all five regressions are to doc benchmarks, due to new feature added in https://github.com/rust-lang/rust/pull/115201 | ||
|
||
Use the same DISubprogram for each instance of the same inlined function within a caller [#115417](https://github.com/rust-lang/rust/pull/115417) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=ffc48e3eda36e288f76b4022d72d94321887ebf5&end=62ebe3a2b177d50ec664798d731b8a8d1a9120d1&stat=instructions:u) | ||
|
||
| (instructions:u) | mean | range | count | | ||
|:----------------------------------:|:----:|:------------:|:-----:| | ||
| Regressions ❌ <br /> (primary) | 1.0% | [0.6%, 1.3%] | 3 | | ||
| Regressions ❌ <br /> (secondary) | - | - | 0 | | ||
| Improvements ✅ <br /> (primary) | - | - | 0 | | ||
| Improvements ✅ <br /> (secondary) | - | - | 0 | | ||
| All ❌✅ (primary) | 1.0% | [0.6%, 1.3%] | 3 | | ||
|
||
* already marked as triaged | ||
* regression was expected, though we may be able to claw back performance after resolving rust#115455 | ||
|
||
#### Improvements | ||
|
||
Span tweaks [#115594](https://github.com/rust-lang/rust/pull/115594) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=38bbc2ce03a2369d96898d58cc0aa06f1a4b5dcf&end=6cc1898f5f4f3ffec96ce2b7c3be723db558d470&stat=instructions:u) | ||
|
||
| (instructions:u) | mean | range | count | | ||
|:----------------------------------:|:-----:|:--------------:|:-----:| | ||
| Regressions ❌ <br /> (primary) | - | - | 0 | | ||
| Regressions ❌ <br /> (secondary) | - | - | 0 | | ||
| Improvements ✅ <br /> (primary) | -0.4% | [-0.4%, -0.4%] | 1 | | ||
| Improvements ✅ <br /> (secondary) | -0.4% | [-0.5%, -0.3%] | 6 | | ||
| All ❌✅ (primary) | -0.4% | [-0.4%, -0.4%] | 1 | | ||
|
||
|
||
Disentangle `Debug` and `Display` for `Ty`. [#115661](https://github.com/rust-lang/rust/pull/115661) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=e39976ff89f91b742916349859e8d877a4876783&end=7d1e416d3234bdfed6443dc2e4301f2d6f063525&stat=instructions:u) | ||
|
||
| (instructions:u) | mean | range | count | | ||
|:----------------------------------:|:-----:|:--------------:|:-----:| | ||
| Regressions ❌ <br /> (primary) | - | - | 0 | | ||
| Regressions ❌ <br /> (secondary) | - | - | 0 | | ||
| Improvements ✅ <br /> (primary) | -0.3% | [-0.3%, -0.2%] | 4 | | ||
| Improvements ✅ <br /> (secondary) | -0.3% | [-0.5%, -0.2%] | 3 | | ||
| All ❌✅ (primary) | -0.3% | [-0.3%, -0.2%] | 4 | | ||
|
||
|
||
#### Mixed | ||
|
||
Represent MIR composite debuginfo as projections instead of aggregates [#115252](https://github.com/rust-lang/rust/pull/115252) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=a0c28cd9dc99d9acb015d06f6b27c640adad3550&end=a5b2ac6906d2b688db4938f842057cde6054449c&stat=instructions:u) | ||
|
||
| (instructions:u) | mean | range | count | | ||
|:----------------------------------:|:-----:|:--------------:|:-----:| | ||
| Regressions ❌ <br /> (primary) | 3.9% | [3.9%, 3.9%] | 1 | | ||
| Regressions ❌ <br /> (secondary) | - | - | 0 | | ||
| Improvements ✅ <br /> (primary) | -0.3% | [-0.3%, -0.3%] | 2 | | ||
| Improvements ✅ <br /> (secondary) | -0.4% | [-0.4%, -0.3%] | 4 | | ||
| All ❌✅ (primary) | 1.1% | [-0.3%, 3.9%] | 3 | | ||
|
||
* The single regression is to exa-0.10.1-opt-full | ||
* However, nnethercote noted that this PR introduced broad (if small) | ||
regressions to linked artifact (aka binary) sizes (in both opt and debug settings) | ||
* not marking as triaged | ||
|
||
Use a specialized varint + bitpacking scheme for DepGraph encoding [#110050](https://github.com/rust-lang/rust/pull/110050) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=4e5b31c2b0023dba53a1b2827f4b7ac42aaaa18f&end=f00c1399987c60b4e884afc42f4aa6226855e9ae&stat=instructions:u) | ||
|
||
| (instructions:u) | mean | range | count | | ||
|:----------------------------------:|:-----:|:--------------:|:-----:| | ||
| Regressions ❌ <br /> (primary) | - | - | 0 | | ||
| Regressions ❌ <br /> (secondary) | 0.5% | [0.3%, 0.8%] | 4 | | ||
| Improvements ✅ <br /> (primary) | -1.7% | [-5.8%, -0.3%] | 104 | | ||
| Improvements ✅ <br /> (secondary) | -1.4% | [-2.9%, -0.5%] | 32 | | ||
| All ❌✅ (primary) | -1.7% | [-5.8%, -0.3%] | 104 | | ||
|
||
* on its surface, the improvements to instruction counts here clearly outweigh the regressions | ||
* it is worth noting that the cycle counts did not see the same trends; | ||
there were zero improvements and 7 primary regressions to cycle counts. | ||
* still, marking as triaged; this PR has gone through enough performance evaluation already. | ||
|
||
Rollup of 7 pull requests [#115665](https://github.com/rust-lang/rust/pull/115665) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=3d249706aa8b0167dd49efa1b3ce7cc0e9cbba08&end=de4cba3a98a15a891ad708a049c7fb5682083d97&stat=instructions:u) | ||
|
||
| (instructions:u) | mean | range | count | | ||
|:----------------------------------:|:-----:|:--------------:|:-----:| | ||
| Regressions ❌ <br /> (primary) | 0.7% | [0.6%, 0.7%] | 2 | | ||
| Regressions ❌ <br /> (secondary) | 0.6% | [0.5%, 0.7%] | 5 | | ||
| Improvements ✅ <br /> (primary) | - | - | 0 | | ||
| Improvements ✅ <br /> (secondary) | -0.3% | [-0.3%, -0.3%] | 1 | | ||
| All ❌✅ (primary) | 0.7% | [0.6%, 0.7%] | 2 | | ||
|
||
* primary regressions were helloworld-check (incr-unchanged and incr-patched:println) | ||
* marking as triaged; not worth investigating a rollup for that benchmark. | ||
|
||
Avoid a `source_span` query when encoding Spans into query results [#115657](https://github.com/rust-lang/rust/pull/115657) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=5ede9408945b46ab183dd228253297bdf62304f7&end=38bbc2ce03a2369d96898d58cc0aa06f1a4b5dcf&stat=instructions:u) | ||
|
||
| (instructions:u) | mean | range | count | | ||
|:----------------------------------:|:-----:|:--------------:|:-----:| | ||
| Regressions ❌ <br /> (primary) | 0.4% | [0.3%, 0.4%] | 2 | | ||
| Regressions ❌ <br /> (secondary) | 0.7% | [0.4%, 1.0%] | 7 | | ||
| Improvements ✅ <br /> (primary) | -0.4% | [-0.4%, -0.4%] | 2 | | ||
| Improvements ✅ <br /> (secondary) | -0.5% | [-0.6%, -0.4%] | 4 | | ||
| All ❌✅ (primary) | -0.0% | [-0.4%, 0.4%] | 4 | | ||
|
||
* primary regressions are to diesel-check (full and incr-full). | ||
* This is fixing a soundness issue with the dep-graph maintenance; therefore, these regressions seem tolerable. | ||
* Marking as triaged | ||
|
||
Encode only MIR reachable from other crates [#115306](https://github.com/rust-lang/rust/pull/115306) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=8e37c509fda1f7387895e33783cba94ea3960a29&end=7418413a7fad1c4e8b82f970bd78af030e5f813e&stat=instructions:u) | ||
|
||
| (instructions:u) | mean | range | count | | ||
|:----------------------------------:|:-----:|:--------------:|:-----:| | ||
| Regressions ❌ <br /> (primary) | 0.8% | [0.3%, 2.4%] | 15 | | ||
| Regressions ❌ <br /> (secondary) | 1.9% | [0.3%, 9.1%] | 7 | | ||
| Improvements ✅ <br /> (primary) | -1.3% | [-2.7%, -0.4%] | 12 | | ||
| Improvements ✅ <br /> (secondary) | -0.9% | [-1.2%, -0.7%] | 5 | | ||
| All ❌✅ (primary) | -0.1% | [-2.7%, 2.4%] | 27 | | ||
|
||
* the big (>1%) primary regressions were to three check-incr-unchanged cases: cranelift-codegen-0.82.1, html5ever-0.26.0, and hyper-0.14.18 | ||
* the regressions seem unfortunate, but tolerable given the improvement to linked artifact sizes | ||
* marking as triaged |