Enable MIR inlining of very small functions in debug builds #112255

saethlin · 2023-06-03T19:00:15Z

This interacts with #116505

r? @ghost

saethlin · 2023-06-03T19:00:30Z

@bors try @rust-timer queue

bors · 2023-06-03T19:00:38Z

⌛ Trying commit 533fdbfa47c755688f9ff142949809a3cda52c91 with merge fc943c1fd851b1eec39f3a13184baec258b4afc7...

saethlin · 2023-06-03T19:04:05Z

compiler/rustc_mir_transform/src/inline.rs

@@ -424,6 +415,10 @@ impl<'tcx> Inliner<'tcx> {
        if callee_body.basic_blocks.len() <= 3 {
            threshold += threshold / 4;
        }
+
+        if tcx.sess.mir_opt_level() == 1 || tcx.sess.opts.incremental.is_some() {
+            threshold = 2 * INSTR_COST;


This is twice INSTR_COST right now because a return terminator gets INSTR_COST, so if the threshold were just INSTR_COST that would only inline functions that are just a terminator.

Would be good to have that as a comment in the code

bors · 2023-06-03T20:29:15Z

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

rust-timer · 2023-06-03T21:45:07Z

Finished benchmarking commit (fc943c1fd851b1eec39f3a13184baec258b4afc7): comparison URL.

Overall result: ❌✅ regressions and improvements - 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.

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-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)	6.6%	[0.3%, 52.7%]	83
Regressions ❌ (secondary)	1.5%	[0.3%, 4.9%]	23
Improvements ✅ (primary)	-2.2%	[-5.1%, -0.2%]	13
Improvements ✅ (secondary)	-5.8%	[-42.9%, -1.3%]	14
All ❌✅ (primary)	5.4%	[-5.1%, 52.7%]	96

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)	5.1%	[0.2%, 13.0%]	53
Regressions ❌ (secondary)	2.2%	[2.0%, 2.5%]	2
Improvements ✅ (primary)	-3.9%	[-6.9%, -0.9%]	2
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	4.8%	[-6.9%, 13.0%]	55

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)	9.3%	[1.3%, 66.7%]	50
Regressions ❌ (secondary)	4.3%	[2.7%, 6.0%]	5
Improvements ✅ (primary)	-3.4%	[-5.6%, -1.8%]	8
Improvements ✅ (secondary)	-6.6%	[-45.2%, -1.8%]	12
All ❌✅ (primary)	7.5%	[-5.6%, 66.7%]	58

Binary size

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)	4.4%	[0.0%, 24.5%]	94
Regressions ❌ (secondary)	1.9%	[0.0%, 5.5%]	15
Improvements ✅ (primary)	-0.4%	[-0.7%, -0.1%]	25
Improvements ✅ (secondary)	-3.6%	[-7.0%, -0.0%]	22
All ❌✅ (primary)	3.4%	[-0.7%, 24.5%]	119

Bootstrap: 647.253s -> 647.473s (0.03%)

saethlin · 2023-06-04T02:05:47Z

@bors try @rust-timer queue

bors · 2023-06-04T02:05:56Z

⌛ Trying commit 103aefd5025589f9114d3a102b56bd548275870a with merge e02925ba6e5567725a48d1c936639d6ee24e93ba...

bors · 2023-06-04T04:04:48Z

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

rust-timer · 2023-06-04T06:11:51Z

Finished benchmarking commit (e02925ba6e5567725a48d1c936639d6ee24e93ba): comparison URL.

Overall result: ❌✅ regressions and improvements - 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.

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-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)	1.1%	[0.2%, 4.3%]	25
Regressions ❌ (secondary)	0.6%	[0.2%, 3.4%]	15
Improvements ✅ (primary)	-0.4%	[-2.1%, -0.2%]	32
Improvements ✅ (secondary)	-2.4%	[-43.1%, -0.2%]	38
All ❌✅ (primary)	0.2%	[-2.1%, 4.3%]	57

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)	3.5%	[1.1%, 6.4%]	4
Regressions ❌ (secondary)	4.4%	[4.4%, 4.4%]	1
Improvements ✅ (primary)	-2.9%	[-9.7%, -0.6%]	5
Improvements ✅ (secondary)	-2.6%	[-2.6%, -2.6%]	1
All ❌✅ (primary)	-0.1%	[-9.7%, 6.4%]	9

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)	2.2%	[0.9%, 4.4%]	10
Regressions ❌ (secondary)	5.4%	[4.6%, 6.3%]	2
Improvements ✅ (primary)	-1.8%	[-2.3%, -1.4%]	2
Improvements ✅ (secondary)	-7.1%	[-45.5%, -1.4%]	11
All ❌✅ (primary)	1.5%	[-2.3%, 4.4%]	12

Binary size

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.3%	[0.0%, 0.6%]	9
Regressions ❌ (secondary)	0.4%	[0.0%, 1.4%]	7
Improvements ✅ (primary)	-0.6%	[-11.4%, -0.0%]	59
Improvements ✅ (secondary)	-5.2%	[-7.0%, -0.0%]	13
All ❌✅ (primary)	-0.5%	[-11.4%, 0.6%]	68

