improve cgu schedule with greedy partition

[csmoe]

The difference between merged cgu should be more smaller than the old code which merges the small cgus into less-small cgu.

r? @bjorn3 may I have a perf-run? thanks :)

[matthiaskrgr]

@bors try @rust-timer queue

[bors]

⌛ Trying commit c2ea570 with merge 4b3f9c2e29e1cc79fdc77597dbbe2cf8db1c5bfa...

[bors]

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

[rust-timer]

Finished benchmarking commit (4b3f9c2e29e1cc79fdc77597dbbe2cf8db1c5bfa): 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.8%	[0.3%, 13.5%]	25
Regressions ❌ (secondary)	9.6%	[0.3%, 98.6%]	39
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.7%	[-0.9%, -0.5%]	8
All ❌✅ (primary)	2.8%	[0.3%, 13.5%]	25

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.7%	[2.0%, 12.2%]	13
Regressions ❌ (secondary)	6.8%	[2.2%, 20.5%]	22
Improvements ✅ (primary)	-6.1%	[-7.7%, -4.6%]	2
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	3.2%	[-7.7%, 12.2%]	15

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)	5.2%	[1.2%, 19.3%]	18
Regressions ❌ (secondary)	15.8%	[1.5%, 99.9%]	25
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	5.2%	[1.2%, 19.3%]	18

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)	1.4%	[0.1%, 15.6%]	41
Regressions ❌ (secondary)	12.5%	[0.1%, 110.8%]	20
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	1.4%	[0.1%, 15.6%]	41

Bootstrap: 656.258s -> 658.359s (0.32%)

[lqd]

cc @nnethercote as I believe they have tried a greedy partitioning recently.

It also seems this PR removes the minimum CGU size property added in #112448, and part of the regressions may be coming from losing what that PR gained.

[nnethercote]

I have been working on improving CGU handling for several weeks. It's a difficult problem and many things that you might expect to improve performance do not. In particular, the interplay between walltime (speed), max-rss (memory use), and binary size is challenging -- it's hard to improve one or more of these across multiple benchmarks without badly regressing others.

Some PRs that may be of interest:

• Reduce the default number of CGUs for incremental. #111666, Improve the CGU merging algorithm #111712, Improve CGU creation algorithms #111900, Tweak CGU order. #111937 were all unsuccessful attempts.
• Introduce a minimum CGU size in non-incremental builds. #112448 was a success.
• More aggressive CGU merging #112648 and Inline before merging cgus #112695 are ongoing. Unclear if they will be a success.

There is a function debug_dump in compiler/rustc_monomorphize/src/partitioning.rs that is very useful. Try changing the debug! statement to eprintln! and inspect the output.

If you want to continue working on this, we should coordinate, perhaps on Zulip.

[csmoe]

Thanks for your reviews and I did read the previous great PRs. If I understood @nnethercote‘s greedy approach right, they tried to merge as much as possible.

While I’m implementing one of the algorithm for multi-way number partition problems to ease the size diff between cgu, unfortunately the perf said llvm opt pass was unexpectedly slower 🥲

And I wasn’t intended to remove the MIN const, because the partition count must be known from the beginning in my approach, while MIN applied to the loop process “dynamically”, so I just commented it out for a experiment.

[nnethercote]

#111712 implemented a greedy approach. It gave much better results than the current algorithm in terms of making the CGU sizes more similar. But it resulted in worse compile-time speed. One reason for this is that the size estimates are not very good.

So if you have, for example, 10 CGUs of equal estimated size but one of those estimates is too small by 30%, then you'll end up with one CGU that takes 30% longer than the other to compile -- that's the "long pole" that's holding everything up. In contrast, a worse merge might leave you with 10 CGUs of varying size, which leaves more room to accommodate inaccuracies. E.g. most of the smaller CGUs can have their size underestimated without it hurting much.

There are a lot of surprising behaviours here. I recommend using a profiler like the self-profiler or samply that can show thread execution, because seeing how long the different codegen threads take is very helpful. For example, this screenshot is from samply:

[rust-log-analyzer]

The job x86_64-gnu-llvm-14 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

failures:

---- [codegen-units] tests/codegen-units/partitioning/incremental-merging.rs stdout ----

The following items were assigned to wrong codegen units:

fn aaa::foo
  expected: incremental_merging-aaa--incremental_merging-bbb[External] 
  actual:   incremental_merging-aaa--incremental_merging-bbb--incremental_merging-ccc--incremental_merging-ddd[External] 

fn bbb::foo
  expected: incremental_merging-aaa--incremental_merging-bbb[External] 
  actual:   incremental_merging-aaa--incremental_merging-bbb--incremental_merging-ccc--incremental_merging-ddd[External] 
Build completed unsuccessfully in 0:13:27
Build completed unsuccessfully in 0:13:27
fn ccc::foo
  expected: incremental_merging-ccc[External] 
  actual:   incremental_merging-aaa--incremental_merging-bbb--incremental_merging-ccc--incremental_merging-ddd[External] 
fn ddd::foo
fn ddd::foo
  expected: incremental_merging-ddd[External] 
  actual:   incremental_merging-aaa--incremental_merging-bbb--incremental_merging-ccc--incremental_merging-ddd[External] 
thread '[codegen-units] tests/codegen-units/partitioning/incremental-merging.rs' panicked at 'explicit panic', src/tools/compiletest/src/runtest.rs:2906:13
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

[bors]

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

[csmoe]

benchmarked locally, it regressed.