2022-06-07 Triage Log

A busy week in compiler performance, but fortunately improvements outweighed regressions. The biggest improvements came from @nnethercote's work on making the decoding of SourceFile::lines lazy which significantly cuts the costs of decoding crate metadata. The biggest regressions came from the removal of json handling in rustc_serialize which has been a multi-month effort to improve the maintainability of json (de-)serialization in the compiler.

Triage done by @rylev. Revision range: 0a43923a..bb55bd

Summary:

	mean	max	count
Regressions 😿 (primary)	0.5%	3.2%	36
Regressions 😿 (secondary)	0.3%	0.9%	15
Improvements 🎉 (primary)	-1.3%	-15.1%	124
Improvements 🎉 (secondary)	-2.7%	-13.5%	182
All 😿🎉 (primary)	-0.9%	-15.1%	160

2 Regression, 6 Improvements, 5 Mixed; 4 of them in rollups 48 artifact comparisons made in total

Regressions

rewrite error handling for unresolved inference vars #89862 (Comparison Link)

	mean	max	count
Regressions 😿 (primary)	0.2%	0.3%	7
Regressions 😿 (secondary)	0.4%	1.0%	23
Improvements 🎉 (primary)	N/A	N/A	0
Improvements 🎉 (secondary)	N/A	N/A	0
All 😿🎉 (primary)	0.2%	0.3%	7

Ran cache grind diff and saw that ObligationForest::process_obligations is getting called a lot more.
I‘m very unfamiliar with trait resolution, so I’m unsure if this is a red herring or not.
In any case, here is the full diff.
Left a comment as such here

Remove all json handling from rustc_serialize #85993 (Comparison Link)

	mean	max	count
Regressions 😿 (primary)	0.5%	1.3%	68
Regressions 😿 (secondary)	0.9%	3.5%	40
Improvements 🎉 (primary)	-0.4%	-0.6%	3
Improvements 🎉 (secondary)	-0.5%	-1.0%	24
All 😿🎉 (primary)	0.5%	1.3%	71

@nnethercote was not able to reproduce the regressions in a local build, and it seems the consensus is that the regressions are worth the hit.

Improvements

Make params be SmallVec as originally was #97670 (Comparison Link)

	mean	max	count
Regressions 😿 (primary)	N/A	N/A	0
Regressions 😿 (secondary)	N/A	N/A	0
Improvements 🎉 (primary)	-0.2%	-0.2%	3
Improvements 🎉 (secondary)	N/A	N/A	0
All 😿🎉 (primary)	-0.2%	-0.2%	3

Add PID to LLVM PGO profile path #97137 (Comparison Link)

	mean	max	count
Regressions 😿 (primary)	N/A	N/A	0
Regressions 😿 (secondary)	N/A	N/A	0
Improvements 🎉 (primary)	-0.6%	-0.8%	20
Improvements 🎉 (secondary)	-0.7%	-1.0%	8
All 😿🎉 (primary)	-0.6%	-0.8%	20

Rollup of 6 pull requests #97742 (Comparison Link)

	mean	max	count
Regressions 😿 (primary)	N/A	N/A	0
Regressions 😿 (secondary)	N/A	N/A	0
Improvements 🎉 (primary)	N/A	N/A	0
Improvements 🎉 (secondary)	-1.9%	-2.6%	12
All 😿🎉 (primary)	N/A	N/A	0

interpret: better control over whether we read data with provenance #97684 (Comparison Link)

	mean	max	count
Regressions 😿 (primary)	N/A	N/A	0
Regressions 😿 (secondary)	N/A	N/A	0
Improvements 🎉 (primary)	-0.7%	-1.9%	8
Improvements 🎉 (secondary)	-5.5%	-10.5%	12
All 😿🎉 (primary)	-0.7%	-1.9%	8

Remove migrate borrowck mode #95565 (Comparison Link)

	mean	max	count
Regressions 😿 (primary)	N/A	N/A	0
Regressions 😿 (secondary)	N/A	N/A	0
Improvements 🎉 (primary)	-0.5%	-1.9%	47
Improvements 🎉 (secondary)	-0.5%	-1.4%	21
All 😿🎉 (primary)	-0.5%	-1.9%	47

Lazify SourceFile::lines. #97575 (Comparison Link)

	mean	max	count
Regressions 😿 (primary)	N/A	N/A	0
Regressions 😿 (secondary)	0.4%	0.5%	6
Improvements 🎉 (primary)	-1.8%	-15.3%	52
Improvements 🎉 (secondary)	-2.9%	-13.8%	124
All 😿🎉 (primary)	-1.8%	-15.3%	52

Mixed

Add #[inline] to Vec's Deref/DerefMut #97553 (Comparison Link)

	mean	max	count
Regressions 😿 (primary)	0.5%	0.8%	6
Regressions 😿 (secondary)	0.4%	0.7%	31
Improvements 🎉 (primary)	-1.2%	-1.7%	10
Improvements 🎉 (secondary)	-1.1%	-1.9%	10
All 😿🎉 (primary)	-0.5%	-1.7%	16

As with any chance to inlining, performance is expected to change and to not always have a positive impact.
The improvements outweigh the regressions (especially in primary benchmarks), and so it doesn't seem worth it to dig too much more into this.

Rollup of 6 pull requests #97644 (Comparison Link)

	mean	max	count
Regressions 😿 (primary)	N/A	N/A	0
Regressions 😿 (secondary)	0.6%	0.7%	5
Improvements 🎉 (primary)	-0.2%	-0.3%	2
Improvements 🎉 (secondary)	-0.3%	-0.5%	11
All 😿🎉 (primary)	-0.2%	-0.3%	2

The improvements outweigh the regressions (which are pretty small and contained in secondary benchmarks).
Given it‘s in a rollup, it’s not worth the effort to investigate.

Rollup of 5 pull requests #97654 (Comparison Link)

	mean	max	count
Regressions 😿 (primary)	N/A	N/A	0
Regressions 😿 (secondary)	0.6%	0.6%	1
Improvements 🎉 (primary)	N/A	N/A	0
Improvements 🎉 (secondary)	-0.6%	-0.8%	8
All 😿🎉 (primary)	N/A	N/A	0

The improvements outweigh the regressions (which are pretty small and contained in secondary benchmarks).
Given it‘s in a rollup, it’s not worth the effort to investigate.

Rollup of 3 pull requests #97694 (Comparison Link)

	mean	max	count
Regressions 😿 (primary)	0.2%	0.2%	1
Regressions 😿 (secondary)	0.3%	0.5%	5
Improvements 🎉 (primary)	-0.4%	-0.6%	12
Improvements 🎉 (secondary)	-0.3%	-0.5%	13
All 😿🎉 (primary)	-0.4%	-0.6%	13

The improvements outweigh the regressions (which are pretty small and almost completely contained in secondary benchmarks).
Given it‘s in a rollup, it’s not worth the effort to investigate.

Inline bridge::Buffer methods. #97604 (Comparison Link)

	mean	max	count
Regressions 😿 (primary)	3.8%	3.8%	1
Regressions 😿 (secondary)	N/A	N/A	0
Improvements 🎉 (primary)	-0.3%	-0.3%	1
Improvements 🎉 (secondary)	-2.1%	-2.1%	3
All 😿🎉 (primary)	1.8%	3.8%	2

I'm a bit confused as this PR was opened to address a performance regression, but it seems to be itself a partial perf regression (at least in instruction counts).
This causes a near 4% perf regression in serde_derive-1.0.136. Granted that particular benchmark has been somewhat noisy but not nearly to the level seen here.
Add a comment seeking justification