Bootstrap: 647.755s -> 652.907s (0.80%)

saethlin · 2023-06-04T15:53:46Z

@bors try @rust-timer queue

bors · 2023-10-14T06:51:55Z

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

rust-timer · 2023-10-14T08:13:28Z

Finished benchmarking commit (a53f4ae): comparison URL.

Overall result: ❌✅ regressions and improvements - 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.

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-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)	3.0%	[0.9%, 7.1%]	6
Regressions ❌ (secondary)	1.9%	[0.2%, 5.1%]	3
Improvements ✅ (primary)	-0.3%	[-0.3%, -0.2%]	5
Improvements ✅ (secondary)	-6.5%	[-40.5%, -0.9%]	11
All ❌✅ (primary)	1.5%	[-0.3%, 7.1%]	11

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)	5.3%	[1.0%, 22.9%]	9
Regressions ❌ (secondary)	5.7%	[5.7%, 5.7%]	1
Improvements ✅ (primary)	-1.4%	[-2.8%, -0.4%]	4
Improvements ✅ (secondary)	-3.4%	[-4.9%, -2.0%]	2
All ❌✅ (primary)	3.2%	[-2.8%, 22.9%]	13

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)	3.7%	[1.6%, 6.3%]	6
Regressions ❌ (secondary)	5.9%	[5.9%, 5.9%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-6.9%	[-42.7%, -1.3%]	10
All ❌✅ (primary)	3.7%	[1.6%, 6.3%]	6

Binary size

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.2%	[0.0%, 0.9%]	28
Regressions ❌ (secondary)	0.5%	[0.0%, 1.2%]	4
Improvements ✅ (primary)	-0.3%	[-1.0%, -0.1%]	21
Improvements ✅ (secondary)	-5.5%	[-7.3%, -0.1%]	13
All ❌✅ (primary)	-0.0%	[-1.0%, 0.9%]	49

Bootstrap: 627.414s -> 629.869s (0.39%)
Artifact size: 271.28 MiB -> 271.25 MiB (-0.01%)

saethlin · 2023-10-14T18:08:02Z

@bors try @rust-timer queue

bors · 2023-10-14T18:09:11Z

⌛ Trying commit 55eefde with merge f122ffa...

…ions, r=<try> Enable MIR inlining of very small functions in debug builds This interacts with rust-lang#116505 r? `@ghost`

bors · 2023-10-14T19:21:45Z

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

rust-timer · 2023-10-15T02:35:41Z

Finished benchmarking commit (f122ffa): comparison URL.

Overall result: ❌✅ regressions and improvements - 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.

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-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)	3.0%	[0.8%, 6.9%]	6
Regressions ❌ (secondary)	1.9%	[0.3%, 4.9%]	3
Improvements ✅ (primary)	-0.3%	[-0.6%, -0.3%]	7
Improvements ✅ (secondary)	-6.0%	[-40.3%, -0.2%]	12
All ❌✅ (primary)	1.2%	[-0.6%, 6.9%]	13

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)	6.9%	[1.6%, 19.6%]	6
Regressions ❌ (secondary)	6.0%	[6.0%, 6.0%]	1
Improvements ✅ (primary)	-1.0%	[-1.4%, -0.4%]	3
Improvements ✅ (secondary)	-3.8%	[-5.1%, -2.6%]	3
All ❌✅ (primary)	4.3%	[-1.4%, 19.6%]	9

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)	3.5%	[1.5%, 5.7%]	5
Regressions ❌ (secondary)	5.1%	[5.1%, 5.1%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-6.7%	[-42.6%, -1.4%]	11
All ❌✅ (primary)	3.5%	[1.5%, 5.7%]	5

Binary size

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.2%	[0.0%, 1.0%]	26
Regressions ❌ (secondary)	0.5%	[0.0%, 1.2%]	4
Improvements ✅ (primary)	-0.3%	[-1.3%, -0.1%]	23
Improvements ✅ (secondary)	-5.5%	[-7.3%, -0.1%]	13
All ❌✅ (primary)	-0.0%	[-1.3%, 1.0%]	49

Bootstrap: 627.524s -> 628.153s (0.10%)
Artifact size: 271.31 MiB -> 271.21 MiB (-0.04%)

saethlin · 2023-10-18T04:08:26Z

@bors try @rust-timer queue

bors · 2023-10-18T04:09:34Z

⌛ Trying commit 2848a10 with merge 17db0d8...

…ions, r=<try> Enable MIR inlining of very small functions in debug builds This interacts with rust-lang#116505 r? `@ghost`

bors · 2023-10-18T05:30:32Z

☀️ Try build successful - checks-actions
Build commit: 17db0d8 (17db0d8223b6d22e216f0039bbf4f9dc5ac151e9)

rust-timer · 2023-10-18T06:52:12Z

Finished benchmarking commit (17db0d8): comparison URL.

Overall result: ❌✅ regressions and improvements - 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.

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-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)	2.7%	[0.9%, 8.0%]	9
Regressions ❌ (secondary)	1.9%	[0.3%, 5.0%]	3
Improvements ✅ (primary)	-0.4%	[-0.7%, -0.2%]	8
Improvements ✅ (secondary)	-4.4%	[-41.4%, -0.4%]	17
All ❌✅ (primary)	1.3%	[-0.7%, 8.0%]	17

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)	4.6%	[2.9%, 8.1%]	6
Regressions ❌ (secondary)	4.0%	[3.9%, 4.1%]	2
Improvements ✅ (primary)	-1.5%	[-2.5%, -0.4%]	3
Improvements ✅ (secondary)	-3.4%	[-5.6%, -1.3%]	7
All ❌✅ (primary)	2.6%	[-2.5%, 8.1%]	9

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)	3.4%	[1.5%, 8.3%]	7
Regressions ❌ (secondary)	5.4%	[5.4%, 5.4%]	1
Improvements ✅ (primary)	-1.3%	[-1.6%, -0.8%]	3
Improvements ✅ (secondary)	-7.3%	[-43.5%, -2.2%]	10
All ❌✅ (primary)	2.0%	[-1.6%, 8.3%]	10

