Avoid MIR bloat in inlining #127113

scottmcm · 2024-06-29T07:48:55Z

In #126578 we ended up with more binary size increases than expected.

This change attempts to avoid inlining large things into small things, to avoid that kind of increase, in cases when top-down inlining will still be able to do that inlining later.

r? ghost

scottmcm · 2024-06-29T08:08:06Z

@bors try @rust-timer queue

Avoid MIR bloat in inlining In rust-lang#126578 we ended up with more binary size increases than expected. This change attempts to avoid inlining large things into small things, to avoid that kind of increase, in cases when top-down inlining will still be able to do that inlining later. r? ghost

bors · 2024-06-29T08:09:17Z

⌛ Trying commit ba09e7c with merge 53ba00a...

bors · 2024-06-29T09:53:48Z

☀️ Try build successful - checks-actions
Build commit: 53ba00a (53ba00a12225ff09578421cdb59a0fb925d84474)

rust-timer · 2024-06-29T14:28:55Z

Finished benchmarking commit (53ba00a): 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)	0.9%	[0.2%, 3.1%]	9
Regressions ❌ (secondary)	1.2%	[0.5%, 1.5%]	5
Improvements ✅ (primary)	-0.8%	[-2.1%, -0.3%]	14
Improvements ✅ (secondary)	-1.6%	[-4.6%, -0.3%]	18
All ❌✅ (primary)	-0.1%	[-2.1%, 3.1%]	23

Max RSS (memory usage)

Results (primary 1.6%, secondary -5.9%)

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%	[2.6%, 5.3%]	3
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-4.9%	[-4.9%, -4.9%]	1
Improvements ✅ (secondary)	-5.9%	[-5.9%, -5.9%]	1
All ❌✅ (primary)	1.6%	[-4.9%, 5.3%]	4

Cycles

Results (primary -0.1%, secondary -0.1%)

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%	[1.3%, 3.0%]	2
Regressions ❌ (secondary)	2.6%	[2.0%, 3.5%]	7
Improvements ✅ (primary)	-1.6%	[-2.2%, -0.6%]	3
Improvements ✅ (secondary)	-2.9%	[-4.8%, -1.7%]	7
All ❌✅ (primary)	-0.1%	[-2.2%, 3.0%]	5

Binary size

Results (primary -0.1%, secondary -0.6%)

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.4%	[0.0%, 2.9%]	38
Regressions ❌ (secondary)	1.6%	[0.0%, 6.7%]	5
Improvements ✅ (primary)	-0.5%	[-3.3%, -0.1%]	56
Improvements ✅ (secondary)	-0.8%	[-5.9%, -0.1%]	59
All ❌✅ (primary)	-0.1%	[-3.3%, 2.9%]	94

Bootstrap: 694.857s -> 695.619s (0.11%)
Artifact size: 324.45 MiB -> 327.23 MiB (0.86%)

scottmcm · 2024-06-30T00:48:51Z

@bors try @rust-timer queue

Avoid MIR bloat in inlining In rust-lang#126578 we ended up with more binary size increases than expected. This change attempts to avoid inlining large things into small things, to avoid that kind of increase, in cases when top-down inlining will still be able to do that inlining later. r? ghost

bors · 2024-06-30T00:50:03Z

⌛ Trying commit 00c4b72 with merge 4626152...

bors · 2024-06-30T02:32:05Z

☀️ Try build successful - checks-actions
Build commit: 4626152 (462615258ba2e28b85645ad27fc9163232532659)

rust-timer · 2024-06-30T04:51:31Z

Finished benchmarking commit (4626152): 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)	0.8%	[0.2%, 2.9%]	9
Regressions ❌ (secondary)	0.9%	[0.3%, 1.3%]	7
Improvements ✅ (primary)	-0.9%	[-2.1%, -0.2%]	12
Improvements ✅ (secondary)	-1.5%	[-4.9%, -0.3%]	23
All ❌✅ (primary)	-0.2%	[-2.1%, 2.9%]	21

Max RSS (memory usage)

Results (primary 1.7%, secondary -6.2%)

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%	[2.0%, 6.5%]	5
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-6.9%	[-6.9%, -6.9%]	1
Improvements ✅ (secondary)	-6.2%	[-6.4%, -6.1%]	2
All ❌✅ (primary)	1.7%	[-6.9%, 6.5%]	6

