Use ICF (identical code folding) for building rustc #99062

Kobzol · 2022-07-08T18:06:29Z

It seems that ICF (identical code folding) is able to remove duplicated functions created by monomorphization from binaries, resulting in smaller binary size and better i-cache utilization. Let's see if it helps for rustc.

I'm not sure if lld is even used for linking rustc on the Linux dist builder, let's see.

rust-highfive · 2022-07-08T18:06:32Z

r? @jyn514

(rust-highfive has picked a reviewer for you, use r? to override)

Kobzol · 2022-07-08T18:06:49Z

@bors try @rust-timer queue

rust-timer · 2022-07-08T18:06:50Z

Awaiting bors try build completion.

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

bors · 2022-07-08T18:07:19Z

⌛ Trying commit 89b5a07d384be9096bdffb981ba7785c1871f2e4 with merge a7e16562cc91c83495165f476a643fba7f508872...

Kobzol · 2022-07-08T18:22:31Z

@bors try @rust-timer queue

rust-timer · 2022-07-08T18:22:33Z

Awaiting bors try build completion.

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

bors · 2022-07-08T18:22:40Z

⌛ Trying commit dc494a1a51a37a4d237d68d1c50c8d6faa0d535f with merge 144f21cd0f517feabe97a817811901326c170fe6...

jyn514 · 2022-07-08T18:43:22Z

I'm not sure if lld is even used for linking rustc on the Linux dist builder, let's see.

Looks like no, you'll have to change that too.

What version of llvm was this introduced in?

Kobzol · 2022-07-08T18:46:18Z

Looks like no, you'll have to change that too.

Indeed, I tried to do that now for the dist build, let's see what happens.

What version of llvm was this introduced in?

If you're talking about ICF, it's not a LLVM thing, it's a linker pass that seems to be supported by multiple linkers (gold, lld etc.). It works on the binary level, after everything has been already compiled. I investigated locally and it seems to improve binary size by a few percent (locally improved librustc_driver.so size by around 5%). I'm not sure why the duplicates are not already filtered out by the LLVM MergeFunctions pass though.

bors · 2022-07-08T20:50:15Z

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

rust-timer · 2022-07-08T20:50:17Z

Queued 144f21cd0f517feabe97a817811901326c170fe6 with parent 45263fc, future comparison URL.

rust-timer · 2022-07-09T07:11:29Z

Finished benchmarking commit (144f21cd0f517feabe97a817811901326c170fe6): comparison url.

Instruction count

Primary benchmarks: 😿 relevant regression found
Secondary benchmarks: mixed results

	mean¹	max	count²
Regressions 😿 (primary)	0.4%	0.4%	1
Regressions 😿 (secondary)	1.8%	2.6%	10
Improvements 🎉 (primary)	N/A	N/A	0
Improvements 🎉 (secondary)	-1.4%	-1.6%	5
All 😿🎉 (primary)	0.4%	0.4%	1

Max RSS (memory usage)

Results

Primary benchmarks: 🎉 relevant improvements found
Secondary benchmarks: mixed results

	mean¹	max	count²
Regressions 😿 (primary)	N/A	N/A	0
Regressions 😿 (secondary)	1.6%	2.6%	13
Improvements 🎉 (primary)	-2.1%	-3.3%	63
Improvements 🎉 (secondary)	-2.4%	-3.0%	24
All 😿🎉 (primary)	-2.1%	-3.3%	63

Cycles

Results

Primary benchmarks: 🎉 relevant improvements found
Secondary benchmarks: 🎉 relevant improvements found

	mean¹	max	count²
Regressions 😿 (primary)	N/A	N/A	0
Regressions 😿 (secondary)	2.0%	2.0%	1
Improvements 🎉 (primary)	-2.7%	-7.6%	25
Improvements 🎉 (secondary)	-3.3%	-5.4%	41
All 😿🎉 (primary)	-2.7%	-7.6%	25

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.
Warning ⚠: The following benchmark(s) failed to build:

rustc

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 ↩ ↩² ↩³

Kobzol · 2022-07-09T08:53:19Z

Cycles look really promising. I'll now try to do ld/lld/lld + ICF builds to find out how it affects rustc binary sizes and if lld by itself has any effect on performance.

@bors try @rust-timer queue

rust-timer · 2022-07-09T08:53:20Z

Awaiting bors try build completion.

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

bors · 2022-07-09T08:53:32Z

⌛ Trying commit 29e5413535ccb0ec42c8be02c0c49afcfa198a8c with merge 986e50fc1e83d611bf485cf02723d7bd89a093b8...

