2022-08-24 Triage Log

Overall some really impressive wins this week. Note in particular PR #100209, “Lazily decode SourceFile from metadata” (which improved 75 primary benchmark scenarios and 158 secondary scenarios) and PR #98655 “Don't derive PartialEq::ne”, which improved 65 primary scenarios and 27 secondary scenarios). There were a few cases that pnkfelix explicitly decided not to mark as triaged; see report for more details there. Also pnkfelix wonders if there is a recent slight-upward trend on max-rss for the past week, see the summary graph

Triage done by @pnkfelix. Revision range: 14a459bf..4a24f08b

Summary:

(instructions:u)meanrangecount
Regressions ❌
(primary)		0.6%[0.4%, 0.8%]27
Regressions ❌
(secondary)		0.4%[0.2%, 0.6%]9
Improvements ✅
(primary)		-1.7%[-20.1%, -0.3%]91
Improvements ✅
(secondary)		-3.6%[-18.7%, -0.3%]160
All ❌✅ (primary)-1.2%[-20.1%, 0.8%]118

3 Regressions, 4 Improvements, 4 Mixed; 3 of them in rollups 43 artifact comparisons made in total

Regressions

Rollup of 15 pull requests #100677 (Comparison Link)

(instructions:u)meanrangecount
Regressions ❌
(primary)		0.3%[0.3%, 0.3%]2
Regressions ❌
(secondary)		1.3%[0.5%, 1.9%]4
Improvements ✅
(primary)		--0
Improvements ✅
(secondary)		--0
All ❌✅ (primary)0.3%[0.3%, 0.3%]2
  • lqd hypothesized this was caused by PR #100652 “Remove deferred sized checks (make them eager)”
  • regressions for #100652 include most of the rollup regressions, all by similar amounts (only ucd was absent from the narrower view).
  • left a comment on PR #100652 and marked it as a regression; marked rollup as triaged.

rustc_metadata: dedupe strings to prevent multiple copies in rmeta/query cache blow file size #98851 (Comparison Link)

(instructions:u)meanrangecount
Regressions ❌
(primary)		0.6%[0.3%, 1.5%]15
Regressions ❌
(secondary)		1.1%[0.3%, 1.6%]21
Improvements ✅
(primary)		--0
Improvements ✅
(secondary)		-0.4%[-0.6%, -0.2%]2
All ❌✅ (primary)0.6%[0.3%, 1.5%]15
  • the performance of this PR was heavily evaluated as part of its development.
  • some regression to instruction counts is compensated for by the improvements file-size and to max-rss.
  • the follow-up PR #100803 is going to more than compensate for the regressions here.
  • marked as triaged.

implied bounds: explicitly state which types are assumed to be wf #100676 (Comparison Link)

(instructions:u)meanrangecount
Regressions ❌
(primary)		0.3%[0.2%, 0.5%]22
Regressions ❌
(secondary)		0.4%[0.2%, 0.8%]22
Improvements ✅
(primary)		--0
Improvements ✅
(secondary)		--0
All ❌✅ (primary)0.3%[0.2%, 0.5%]22
  • This PR was intended to be a refactor, but it turns out it has other problems (see issue 100910).
  • The regressions alone are not cause to revert the PR, but the soundness bug pushes me over the line.
  • Nominated for discussion (of revert) in Thursday's T-compiler meeting. Not tagging as triaged.

Improvements

Don't derive PartialEq::ne. #98655 (Comparison Link)

(instructions:u)meanrangecount
Regressions ❌
(primary)		--0
Regressions ❌
(secondary)		1.0%[1.0%, 1.0%]1
Improvements ✅
(primary)		-0.7%[-1.4%, -0.2%]65
Improvements ✅
(secondary)		-5.2%[-10.0%, -0.3%]27
All ❌✅ (primary)-0.7%[-1.4%, -0.2%]65

Lazily decode SourceFile from metadata #100209 (Comparison Link)