Cycles

Results (primary 0.2%, secondary -1.1%)

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.1%	[1.2%, 2.9%]	2
Regressions ❌ (secondary)	2.8%	[2.4%, 3.1%]	2
Improvements ✅ (primary)	-1.7%	[-1.8%, -1.5%]	2
Improvements ✅ (secondary)	-3.0%	[-4.3%, -1.4%]	4
All ❌✅ (primary)	0.2%	[-1.8%, 2.9%]	4

Binary size

Results (primary -0.1%, secondary -0.6%)

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.4%	[0.0%, 2.9%]	36
Regressions ❌ (secondary)	1.6%	[0.0%, 6.6%]	5
Improvements ✅ (primary)	-0.4%	[-3.3%, -0.1%]	67
Improvements ✅ (secondary)	-0.8%	[-5.9%, -0.1%]	59
All ❌✅ (primary)	-0.1%	[-3.3%, 2.9%]	103

Bootstrap: 695.844s -> 695.214s (-0.09%)
Artifact size: 324.63 MiB -> 327.43 MiB (0.86%)

scottmcm · 2024-06-30T04:58:30Z

Well, I was hoping for a bit more, but this is neutral-to-green on instructions and claws back some of the size losses from #126578 -- for example, image-opt-full was +0.95% in that and is -1.38% here.

r? mir-opt

oli-obk · 2024-07-01T10:33:52Z

compiler/rustc_mir_transform/src/inline.rs

@@ -1091,3 +1103,37 @@ fn try_instance_mir<'tcx>(
    }
    Ok(tcx.instance_mir(instance))
 }
+
+fn body_is_forwarder(body: &Body<'_>) -> bool {


may be useful to look into methods that only forward to other methods, but may trigger autoderef through Deref impls or other things that can cause this method to return false

Yeah, a bigger version of the problem here is that we just loop through the blocks in order, inlining stuff.

If I find time I want to try some bigger changes like a p-queue of Calls so I could do things like apply the trivial things first, then apply a check like this to decide whether to continue.

…i-obk Avoid MIR bloat in inlining In rust-lang#126578 we ended up with more binary size increases than expected. This change attempts to avoid inlining large things into small things, to avoid that kind of increase, in cases when top-down inlining will still be able to do that inlining later. r? ghost

bors · 2024-07-01T11:10:13Z

⌛ Testing commit 00c4b72 with merge 40aef21...

bors · 2024-07-01T11:51:14Z

💔 Test failed - checks-actions

In 126578 we ended up with more binary size increases than expected. This change attempts to avoid inlining large things into small things, to avoid that kind of increase, in cases when top-down inlining will still be able to do that inlining later.

scottmcm · 2024-07-01T12:18:09Z

@bors try

bors · 2024-07-01T12:19:19Z

⌛ Trying commit 23c8ed1 with merge 340acfa...

Avoid MIR bloat in inlining try-job: test-various In rust-lang#126578 we ended up with more binary size increases than expected. This change attempts to avoid inlining large things into small things, to avoid that kind of increase, in cases when top-down inlining will still be able to do that inlining later. r? ghost

bors · 2024-07-01T13:17:33Z

☀️ Try build successful - checks-actions
Build commit: 340acfa (340acfa21e53ed4707b2869fb7788aa8b19514d5)

scottmcm · 2024-07-01T13:41:29Z

Looks like this tweak made something a CGU test was looking for inline away, so I disabled MIR inlining in that test and the try build said that fixed it, so
@bors r=oli-obk

bors · 2024-07-01T13:41:31Z

📌 Commit 23c8ed1 has been approved by oli-obk

It is now in the queue for this repository.

bors · 2024-07-01T14:09:42Z

⌛ Testing commit 23c8ed1 with merge 221e274...

bors · 2024-07-01T16:20:48Z

☀️ Test successful - checks-actions
Approved by: oli-obk
Pushing 221e274 to master...

rust-timer · 2024-07-01T17:37:13Z

Finished benchmarking commit (221e274): comparison URL.

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

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.3%, 2.8%]	6
Regressions ❌ (secondary)	1.6%	[1.5%, 1.9%]	6
Improvements ✅ (primary)	-0.8%	[-2.2%, -0.2%]	17
Improvements ✅ (secondary)	-1.6%	[-4.5%, -0.2%]	18
All ❌✅ (primary)	-0.3%	[-2.2%, 2.8%]	23

