filled in notes.

Author: pnkfelix

triage/2024-07-02.md

@@ -1,8 +1,11 @@
 # 2024-07-02 Triage Log
 
-TODO: Summary
+Over the past 30 days, serde incr-patched:println first had a series
+of wins, accumulating 0.94% instruction count performance circa PR #126965,
+but then had a series of regression culminating with a net regression
+of 0.59% in instruction count over 30 days.
 
-Triage done by **@???**.
+Triage done by **@pnkfelix**.
 Revision range: [c3d7fb39..cf2df68d](https://perf.rust-lang.org/?start=c3d7fb398569407350abe044e786bc7890c90397&end=cf2df68d1f5e56803c97d91e2b1a9f1c9923c533&absolute=false&stat=instructions%3Au)
 
 **Summary**:
@@ -31,6 +34,10 @@ Rollup of 7 pull requests [#126951](https://github.com/rust-lang/rust/pull/12695
 | Improvements ✅ <br /> (secondary) | -    | -            | 0     |
 | All ❌✅ (primary)                 | 0.5% | [0.5%, 0.6%] | 3     |
 
+* regressions are all to serde incr-patched:println {check, debug, opt}.
+* on its own, the regression is limited to instruction counts, and seems minor enough to not warrant deeper investigation.
+* marked as triaged
+* (the 30-day history tells a slightly different story, noted in the summary)
 
 Let's `#[expect]` some lints: Stabilize `lint_reasons` (RFC 2383)  [#120924](https://github.com/rust-lang/rust/pull/120924) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=d7c59370cea68cd17006ec3440a43254fd0eda7d&end=4bc39f028d14c24b04dd17dc425432c6ec354536&stat=instructions:u)
 
@@ -42,6 +49,10 @@ Let's `#[expect]` some lints: Stabilize `lint_reasons` (RFC 2383)  [#120924](htt
 | Improvements ✅ <br /> (secondary) | -    | -            | 0     |
 | All ❌✅ (primary)                 | 0.6% | [0.2%, 1.9%] | 142   |
 
+* wide collection of regressions.
+* PR discussion indicates regression may be inherent to how `#[expect]` is implemented; it is also hypothesized to be "likely" that the implementation can be better optimized.
+* not marking as triaged.
+* (my hope is that someone will look into the "further optimizations" that xFrednet [alludes to above](https://github.com/rust-lang/rust/pull/120924#issuecomment-2202486203), and after we've done a reasonable amount of investigation there, then we can mark this as triaged.) 
 
 Update browser-ui-test version to `0.18.0` [#127010](https://github.com/rust-lang/rust/pull/127010) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=9c3bc805dd9cb84019c124b9a50fdff1e62a7ec9&end=42add88d2275b95c98e512ab680436ede691e853&stat=instructions:u)
 
@@ -53,6 +64,7 @@ Update browser-ui-test version to `0.18.0` [#127010](https://github.com/rust-lan
 | Improvements ✅ <br /> (secondary) | -    | -            | 0     |
 | All ❌✅ (primary)                 | -    | -            | 0     |
 
+* already marked as triaged (secondary benchmark deep-vector is being noisy at the moment).
 
 Implement new effects desugaring [#120639](https://github.com/rust-lang/rust/pull/120639) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=d1b7355d3d7b4ead564dbecb1d240fcc74fff21b&end=ba1d7f4a083e6402679105115ded645512a7aea8&stat=instructions:u)
 
@@ -64,6 +76,9 @@ Implement new effects desugaring [#120639](https://github.com/rust-lang/rust/pul
 | Improvements ✅ <br /> (secondary) | -    | -            | 0     |
 | All ❌✅ (primary)                 | 0.3% | [0.2%, 0.6%] | 72    |
 
+* Biggest (>=0.4%) primary regressions: regex, bitmaps, typenum, stm32f4, exa. (19 variants of those five benchmarks.)
+* the PR author (fee1-dead) has made a couple follow-up attempts to address the regressions, but nothing has hit yet.
+* not marking as triaged, in order to encourage addressing the regressions. (note however: the cycles:u metric didn't regress, at least not past our noise-filtering significance threshold. Nor did task-clock:u. It is not totally clear how much effort is warranted here, apart from a desire to keep the instruction count low just because that is our most stable proxy for "computational effort")
 
 #### Improvements
 
@@ -77,6 +92,8 @@ Save 2 pointers in `TerminatorKind` (96 → 80 bytes) [#126784](https://github.c
 | Improvements ✅ <br /> (secondary) | -0.1% | [-0.1%, -0.1%] | 4     |
 | All ❌✅ (primary)                 | -0.4% | [-0.5%, -0.2%] | 9     |
 
+* improvements are to serde and diesel.
+* skimming 30 day history indicates that the effect is real, though may have been somewhat undone by subsequent changes.
 
 rustdoc: use current stage if download-rustc enabled [#126728](https://github.com/rust-lang/rust/pull/126728) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=127fa2261b730a42e6d98b7927c3888ecd08f3e0&end=a4ce33c0b232deda1cbce447e80f187cd34952a6&stat=instructions:u)
 
@@ -88,6 +105,7 @@ rustdoc: use current stage if download-rustc enabled [#126728](https://github.co
 | Improvements ✅ <br /> (secondary) | -8.0% | [-8.0%, -8.0%] | 1     |
 | All ❌✅ (primary)                 | -     | -              | 0     |
 
+* (this was just deep-vector noise)
 
 Rollup of 9 pull requests [#127174](https://github.com/rust-lang/rust/pull/127174) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=ef3d6fd7002500af0a985f70d3ac5152623c1396&end=6868c831a1eb45c5150ff623cef5e42a8b8946d0&stat=instructions:u)
 
@@ -99,6 +117,8 @@ Rollup of 9 pull requests [#127174](https://github.com/rust-lang/rust/pull/12717
 | Improvements ✅ <br /> (secondary) | -1.3% | [-2.9%, -0.2%] | 36    |
 | All ❌✅ (primary)                 | -0.4% | [-1.1%, -0.2%] | 46    |
 
+* this had broad improvements to instruction counts, but the cycle counts metric reports that there were 13 regressions (one of which, unicode-normalization, was primary) here with only one improvement (none of which was primary).
+* nonetheless, not worth investigating further.
 
 #### Mixed
 
@@ -112,6 +132,8 @@ Rollup of 9 pull requests [#126878](https://github.com/rust-lang/rust/pull/12687
 | Improvements ✅ <br /> (secondary) | -0.3% | [-0.3%, -0.3%] | 1     |
 | All ❌✅ (primary)                 | -0.4% | [-0.5%, -0.3%] | 4     |
 
+* regressions are all to secondary benchmark: coercions.
+* marking as triaged
 
 Add `SliceLike` to `rustc_type_ir`, use it in the generic solver code (+ some other changes) [#126813](https://github.com/rust-lang/rust/pull/126813) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=6b0f4b5ec3aa707ecaa78230722117324a4ce23c&end=5b270e1198e911247244b035a6f06ce3af0a4420&stat=instructions:u)
 
@@ -123,6 +145,8 @@ Add `SliceLike` to `rustc_type_ir`, use it in the generic solver code (+ some ot
 | Improvements ✅ <br /> (secondary) | -0.7% | [-2.2%, -0.2%] | 9     |
 | All ❌✅ (primary)                 | -0.4% | [-0.6%, -0.3%] | 12    |
 
+* regressions are all to secondary benchmark: match-stress
+* marking as triaged
 
 Also get `add nuw` from `uN::checked_add` [#126852](https://github.com/rust-lang/rust/pull/126852) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=5b270e1198e911247244b035a6f06ce3af0a4420&end=fc555cd832ee743ff5410c35de2b0dd59ec4322e&stat=instructions:u)
 
@@ -134,6 +158,9 @@ Also get `add nuw` from `uN::checked_add` [#126852](https://github.com/rust-lang
 | Improvements ✅ <br /> (secondary) | -1.3% | [-1.4%, -0.9%] | 7     |
 | All ❌✅ (primary)                 | 0.4%  | [-0.3%, 0.9%]  | 5     |
 
+* PR was analyzed and thought to be a net win, despite the anticipated regression to compiler instruction-counts
+* but there was a bystander follow-up comment that the result here might be a pessimization, at least for Intel x86.
+* not marking as triaged, in hopes that follow-up comment gets addressed in some manner.
 
 ast: Standardize visiting order for attributes and node IDs [#125741](https://github.com/rust-lang/rust/pull/125741) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=c2d2bb38c9067d983d13505c47e761308b1694db&end=d929a42a664c026167800801b26d734db925314f&stat=instructions:u)
 
@@ -145,6 +172,8 @@ ast: Standardize visiting order for attributes and node IDs [#125741](https://gi
 | Improvements ✅ <br /> (secondary) | -0.3% | [-0.4%, -0.2%] | 12    |
 | All ❌✅ (primary)                 | -     | -              | 0     |
 
+* solely regressions to secondary benchmark: tt-muncher.
+* marking as triaged
 
 Rollup of 8 pull requests [#126965](https://github.com/rust-lang/rust/pull/126965) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=c290e9de32e8ba6a673ef125fde40eadd395d170&end=fda509e817abeeecb5b76bc1de844f355675c81e&stat=instructions:u)
 
@@ -156,6 +185,8 @@ Rollup of 8 pull requests [#126965](https://github.com/rust-lang/rust/pull/12696
 | Improvements ✅ <br /> (secondary) | -3.0% | [-5.7%, -0.3%] | 2     |
 | All ❌✅ (primary)                 | -     | -              | 0     |
 
+* solely regressions to secondary benchmark: derive
+* marking as triaged
 
 Remove more `PtrToPtr` casts in GVN [#126844](https://github.com/rust-lang/rust/pull/126844) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=4bdf8d2d5877f20b54c1506a607ad8c4744cc387&end=d7c59370cea68cd17006ec3440a43254fd0eda7d&stat=instructions:u)
 
@@ -167,6 +198,10 @@ Remove more `PtrToPtr` casts in GVN [#126844](https://github.com/rust-lang/rust/
 | Improvements ✅ <br /> (secondary) | -0.3% | [-0.3%, -0.3%] | 1     |
 | All ❌✅ (primary)                 | 0.6%  | [-1.1%, 2.9%]  | 6     |
 
+* Main primary regressions to opt-full benchmarks ripgrep (2.89%), webrender (1.11%), html5ever (0.70%).
+* Some interesting discussion on the PR about the cause; e.g. are PR's like this causing individual MIR reduction that leads to more inlining and then more bloat overall?
+* but I do not think any of that would cause us to undo this particular change; there are higher level inlining and code-generation policies that need to be revisited.
+* marking as triaged.
 
 Rollup of 6 pull requests [#127014](https://github.com/rust-lang/rust/pull/127014) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=536235f07e57c9108c6c3b1eacb323164e0f4cfb&end=127fa2261b730a42e6d98b7927c3888ecd08f3e0&stat=instructions:u)
 
@@ -178,6 +213,7 @@ Rollup of 6 pull requests [#127014](https://github.com/rust-lang/rust/pull/12701
 | Improvements ✅ <br /> (secondary) | -2.2% | [-5.0%, -0.2%] | 13    |
 | All ❌✅ (primary)                 | -0.2% | [-0.2%, -0.2%] | 1     |
 
+* already marked as triaged (sole regressionw as to noisy deep-vector)
 
 Rollup of 6 pull requests [#127076](https://github.com/rust-lang/rust/pull/127076) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=42add88d2275b95c98e512ab680436ede691e853&end=99f77a2eda555b50b518f74823ab636a20efb87f&stat=instructions:u)
 
@@ -189,6 +225,10 @@ Rollup of 6 pull requests [#127076](https://github.com/rust-lang/rust/pull/12707
 | Improvements ✅ <br /> (secondary) | -0.7% | [-6.2%, -0.2%] | 17    |
 | All ❌✅ (primary)                 | -0.2% | [-2.7%, 2.1%]  | 4     |
 
+* regressions are to opt-full: image (2.11%) and cargo (0.61%).
+* eyeballing the self-profile results provides a hint that we might be spending more time in LLVM optimizations passes after this rollup PR landed.
+* fired off some follow-up rust-timer builds on a couple potential culprits, but I admit that I'm only making semi-educated guesses. (outcome: It wasn't PR #124741 nor PR #126970)
+* not marking as triaged, but if no one can identify a root cause within a week, then we should just mark it so at that point.
 
 Rollup of 11 pull requests [#127096](https://github.com/rust-lang/rust/pull/127096) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=9ed2ab3790ff41bf741dd690befd6a1c1e2b23ca&end=d38cd229b75a7a608e4971c46d1fb5a99343e430&stat=instructions:u)
 
@@ -200,6 +240,13 @@ Rollup of 11 pull requests [#127096](https://github.com/rust-lang/rust/pull/1270
 | Improvements ✅ <br /> (secondary) | -     | -              | 0     |
 | All ❌✅ (primary)                 | -1.9% | [-6.2%, 0.7%]  | 19    |
 
+* all 7 primary regressions are variants of syn; all but one are incremental.
+* skimming the detailed results reports for the top three regressing variants, I see the following queries at the top of the ordering by time-delta: incr_comp_persist_dep_graph, hir_crate, codegen_copy_artifacts_from_incr_cache, early_lint_checks...
+* what in this rollup would have impacted those incremental-compilation related queries?
+* PR #1270668 already had its own dedicated rustc-perf run.
+* (is this potentially just fallout noise from internal API changes like that in PR #127071?)
+* fired off a rust-timer build against that, just to scratch that itch.
+* not marking as triaged, but if no one can identify a root cause within a week, then we should just mark it so at that point.
 
 Automatically taint InferCtxt when errors are emitted [#126996](https://github.com/rust-lang/rust/pull/126996) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=f92a6c41e644d6222be77b20396daec5e77661f3&end=7b21c18fe4de32a7d2faa468e6ef69abff013f85&stat=instructions:u)
 
@@ -211,6 +258,8 @@ Automatically taint InferCtxt when errors are emitted [#126996](https://github.c
 | Improvements ✅ <br /> (secondary) | -     | -              | 0     |
 | All ❌✅ (primary)                 | -0.2% | [-0.2%, -0.2%] | 1     |
 
+* all regressions are to secondary match-stress benchmark, and were anticipated during a perf run during review
+* marking as triaged.
 
 Avoid MIR bloat in inlining [#127113](https://github.com/rust-lang/rust/pull/127113) [(Comparison Link)](https://perf.rust-lang.org/compare.html?start=c3774be7411722d3695de2ab1da9a358d0d5c82c&end=221e2741c39515a5de6da42d8c76ee1e132c2c74&stat=instructions:u)
 
@@ -222,6 +271,11 @@ Avoid MIR bloat in inlining [#127113](https://github.com/rust-lang/rust/pull/127
 | Improvements ✅ <br /> (secondary) | -1.6% | [-4.5%, -0.2%] | 18    |
 | All ❌✅ (primary)                 | -0.3% | [-2.2%, 2.8%]  | 23    |
 
+* regressed opt-full html5ever, diesel, hyper, and clap. Also regressed ripgrep and regex in two isolated opt incremental scenarios.
+* overall gains more than it loses, as [noted after the perf run](https://github.com/rust-lang/rust/pull/127113#issuecomment-2198433788) done during PR development.
+* the big impact was to binary sizes, where the improvement is pretty clear.
+* marking as triaged.
+
 
 #### Untriaged Pull Requests