[BOLT] Use CDSort and CDSplit

[aaupov]

CDSort and CDSplit are the most recent versions of function ordering and function splitting algorithms with some improvements over the previous baseline (ext-tsp and two-way splitting).

[rustbot]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Mark-Simulacrum (or someone else) soon.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

• @rustbot author: the review is finished, PR author should check the comments and take action accordingly
• @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

[aaupov]

CC @Kobzol

[Kobzol]

Hi! I already tried cdsort, it provided no performance benefit. I haven't tried it with .arg("-split-strategy=cdsplit") though.

I think that we'd need to bump our LLVM version for this, as it's 17.0.4 currently, and that doesn't know about cdsort, AFAIK.

[aaupov]

Hi! I already tried cdsort, it provided no performance benefit.

Interesting, do you have a link with perf stats? We observed some improvements on large binaries in icache and itlb misses.

I haven't tried it with .arg("-split-strategy=cdsplit") though.

CDSplit is separate from cdsort and could be beneficial with hfsort+.

I think that we'd need to bump our LLVM version for this, as it's 17.0.4 currently, and that doesn't know about cdsort, AFAIK.

Sorry about not checking the LLVM version used. CDSplit is also very recent and was only upstreamed a month ago. If you don't use trunk BOLT, it will probably be in the next release.

[Kobzol]

I think that we have talked about this on Discord 😅

The previous attempt was this commit, the results are here.

There were a few cycles wins, nothing too big though. We could switch to cdsort once we bump LLVM to 18.

[Kobzol]

Looking at the results again, there were some nice max RSS improvements, and bootstrap time was also improved by 5s, which isn't trivial. Maybe I was too harsh to cdsort, it looks like an actual improvement.

[Kobzol]

I would suggest to first try only .arg("-split-strategy=cdsplit") in this PR, I can start a perf. run for that (assuming that we don't need LLVM 18 for it).

[aaupov]

I would suggest to first try only .arg("-split-strategy=cdsplit") in this PR, I can start a perf. run for that (assuming that we don't need LLVM 18 for it).

We do need trunk (or LLVM 18 rc) BOLT for CDSplit.

[Kobzol]

I would suggest waiting for LLVM 18 RC, and then we can try both options.

[Mark-Simulacrum]

r? @Kobzol

[Kobzol]

LLVM 18-rc1 has been tagged, so perhaps we can try it. The change needs to be made here.

[mati865]

I think it'd also require PassWrapper changes from nikic@be8c330

[Kobzol]

@aaupov We now have LLVM 18 as the host compiler on CI. I tried the new CDSort/CDSpltit flags here and they provide nice max RSS reductions and also some cycle wins.

Could you please rebase so that I can start another perf. run?

[Kobzol]

@bors try @rust-timer queue

[bors]

⌛ Trying commit 13ef6d4 with merge 4bd76ed...

[bors]

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

[rust-timer]

Finished benchmarking commit (4bd76ed): 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
Improvements ✅ (secondary)	-0.1%	[-0.1%, -0.1%]	1
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
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-3.0%	[-7.2%, -1.2%]	55
Improvements ✅ (secondary)	-3.3%	[-7.8%, -0.5%]	114
All ❌✅ (primary)	-3.0%	[-7.2%, -1.2%]	55

Cycles

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

Binary size

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

Bootstrap: 670.527s -> 667.725s (-0.42%)
Artifact size: 311.59 MiB -> 312.70 MiB (0.36%)

[Kobzol]

A slight LLVM/rustc artifact size increase, but also very nice Max-RSS improvements. Thank you! Let us know if you have any other useful BOLT flags up your sleeve :)

@bors r+

[bors]

📌 Commit 13ef6d4 has been approved by Kobzol

It is now in the queue for this repository.

[bors]

⌛ Testing commit 13ef6d4 with merge 9023f90...

[bors]

☀️ Test successful - checks-actions
Approved by: Kobzol
Pushing 9023f90 to master...

[rust-timer]

Finished benchmarking commit (9023f90): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

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

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
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-2.5%	[-4.6%, -0.8%]	37
Improvements ✅ (secondary)	-3.9%	[-8.1%, -0.5%]	39
All ❌✅ (primary)	-2.5%	[-4.6%, -0.8%]	37

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)	-	-	0
Improvements ✅ (primary)	-0.8%	[-0.8%, -0.8%]	1
Improvements ✅ (secondary)	-0.9%	[-0.9%, -0.9%]	1
All ❌✅ (primary)	-0.8%	[-0.8%, -0.8%]	1

Binary size

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

Bootstrap: 671.684s -> 667.951s (-0.56%)
Artifact size: 311.57 MiB -> 312.73 MiB (0.37%)