Max RSS (memory usage)

Results (primary -0.5%, secondary -6.4%)

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.7%	[0.6%, 5.4%]	6
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-6.7%	[-8.7%, -5.2%]	3
Improvements ✅ (secondary)	-6.4%	[-6.4%, -6.4%]	1
All ❌✅ (primary)	-0.5%	[-8.7%, 5.4%]	9

Cycles

Results (primary -0.8%, secondary -3.0%)

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.6%	[2.6%, 2.6%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-1.6%	[-2.0%, -1.0%]	4
Improvements ✅ (secondary)	-3.0%	[-4.2%, -1.5%]	5
All ❌✅ (primary)	-0.8%	[-2.0%, 2.6%]	5

Binary size

Results (primary -0.2%, secondary -0.6%)

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.4%	[0.0%, 2.9%]	36
Regressions ❌ (secondary)	1.6%	[0.0%, 6.6%]	5
Improvements ✅ (primary)	-0.5%	[-3.3%, -0.1%]	71
Improvements ✅ (secondary)	-0.8%	[-5.9%, -0.1%]	59
All ❌✅ (primary)	-0.2%	[-3.3%, 2.9%]	107

Bootstrap: 697.337s -> 695.345s (-0.29%)
Artifact size: 326.67 MiB -> 327.55 MiB (0.27%)

pnkfelix · 2024-07-03T03:08:45Z

Visiting for weekly rustc-perf triage

regressed opt-full html5ever, diesel, hyper, and clap. Also regressed ripgrep and regex in two isolated opt incremental scenarios.
overall gains more than it loses, as noted after the perf run done during PR development.
the biggest (and notably positive) impact was to binary sizes, where the improvement is pretty clear.
marking as triaged.

@rustbot label: +perf-regression-triaged

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 29, 2024

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 29, 2024

scottmcm mentioned this pull request Jun 29, 2024

Account for things that optimize out in inlining costs #126578

Merged

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 29, 2024

scottmcm force-pushed the retune-inlining-again branch from ba09e7c to 29e8c80 Compare June 29, 2024 18:53

This comment has been minimized.

Sign in to view

scottmcm force-pushed the retune-inlining-again branch from 29e8c80 to 3f104b1 Compare June 29, 2024 20:18

This comment has been minimized.

Sign in to view

scottmcm force-pushed the retune-inlining-again branch from 3f104b1 to 0d36257 Compare June 29, 2024 22:00

This comment has been minimized.

Sign in to view

scottmcm force-pushed the retune-inlining-again branch from 0d36257 to 00c4b72 Compare June 30, 2024 00:27

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 30, 2024

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 30, 2024

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 1, 2024

oli-obk reviewed Jul 1, 2024

View reviewed changes

This comment has been minimized.

Sign in to view

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 1, 2024

Avoid MIR bloat in inlining

23c8ed1

In 126578 we ended up with more binary size increases than expected. This change attempts to avoid inlining large things into small things, to avoid that kind of increase, in cases when top-down inlining will still be able to do that inlining later.

scottmcm force-pushed the retune-inlining-again branch from 00c4b72 to 23c8ed1 Compare July 1, 2024 12:17

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 1, 2024

bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 1, 2024

bors merged commit 221e274 into rust-lang:master Jul 1, 2024
7 checks passed

rustbot added this to the 1.81.0 milestone Jul 1, 2024

bors mentioned this pull request Jul 1, 2024

Apply "polymorphization at home" to RawVec #126793

Merged

rustbot added the perf-regression-triaged The performance regression has been triaged. label Jul 3, 2024

DianQK mentioned this pull request Jul 14, 2024

.next().unwrap_unchecked() on Iterator doesn't optimize as expected #107681

Closed

scottmcm deleted the retune-inlining-again branch August 9, 2024 06:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Avoid MIR bloat in inlining #127113

Avoid MIR bloat in inlining #127113

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Avoid MIR bloat in inlining #127113

Avoid MIR bloat in inlining #127113

Conversation

This comment has been minimized.

This comment has been minimized.

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

Overall result: ❌✅ regressions and improvements - ACTION NEEDED