MIR dominators: computed on-demand and cached

[tmiasko]

• Cached inside MIR body - motivated by potential reuse opportunity when
  code generating different instantiations of the same generic code.

• Delayed until demanded in borrowck - motivated by observation that
  some check / metadata only builds, where dominator computation is hot,
  never query the dominator tree.

[rustbot]

r? @michaelwoerister

(rustbot has picked a reviewer for you, use r? to override)

[tmiasko]

@bors try @rust-timer queue

[bors]

⌛ Trying commit b27385511d924359203d0e48811d3007cb00b609 with merge ac9ac65b7d6f65d110c0805046ceecd553737a3f...

[bors]

☀️ Try build successful - checks-actions
Build commit: ac9ac65b7d6f65d110c0805046ceecd553737a3f (ac9ac65b7d6f65d110c0805046ceecd553737a3f)

[bors]

☀️ Try build successful - checks-actions
Build commit: ac9ac65b7d6f65d110c0805046ceecd553737a3f (ac9ac65b7d6f65d110c0805046ceecd553737a3f)

[rust-timer]

Finished benchmarking commit (ac9ac65b7d6f65d110c0805046ceecd553737a3f): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.3%	[-0.5%, -0.2%]	22
Improvements ✅ (secondary)	-0.6%	[-1.4%, -0.2%]	15
All ❌✅ (primary)	-0.3%	[-0.5%, -0.2%]	22

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	0.8%	[0.5%, 1.3%]	3
Regressions ❌ (secondary)	1.6%	[0.8%, 3.3%]	3
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-5.3%	[-5.3%, -5.3%]	1
All ❌✅ (primary)	0.8%	[0.5%, 1.3%]	3

Cycles

This benchmark run did not return any relevant results for this metric.

[tmiasko]

Separated part of changes into #106976 and #106975. Not sure if caching dominators globally is good time / memory trade-off.

[cjgillot]

I think this PR is a good idea. From the perf report, average effect on max-rss is almost 0, so we can't conclude that this caching has a significant memory impact, while it sure has a instr-count impact.

Borrowck is not the only place that may use dominators. #106908 and #106285 may benefit from this caching too.

[tmiasko]

If you are still interested in this, I can reopen and rerun perf after merging the borrowck changes in #106976, so the effect of caching is benchmarked in isolation.

[tmiasko]

@bors try @rust-timer queue

[bors]

⌛ Trying commit a705ccb with merge 31e5d42f7a60fc1ac5c6b1c67ff019141c11a208...

[bors]

☀️ Try build successful - checks-actions
Build commit: 31e5d42f7a60fc1ac5c6b1c67ff019141c11a208 (31e5d42f7a60fc1ac5c6b1c67ff019141c11a208)

[bors]

☀️ Try build successful - checks-actions
Build commit: 31e5d42f7a60fc1ac5c6b1c67ff019141c11a208 (31e5d42f7a60fc1ac5c6b1c67ff019141c11a208)

[rust-timer]

Finished benchmarking commit (31e5d42f7a60fc1ac5c6b1c67ff019141c11a208): comparison URL.

Overall result: ❌ regressions - no action needed

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.5%	[0.5%, 0.5%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	0.5%	[0.5%, 0.6%]	2
Regressions ❌ (secondary)	1.2%	[1.1%, 1.2%]	3
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.7%	[-6.7%, -2.0%]	4
All ❌✅ (primary)	0.5%	[0.5%, 0.6%]	2

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	1.7%	[1.2%, 2.1%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

[tmiasko]

Caching on its own is not an improvement right now. It might make sense to revisit this after #106908, #106285, #107172 land, if all those passes can indeed be organized to reuse the computation.