(instructions:u)meanrangecount
Regressions ❌
(primary)		0.2%[0.2%, 0.2%]2
Regressions ❌
(secondary)		0.7%[0.4%, 0.9%]3
Improvements ✅
(primary)		-1.6%[-19.6%, -0.2%]75
Improvements ✅
(secondary)		-3.0%[-18.3%, -0.2%]158
All ❌✅ (primary)-1.6%[-19.6%, 0.2%]77
  • Don‘t get too excited y’all, that 19.6% improvement was to helloworld.
  • having said that, this does represent a huge win across a broad suite of benchmarks, nearly all in incremental.
  • (also, lqd notes that helloworld is a useful proxy for near-trivial build.rs scripts.)

Update minifier version to 0.2.2 #100624 (Comparison Link)

(instructions:u)meanrangecount
Regressions ❌
(primary)		--0
Regressions ❌
(secondary)		--0
Improvements ✅
(primary)		-0.6%[-1.6%, -0.3%]13
Improvements ✅
(secondary)		-1.1%[-1.5%, -0.3%]20
All ❌✅ (primary)-0.6%[-1.6%, -0.3%]13
  • As noted by nnethercote, the cycles and max-rss results are neutral or under noise threshold, while instruction counts improved.

Kind-less SessionDiagnostic derive #100765 (Comparison Link)

(instructions:u)meanrangecount
Regressions ❌
(primary)		--0
Regressions ❌
(secondary)		--0
Improvements ✅
(primary)		-0.8%[-0.9%, -0.6%]5
Improvements ✅
(secondary)		--0
All ❌✅ (primary)-0.8%[-0.9%, -0.6%]5
  • all five improvements are to instances of regex-opt-incr-{patched,full} benchmark

Mixed

Rollup of 9 pull requests #100810 (Comparison Link)

(instructions:u)meanrangecount
Regressions ❌
(primary)		--0
Regressions ❌
(secondary)		0.3%[0.2%, 0.3%]2
Improvements ✅
(primary)		-0.7%[-0.9%, -0.3%]8
Improvements ✅
(secondary)		--0
All ❌✅ (primary)-0.7%[-0.9%, -0.3%]8
  • already triaged: “The small number of small improvements slightly outweighs the small number of small regressions. No further action is needed.”

update Miri #100841 (Comparison Link)

(instructions:u)meanrangecount
Regressions ❌
(primary)		0.8%[0.5%, 1.0%]5
Regressions ❌
(secondary)		0.2%[0.2%, 0.2%]1
Improvements ✅
(primary)		-0.8%[-0.8%, -0.8%]1
Improvements ✅
(secondary)		--0
All ❌✅ (primary)0.5%[-0.8%, 1.0%]6
  • regressions were in regex-opt-incr-patched (and ucd-doc-full, but that was just 0.24%)
  • while the regression here is unfortunate, there is not much we can expect to do in the short term to address it
  • its not even clear whether miri is really at fault; the detailed query info says that the regression is due to LLVM_lto_optimize. Could the miri changes have somehow caused the codegen unit partitioning to change? Why would a miri update affect the time for LLVM_lto_optimize?
  • not marking as triaged. I‘m not sure if anyone can justify spending time to look at this, but I don’t want to just let it slide through just yet.

Rollup of 11 pull requests #100847 (Comparison Link)

(instructions:u)meanrangecount
Regressions ❌
(primary)		--0
Regressions ❌
(secondary)		0.2%[0.2%, 0.2%]1
Improvements ✅
(primary)		-0.5%[-0.5%, -0.5%]1
Improvements ✅
(secondary)		-0.7%[-1.1%, -0.3%]5
All ❌✅ (primary)-0.5%[-0.5%, -0.5%]1
  • benefits here heavily outweigh the one minor regression.
  • already triaged by nnethercote

Use AttrVec more #100668 (Comparison Link)

(instructions:u)meanrangecount
Regressions ❌
(primary)		0.6%[0.2%, 1.0%]2
Regressions ❌
(secondary)		0.6%[0.3%, 1.3%]8
Improvements ✅
(primary)		-0.3%[-0.4%, -0.2%]7
Improvements ✅
(secondary)		-0.5%[-1.2%, -0.2%]15
All ❌✅ (primary)-0.1%[-0.4%, 1.0%]9
  • already triaged by nnethercote: “a few small wins and losses here, which balance each other out, and the net effect is perf-neutral.”

Untriaged Pull Requests