Introduce MIR summary to avoid loading large bodies without inlining them #89708

cjgillot · 2021-10-09T17:06:39Z

MIR inlining loads the bodies of all callees in order to assess whether to inline them. This implies useless decoding of large MIR bodies, which may incur a significant cost (#80536).

In order to avoid loading such large MIR, this PR introduces a MIR summary structure with pre-computed statistics. Those statistics are used as a fast-reject path for inlining.

r? @wesleywiser

compiler/rustc_middle/src/ty/mod.rs

wesleywiser · 2021-10-17T22:00:23Z

compiler/rustc_mir_transform/src/inline.rs

+        if callee_summary.diverges {
+            threshold = 0;
+        }
+        let threshold = threshold;


Nit: seems like we can remove this line 🙂

wesleywiser · 2021-10-17T22:15:19Z

compiler/rustc_mir_transform/src/lib.rs

+
+fn optimized_mir_summary<'tcx>(tcx: TyCtxt<'tcx>, did: DefId) -> Summary {
+    let body = tcx.optimized_mir(did);
+    let param_env = tcx.param_env_reveal_all_normalized(body.source.def_id());


Suggested change

let param_env = tcx.param_env_reveal_all_normalized(body.source.def_id());

let param_env = tcx.param_env_reveal_all_normalized(did);

wesleywiser · 2021-10-18T20:11:08Z

@bors try @rust-timer queue

rust-timer · 2021-10-18T20:11:10Z

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

bors · 2021-10-18T20:11:21Z

⌛ Trying commit a24590a517c6d5572a907a2e1a116c2e4cc02c13 with merge a1873ce951ce1a384c423d5b4ff6fae61d723370...

bors · 2021-10-18T21:48:09Z

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

rust-timer · 2021-10-18T21:48:10Z

Queued a1873ce951ce1a384c423d5b4ff6fae61d723370 with parent 5dab47d, future comparison URL.

rust-timer · 2021-10-18T23:45:32Z

Finished benchmarking commit (a1873ce951ce1a384c423d5b4ff6fae61d723370): comparison url.

Summary: This change led to very large relevant regressions 😿 in compiler performance.

Very large regression in instruction counts (up to 7.7% on full builds of deeply-nested-async)

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

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 led to changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

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

cjgillot · 2021-12-10T11:34:05Z

Marking as blocked on #82280

bors · 2022-02-24T12:39:43Z

☔ The latest upstream changes (presumably #94129) made this pull request unmergeable. Please resolve the merge conflicts.

rustbot · 2022-07-30T15:16:43Z

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

cjgillot · 2022-07-30T15:18:06Z

@bors try @rust-timer queue

rust-timer · 2022-07-30T15:18:07Z

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

bors · 2022-07-30T15:18:16Z

⌛ Trying commit 3f4afce with merge 92b8e388fcdff0238f30b942398546107fc237fb...

bors · 2022-07-30T16:45:03Z

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

rust-timer · 2022-07-30T16:45:05Z

Queued 92b8e388fcdff0238f30b942398546107fc237fb with parent c907b6f, future comparison URL.

rust-timer · 2022-07-30T18:46:07Z

Finished benchmarking commit (92b8e388fcdff0238f30b942398546107fc237fb): comparison url.

Instruction count

Primary benchmarks: mixed results
Secondary benchmarks: mixed results

	mean¹	max	count²
Regressions 😿 (primary)	0.4%	0.7%	10
Regressions 😿 (secondary)	0.5%	1.0%	9
Improvements 🎉 (primary)	-0.2%	-0.3%	2
Improvements 🎉 (secondary)	-0.5%	-0.7%	2
All 😿🎉 (primary)	0.3%	0.7%	12

Max RSS (memory usage)

Results

Primary benchmarks: 😿 relevant regressions found
Secondary benchmarks: 🎉 relevant improvement found

	mean¹	max	count²
Regressions 😿 (primary)	3.0%	7.5%	3
Regressions 😿 (secondary)	N/A	N/A	0
Improvements 🎉 (primary)	N/A	N/A	0
Improvements 🎉 (secondary)	-3.9%	-3.9%	1
All 😿🎉 (primary)	3.0%	7.5%	3

Cycles

Results

Primary benchmarks: no relevant changes found
Secondary benchmarks: 😿 relevant regression found

	mean¹	max	count²
Regressions 😿 (primary)	N/A	N/A	0
Regressions 😿 (secondary)	3.8%	3.8%	1
Improvements 🎉 (primary)	N/A	N/A	0
Improvements 🎉 (secondary)	N/A	N/A	0
All 😿🎉 (primary)	N/A	N/A	0

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

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.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

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

the arithmetic mean of the percent change ↩ ↩² ↩³
number of relevant changes ↩ ↩² ↩³

rust-highfive assigned wesleywiser Oct 9, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 9, 2021

tmiasko reviewed Oct 9, 2021

View reviewed changes

compiler/rustc_middle/src/ty/mod.rs Outdated Show resolved Hide resolved

apiraino added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Oct 14, 2021

wesleywiser reviewed Oct 17, 2021

View reviewed changes

cjgillot force-pushed the mir-summary branch from 9e839c6 to a24590a Compare October 18, 2021 18:12

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Oct 18, 2021

rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Oct 18, 2021

camelid added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 5, 2021

JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 5, 2021

cjgillot added S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 10, 2021

cjgillot force-pushed the mir-summary branch from a24590a to 2388230 Compare January 15, 2022 20:35

cjgillot force-pushed the mir-summary branch from 2388230 to b9d0a4e Compare January 22, 2022 23:50

cjgillot added 3 commits July 30, 2022 16:51

Extract inline cost computation.

3172dfa

Introduce optimized_mir_summary.

d41a04d

Use optimized_mir_summary.

3f4afce

cjgillot force-pushed the mir-summary branch from b9d0a4e to 3f4afce Compare July 30, 2022 15:16

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jul 30, 2022

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Jul 30, 2022

cjgillot closed this Jul 30, 2022

cjgillot deleted the mir-summary branch July 30, 2022 20:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce MIR summary to avoid loading large bodies without inlining them #89708

Introduce MIR summary to avoid loading large bodies without inlining them #89708

cjgillot commented Oct 9, 2021

wesleywiser Oct 17, 2021

wesleywiser Oct 17, 2021

wesleywiser commented Oct 18, 2021

rust-timer commented Oct 18, 2021

bors commented Oct 18, 2021

bors commented Oct 18, 2021

rust-timer commented Oct 18, 2021

rust-timer commented Oct 18, 2021

cjgillot commented Dec 10, 2021

bors commented Feb 24, 2022

rustbot commented Jul 30, 2022

cjgillot commented Jul 30, 2022

rust-timer commented Jul 30, 2022

bors commented Jul 30, 2022

bors commented Jul 30, 2022

rust-timer commented Jul 30, 2022

rust-timer commented Jul 30, 2022

	let param_env = tcx.param_env_reveal_all_normalized(body.source.def_id());
	let param_env = tcx.param_env_reveal_all_normalized(did);

Introduce MIR summary to avoid loading large bodies without inlining them #89708

Introduce MIR summary to avoid loading large bodies without inlining them #89708

Conversation

cjgillot commented Oct 9, 2021

wesleywiser Oct 17, 2021

Choose a reason for hiding this comment

wesleywiser Oct 17, 2021

Choose a reason for hiding this comment

wesleywiser commented Oct 18, 2021

rust-timer commented Oct 18, 2021

bors commented Oct 18, 2021

bors commented Oct 18, 2021

rust-timer commented Oct 18, 2021

rust-timer commented Oct 18, 2021

cjgillot commented Dec 10, 2021

bors commented Feb 24, 2022

rustbot commented Jul 30, 2022

cjgillot commented Jul 30, 2022

rust-timer commented Jul 30, 2022

bors commented Jul 30, 2022

bors commented Jul 30, 2022

rust-timer commented Jul 30, 2022

rust-timer commented Jul 30, 2022

Instruction count

Max RSS (memory usage)

Cycles

Footnotes