Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Triage 2024 10 21 #1999

Merged
merged 2 commits into from
Oct 23, 2024
Merged

Triage 2024 10 21 #1999

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
23ff293
Initial triage report template.
pnkfelix Oct 21, 2024
a773b31
triage report for this week.
pnkfelix Oct 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
159 changes: 159 additions & 0 deletions triage/2024-10-21.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,159 @@
# 2024-10-21 Triage Log

Some tidy improvements from switching to next generation trait solver
Kobzol marked this conversation as resolved.
Show resolved Hide resolved
(solely for coherence checking) and from simplifying our dataflow
analysis framework. There were some binary size regressions associated
with PR 126557 (adding `#[track_caller]` to allocating methods of
`Vec` and `VecDeque`), which I have handed off to T-libs to choose
whether to investigate further.

Triage done by **@pnkfelix**.
Revision range: [5ceb623a..3e33bda0](https://perf.rust-lang.org/?start=5ceb623a4abd66e91e7959d25caaf0523f1a7f7c&end=3e33bda0326586a6e1e34d0f5c060ca6d116e6a4&absolute=false&stat=instructions%3Au)

**Summary**:

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | 0.4% | [0.2%, 0.9%] | 43 |
| Regressions ❌ <br /> (secondary) | 0.4% | [0.2%, 0.7%] | 36 |
| Improvements ✅ <br /> (primary) | -0.8% | [-5.1%, -0.2%] | 92 |
| Improvements ✅ <br /> (secondary) | -0.8% | [-2.0%, -0.1%] | 65 |
| All ❌✅ (primary) | -0.4% | [-5.1%, 0.9%] | 135 |


0 Regressions, 3 Improvements, 6 Mixed; 3 of them in rollups
47 artifact comparisons made in total

#### Regressions



#### Improvements

stabilize `-Znext-solver=coherence` again [#130654](https://github.com/rust-lang/rust/pull/130654) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=f79fae3069c449993eda6b16934da3b144cb8a66&end=a0c2aba29aa9ea50a7c45c3391dd446f856bef7b&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | - | - | 0 |
| Regressions ❌ <br /> (secondary) | - | - | 0 |
| Improvements ✅ <br /> (primary) | -1.9% | [-4.5%, -0.2%] | 17 |
| Improvements ✅ <br /> (secondary) | -0.3% | [-0.3%, -0.3%] | 3 |
| All ❌✅ (primary) | -1.9% | [-4.5%, -0.2%] | 17 |

* improvements are to bitmaps and nalgebra
* (presumably due to their heavy use of trait machinery; though, skimming bitmaps, I'm not sure if I see heavy use of trait machinery there)
* what else can I say, except, amazing!

Remove `GenKillAnalysis` [#131481](https://github.com/rust-lang/rust/pull/131481) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=1f67a7aa8d5b30c43c28ed9b2621cf4b7b8bb963&end=d829780c4e4ef11f5e09c1c5ed9684c12aad7236&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | - | - | 0 |
| Regressions ❌ <br /> (secondary) | 1.6% | [1.6%, 1.6%] | 1 |
| Improvements ✅ <br /> (primary) | -0.4% | [-0.7%, -0.2%] | 24 |
| Improvements ✅ <br /> (secondary) | -0.9% | [-1.9%, -0.1%] | 19 |
| All ❌✅ (primary) | -0.4% | [-0.7%, -0.2%] | 24 |

* improvements are spread across html5ever, serde, cargo, libc, diesel, and bitmaps.
* wow. I'm surprised `GenKillAnalysis` is a pessimization
* (maybe this is a sign that our basic blocks tend to hold a small number of instructions...?)
* anyway, overall amazing work.

Rollup of 8 pull requests [#131792](https://github.com/rust-lang/rust/pull/131792) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=bed75e7c21e8d18bd536a0f7c9e479d2f6707db3&end=7342830c05ec0996e9e4b7df550b1043dca7829c&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | - | - | 0 |
| Regressions ❌ <br /> (secondary) | - | - | 0 |
| Improvements ✅ <br /> (primary) | -0.2% | [-0.2%, -0.2%] | 5 |
| Improvements ✅ <br /> (secondary) | - | - | 0 |
| All ❌✅ (primary) | -0.2% | [-0.2%, -0.2%] | 5 |

* improvements are to incremental scenarios for stm32f4 (and one also for libc).


#### Mixed

Add `#[track_caller]` to allocating methods of `Vec` & `VecDeque` [#126557](https://github.com/rust-lang/rust/pull/126557) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=5ceb623a4abd66e91e7959d25caaf0523f1a7f7c&end=f6648f252a05a0a46c865d7ec836b46290613bf9&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | 0.3% | [0.0%, 0.5%] | 26 |
| Regressions ❌ <br /> (secondary) | - | - | 0 |
| Improvements ✅ <br /> (primary) | - | - | 0 |
| Improvements ✅ <br /> (secondary) | -0.2% | [-0.2%, -0.1%] | 4 |
| All ❌✅ (primary) | 0.3% | [0.0%, 0.5%] | 26 |

* based on a prior perf run (which predicted 16 primary regressions of roughly the same magnitude as observed here), the T-libs team had [already approved this PR](https://github.com/rust-lang/rust/pull/126557#issuecomment-2329482185) under the assumption that there wouldn't be a *runtime impact* from this.
* there was a note from nnethercote that it isn't totally clear if the binary size increases were anticipated.
* marking as triaged

Rollup of 8 pull requests [#131690](https://github.com/rust-lang/rust/pull/131690) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=17a19e684cdf3ca088af8b4da6a6209d128913f4&end=9322d183f45e0fd5a509820874cc5ff27744a479&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | 0.4% | [0.4%, 0.4%] | 1 |
| Regressions ❌ <br /> (secondary) | - | - | 0 |
| Improvements ✅ <br /> (primary) | -0.6% | [-0.8%, -0.3%] | 2 |
| Improvements ✅ <br /> (secondary) | - | - | 0 |
| All ❌✅ (primary) | -0.2% | [-0.8%, 0.4%] | 3 |

* regression to cargo opt-full; improvements to image and html5ever opt-full
* briefly skimmed detailed results. nothing stood out.
* not worth digging into further; marking as triaged

Use `ThinVec` for PredicateObligation storage [#131422](https://github.com/rust-lang/rust/pull/131422) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=9ce3675b438aae22ef0c6147cde2003a418ab722&end=9618da7c9995a673af4841149ba2d1f53b69dd92&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | - | - | 0 |
| Regressions ❌ <br /> (secondary) | 0.3% | [0.3%, 0.5%] | 8 |
| Improvements ✅ <br /> (primary) | -0.3% | [-0.7%, -0.2%] | 15 |
| Improvements ✅ <br /> (secondary) | -0.3% | [-0.7%, -0.2%] | 23 |
| All ❌✅ (primary) | -0.3% | [-0.7%, -0.2%] | 15 |

* improvements outweigh regressions. (and reported performance matches what was anticipated via perf runs.)
* marking as triaged

optimize str.replace [#130223](https://github.com/rust-lang/rust/pull/130223) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=3a85d3fa785d95a7b7bcf4f160b67bffba7afd4a&end=86bd45979a964678b40b79156744f0057759d840&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | 0.5% | [0.2%, 1.2%] | 5 |
| Regressions ❌ <br /> (secondary) | - | - | 0 |
| Improvements ✅ <br /> (primary) | -0.8% | [-0.8%, -0.8%] | 1 |
| Improvements ✅ <br /> (secondary) | -0.4% | [-0.4%, -0.3%] | 6 |
| All ❌✅ (primary) | 0.3% | [-0.8%, 1.2%] | 6 |

* primary regressions to miscellaneous cargo and clap scenarios. (single primary improvement was to cargo incr-patched::println).
* overall seems like minor regressions, potentially "just" instrumentation bias, compared to the expected benefit for microbenchmark reported in PR description.
* marking as triaged.

Rollup of 8 pull requests [#131934](https://github.com/rust-lang/rust/pull/131934) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=a2a1206811d864df2bb61b2fc27ddc45a3589424&end=8069f8d17a6c86a8fd881939fcce359a90c57ff2&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | 0.3% | [0.3%, 0.3%] | 2 |
| Regressions ❌ <br /> (secondary) | 0.2% | [0.2%, 0.3%] | 11 |
| Improvements ✅ <br /> (primary) | -0.2% | [-0.2%, -0.2%] | 1 |
| Improvements ✅ <br /> (secondary) | - | - | 0 |
| All ❌✅ (primary) | 0.1% | [-0.2%, 0.3%] | 3 |

* primary regressions to helloworld (doc) and hyper (opt); primary improvement to libc (doc).
* all the secondary regressions are to doc benchmarks, which led lqd to hypothesize that this is due to PR 131908 which changed the hash used for filename generation to sha256 since it should be stable going forward.
* hyper history seems really noisy.
* marking as triaged

Update rustc-hash to version 2 but again [#131949](https://github.com/rust-lang/rust/pull/131949) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=de977a5acf210f7d71ff83f4b8bc42c274ce4ed9&end=662180b34d95f72d05b7c467b0baf4d23d36b1e1&stat=instructions:u)

| (instructions:u) | mean | range | count |
|:----------------------------------:|:-----:|:--------------:|:-----:|
| Regressions ❌ <br /> (primary) | 0.2% | [0.1%, 0.2%] | 8 |
| Regressions ❌ <br /> (secondary) | 0.3% | [0.1%, 0.7%] | 9 |
| Improvements ✅ <br /> (primary) | -0.3% | [-0.5%, -0.2%] | 10 |
| Improvements ✅ <br /> (secondary) | -0.9% | [-1.4%, -0.6%] | 13 |
| All ❌✅ (primary) | -0.1% | [-0.5%, 0.2%] | 18 |

* primary improvements to unicode-normalization and libc
* primary regressions to typenum and serde
* i don't think there's anything interesting to investigate here. marking as triaged.
Loading