bors · 2022-07-09T10:43:54Z

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

rust-timer · 2022-07-09T10:43:56Z

Queued 986e50fc1e83d611bf485cf02723d7bd89a093b8 with parent 86b8dd5, future comparison URL.

jyn514 · 2022-07-17T16:19:31Z

src/ci/docker/host-x86_64/dist-x86_64-linux/Dockerfile

@@ -122,7 +122,8 @@ ENV RUST_CONFIGURE_ARGS \
      --set target.x86_64-unknown-linux-gnu.ranlib=/rustroot/bin/llvm-ranlib \
      --set llvm.thin-lto=true \
      --set llvm.ninja=false \
-      --set rust.jemalloc
+      --set rust.jemalloc \
+      --set rust.use-lld=true


This only affects Linux; is that intentional? Or should we use lld on other Unix platforms too?

Indeed I wanted to only try it first for Linux x64, as the most common toolchain. I usually test CI compilation optimization flags and similar things for it first, since it tends to have the best support.

I'm guessing this option wouldn't work on windows anyway as we don't use a single section per function there because COFF has a 2^16 - something limit on the section count and it is rather easy to hit this limit if every function has it's own section.

jyn514 · 2022-07-17T16:21:40Z

@Kobzol btw, please use @rustbot ready when you're done responding to comments so we know this is ready for review; otherwise we have to check GitHub notifications and we both get a lot of notifications.

(I made this hard for you just now by commenting at different times, sorry - will try and only leave "review" comments in the future so they're all posted at the same time)

Kobzol · 2022-07-17T16:23:01Z

Ok, I'll try to remember! 😅 I'm used to using GH notifications heavily, but I understand that for you it's quite impractical. I'll try to use rustbot more.

jyn514 · 2022-07-17T16:31:15Z

@bors r+

This seems like a good improvement; @Kobzol can you open an issue to follow up with using ICF for the standard library and platforms other than Linux?

bors · 2022-07-17T16:31:17Z

📌 Commit 97f6f95 has been approved by jyn514

It is now in the queue for this repository.

Kobzol · 2022-07-17T17:00:51Z

@jyn514 I created #99381. I would like to wait first to see if ICF doesn't break something e.g. for nightly users though.

Btw thanks @jonhoo, I got the idea to try ICF while reading Rust for Rustaceans :)

Kobzol · 2022-07-17T17:05:52Z

@jyn514 Since this is perf-sensitive and if there are some issues with ICF, we might want to revert it, maybe it would be better to avoid rollup on this one?

Mark-Simulacrum · 2022-07-17T17:08:48Z

@bors rollup=never

But I suspect this is already true; perf should set it after each run.

Kobzol · 2022-07-17T17:13:16Z

Sorry, forgot about that.

bors · 2022-07-17T21:42:56Z

⌛ Testing commit 97f6f95 with merge 246f66a...

bors · 2022-07-18T00:26:38Z

☀️ Test successful - checks-actions
Approved by: jyn514
Pushing 246f66a to master...

rust-timer · 2022-07-18T01:41:57Z

Finished benchmarking commit (246f66a): comparison url.

Instruction count

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

	mean¹	max	count²
Regressions 😿 (primary)	0.3%	0.4%	2
Regressions 😿 (secondary)	2.6%	2.6%	3
Improvements 🎉 (primary)	N/A	N/A	0
Improvements 🎉 (secondary)	N/A	N/A	0
All 😿🎉 (primary)	0.3%	0.4%	2

Max RSS (memory usage)

Results

Primary benchmarks: 🎉 relevant improvements found
Secondary benchmarks: mixed results

	mean¹	max	count²
Regressions 😿 (primary)	2.1%	2.1%	1
Regressions 😿 (secondary)	2.3%	4.2%	9
Improvements 🎉 (primary)	-1.8%	-3.0%	18
Improvements 🎉 (secondary)	-2.5%	-2.9%	5
All 😿🎉 (primary)	-1.6%	-3.0%	19

Cycles

Results

Primary benchmarks: 🎉 relevant improvements found
Secondary benchmarks: 🎉 relevant improvements found

	mean¹	max	count²
Regressions 😿 (primary)	N/A	N/A	0
Regressions 😿 (secondary)	N/A	N/A	0
Improvements 🎉 (primary)	-2.6%	-6.0%	33
Improvements 🎉 (secondary)	-3.4%	-6.1%	50
All 😿🎉 (primary)	-2.6%	-6.0%	33

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

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

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