Binary size

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.2%	[0.0%, 1.0%]	24
Regressions ❌ (secondary)	0.5%	[0.0%, 1.2%]	4
Improvements ✅ (primary)	-0.3%	[-0.7%, -0.1%]	23
Improvements ✅ (secondary)	-5.5%	[-7.3%, -0.1%]	13
All ❌✅ (primary)	-0.0%	[-0.7%, 1.0%]	47

Bootstrap: 631.742s -> 631.351s (-0.06%)
Artifact size: 304.01 MiB -> 303.99 MiB (-0.01%)

saethlin · 2024-02-14T21:04:09Z

I am leaving the same comment here and on #116898.

I'm closing this because I think this has unsolvable (with our current tooling) compile time performance issues.

I think the performance regressions are driven by increasing the connectivity of the incremental compilation dependency graph. The primary effect of MIR optimizations is to decrease compile times by making less work for LLVM, but if we make builds less incremental in order to produce less IR overall, this is working against itself to improve debug build times.

This tension between MIR optimizations enabled by inlining and incremental build granularity may be solvable by computing the MIR call graph ahead of time and consulting the graph in the MIR inliner to find inlining opportunities that do not significantly increase the connectivity of the graph. But implementing that is beyond me.

saethlin added A-mir-opt Area: MIR optimizations A-mir-opt-inlining Area: MIR inlining S-experimental Status: Ongoing experiment that does not require reviewing and won't be merged in its current state. labels Jun 3, 2023

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 3, 2023

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jun 3, 2023

saethlin commented Jun 3, 2023

View reviewed changes

This comment has been minimized.

Sign in to view

rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Jun 3, 2023

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jun 4, 2023

This comment has been minimized.

Sign in to view

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jun 4, 2023

This comment has been minimized.

Sign in to view

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Oct 14, 2023

saethlin changed the title ~~Always consider inlining for trivial functions~~ Enable MIR inlining of very small functions in debug builds Oct 14, 2023

saethlin force-pushed the always-inline-trivial-functions branch from 8841021 to 55eefde Compare October 14, 2023 17:32

This comment has been minimized.

Sign in to view

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

This comment has been minimized.

Sign in to view

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Oct 15, 2023

Enable MIR inlining of very small functions in debug builds

2848a10

saethlin force-pushed the always-inline-trivial-functions branch from 55eefde to 2848a10 Compare October 18, 2023 04:07

This comment has been minimized.

Sign in to view

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

This comment has been minimized.

Sign in to view

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

saethlin closed this Feb 14, 2024

saethlin mentioned this pull request Feb 14, 2024

Automatic #[inline] for trivial wrappers #116898

Closed

saethlin deleted the always-inline-trivial-functions branch February 14, 2024 21:04

Enable MIR inlining of very small functions in debug builds #112255

Enable MIR inlining of very small functions in debug builds #112255

Conversation

saethlin commented Jun 3, 2023 • edited Loading

saethlin commented Jun 3, 2023

This comment has been minimized.

bors commented Jun 3, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

bors commented Jun 3, 2023

This comment has been minimized.

rust-timer commented Jun 3, 2023

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

saethlin commented Jun 4, 2023

This comment has been minimized.

bors commented Jun 4, 2023

This comment has been minimized.

bors commented Jun 4, 2023

This comment has been minimized.

rust-timer commented Jun 4, 2023

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

This comment has been minimized.

saethlin commented Jun 4, 2023

This comment has been minimized.

bors commented Oct 14, 2023

This comment has been minimized.

rust-timer commented Oct 14, 2023

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

saethlin commented Oct 14, 2023

This comment has been minimized.

bors commented Oct 14, 2023

bors commented Oct 14, 2023

This comment has been minimized.

rust-timer commented Oct 15, 2023

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

saethlin commented Oct 18, 2023

This comment has been minimized.

bors commented Oct 18, 2023

bors commented Oct 18, 2023

This comment has been minimized.

rust-timer commented Oct 18, 2023

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

saethlin commented Feb 14, 2024

saethlin commented Jun 3, 2023 •

edited

Loading