…imulacrum Revert "Use ICF (identical code folding) for building rustc" Reverts rust-lang#99062 Fixes: rust-lang#99440

rylev · 2022-07-19T16:20:53Z

@Kobzol the perf hit looks fairly small. Should we mark this as triaged?

Kobzol · 2022-07-19T16:22:32Z

I would mark it as triaged, since it was a nice win on cycles actually, but this PR got reverted immediately after landing (#99442) since there are some issues with lld. I wonder if reverting should somehow automatically tag the original PR so that it's obvious.

rylev · 2022-07-19T17:06:46Z

@rustbot label: +perf-regression-triaged

Make `[rust] use-lld=true` work on windows Before, it would fail with "error: ignoring unknown argument '-Wl,--icf=all'" This option was introduced in rust-lang#99062 (well, technically rust-lang#99680) See zulip thread: https://rust-lang.zulipchat.com/#narrow/stream/182449-t-compiler.2Fhelp/topic/rust-lld.3A.20error.3A.20ignoring.20unknown.20argument.20'-Wl.2C--icf.3Dall'

rust-highfive assigned jyn514 Jul 8, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 8, 2022

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

This comment has been minimized.

Sign in to view

This comment was marked as outdated.

Sign in to view

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 Jul 9, 2022

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

jyn514 reviewed Jul 17, 2022

View reviewed changes

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 17, 2022

Kobzol mentioned this pull request Jul 17, 2022

Use ICF linker optimization for other platforms than Linux #99381

Open

bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 18, 2022

bors merged commit 246f66a into rust-lang:master Jul 18, 2022

rustbot added this to the 1.64.0 milestone Jul 18, 2022

matthiaskrgr mentioned this pull request Jul 18, 2022

rustup linux rustc executable not executable (symbol lookup error) #99440

Closed

Kobzol mentioned this pull request Jul 18, 2022

Revert "Use ICF (identical code folding) for building rustc" #99442

Merged

bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 18, 2022

Auto merge of rust-lang#99442 - Kobzol:revert-99062-lld-icf, r=Mark-S…

475aec1

…imulacrum Revert "Use ICF (identical code folding) for building rustc" Reverts rust-lang#99062 Fixes: rust-lang#99440

Kobzol mentioned this pull request Jul 19, 2022

[do not merge] Use LLD ICF=safe for compiling rustc #99455

Closed

rustbot added the perf-regression-triaged The performance regression has been triaged. label Jul 19, 2022

khyperia mentioned this pull request Aug 12, 2022

Make [rust] use-lld=true work on windows #100464

Merged

Kobzol deleted the lld-icf branch October 25, 2022 21:26

Use ICF (identical code folding) for building rustc #99062

Use ICF (identical code folding) for building rustc #99062

Conversation

Kobzol commented Jul 8, 2022

rust-highfive commented Jul 8, 2022

Kobzol commented Jul 8, 2022

rust-timer commented Jul 8, 2022

bors commented Jul 8, 2022

This comment has been minimized.

This comment was marked as outdated.

Kobzol commented Jul 8, 2022

rust-timer commented Jul 8, 2022

bors commented Jul 8, 2022

This comment has been minimized.

This comment has been minimized.

jyn514 commented Jul 8, 2022

Kobzol commented Jul 8, 2022

bors commented Jul 8, 2022

rust-timer commented Jul 8, 2022

rust-timer commented Jul 9, 2022

Footnotes

Kobzol commented Jul 9, 2022

rust-timer commented Jul 9, 2022

bors commented Jul 9, 2022

bors commented Jul 9, 2022

rust-timer commented Jul 9, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bjorn3 Jul 17, 2022 • edited Loading

Choose a reason for hiding this comment

jyn514 commented Jul 17, 2022

Kobzol commented Jul 17, 2022

jyn514 commented Jul 17, 2022

bors commented Jul 17, 2022

Kobzol commented Jul 17, 2022 • edited Loading

Kobzol commented Jul 17, 2022

Mark-Simulacrum commented Jul 17, 2022

Kobzol commented Jul 17, 2022

bors commented Jul 17, 2022

bors commented Jul 18, 2022

rust-timer commented Jul 18, 2022

Footnotes

rylev commented Jul 19, 2022

Kobzol commented Jul 19, 2022

rylev commented Jul 19, 2022

bjorn3 Jul 17, 2022 •

edited

Loading

Kobzol commented Jul 17, 2022 •

edited

Loading