Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Build rustc with a single CGU on x64 Linux #107651

Merged
merged 1 commit into from
Oct 1, 2023

Conversation

Kobzol
Copy link
Contributor

@Kobzol Kobzol commented Feb 3, 2023

Follow-up attempt to #87650. I wonder if anything changed with the addition of LTO.

I also enabled a single CGU only for the actual build of the compiler on CI, so that we can better see the perf. effects on the bootstrap benchmark.

@rustbot
Copy link
Collaborator

rustbot commented Feb 3, 2023

r? @pietroalbini

(rustbot has picked a reviewer for you, use r? to override)

@Kobzol
Copy link
Contributor Author

Kobzol commented Feb 3, 2023

@bors try @rust-timer queue

@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Feb 3, 2023
@rust-timer

This comment has been minimized.

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Feb 3, 2023
@bors
Copy link
Contributor

bors commented Feb 3, 2023

⌛ Trying commit 96c912abfc4b44ce5f8fca252500880ffba28d3e with merge 2a203d5ea41b38173f8e09d32d989d0c26770e28...

@bors
Copy link
Contributor

bors commented Feb 4, 2023

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

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (2a203d5ea41b38173f8e09d32d989d0c26770e28): 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%] 68
Regressions ❌
(secondary)
4.7% [0.5%, 27.2%] 57
Improvements ✅
(primary)
-1.1% [-4.1%, -0.2%] 71
Improvements ✅
(secondary)
-1.7% [-7.6%, -0.5%] 121
All ❌✅ (primary) -0.1% [-4.1%, 3.1%] 139

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.1% [-6.4%, -0.5%] 86
Improvements ✅
(secondary)
-3.5% [-7.7%, -0.9%] 153
All ❌✅ (primary) -2.1% [-6.4%, -0.5%] 86

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.0% [1.4%, 2.9%] 3
Regressions ❌
(secondary)
7.2% [2.1%, 17.5%] 20
Improvements ✅
(primary)
-2.3% [-5.4%, -0.8%] 52
Improvements ✅
(secondary)
-3.1% [-4.7%, -1.1%] 77
All ❌✅ (primary) -2.1% [-5.4%, 2.9%] 55

@rustbot rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Feb 4, 2023
@Zoxc
Copy link
Contributor

Zoxc commented Feb 4, 2023

This seems to perform similarly to #107560, which make sense since that compiles most crates with 1 CGU. It's an overall improvement, but with some regressions. It does seem like there's some problem with ThinLTO + PGO for match-stress, but that is a stress test and not terribly important.

@Kobzol
Copy link
Contributor Author

Kobzol commented Feb 4, 2023

The CI time hit is also not that terrible, 8600s (I saw a recent master commit had 8000s), not sure what's the average recently (@Mark-Simulacrum ).

We can get that time down, too. LLVM rebuild time can be lowered, and we can also speedup rustc builds - e.g. we don't need to build with 1 CGU when we do LLVM PGO/BOLT, and I think that we can also skip LTO/1 CGU when building stage 1 (we'd need to modify bootstrap though).

@Zoxc
Copy link
Contributor

Zoxc commented Feb 23, 2023

Building with 1 CGU would be useful to make the --emit options better match the CI builds too.

@Zoxc
Copy link
Contributor

Zoxc commented Apr 7, 2023

I did some local benchmarks to test 16 CGUs (Before) vs. 1 CGU (After) with ThinLTO:

BenchmarkBeforeAfter
TimeTime%
🟣 clap:check:unchanged0.4631s0.4475s💚 -3.37%
🟣 hyper:check:unchanged0.1528s0.1472s💚 -3.65%
🟣 regex:check:unchanged0.3433s0.3370s💚 -1.83%
🟣 syn:check:unchanged0.6308s0.6065s💚 -3.84%
🟣 syntex_syntax:check:unchanged1.8294s1.7532s💚 -4.16%
Total3.4193s3.2914s💚 -3.74%
Summary1.0000s0.9663s💚 -3.37%
BenchmarkBeforeAfter
TimeTime%
🟣 clap:check1.6721s1.5579s💚 -6.83%
🟣 hyper:check0.2592s0.2418s💚 -6.69%
🟣 regex:check0.9330s0.8702s💚 -6.74%
🟣 syn:check1.4963s1.3939s💚 -6.85%
🟣 syntex_syntax:check5.8074s5.3920s💚 -7.15%
Total10.1681s9.4558s💚 -7.00%
Summary1.0000s0.9315s💚 -6.85%

@Kobzol
Copy link
Contributor Author

Kobzol commented May 17, 2023

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label May 17, 2023
@bors
Copy link
Contributor

bors commented May 17, 2023

⌛ Trying commit 96c912abfc4b44ce5f8fca252500880ffba28d3e with merge 0b5019f4e7c7c5e8190c0d4e9686840a7a3cfc00...

@bors
Copy link
Contributor

bors commented May 17, 2023

💔 Test failed - checks-actions

@rust-log-analyzer

This comment has been minimized.

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 17, 2023
@Kobzol
Copy link
Contributor Author

Kobzol commented May 25, 2023

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented Sep 5, 2023

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

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (146169d35b066f541a24c7489cd7ef4397300cf5): 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.7% [0.3%, 2.0%] 31
Regressions ❌
(secondary)
1.7% [0.6%, 2.4%] 16
Improvements ✅
(primary)
-1.3% [-4.1%, -0.3%] 96
Improvements ✅
(secondary)
-1.7% [-3.7%, -0.5%] 122
All ❌✅ (primary) -0.8% [-4.1%, 2.0%] 127

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.1% [-4.9%, -0.6%] 68
Improvements ✅
(secondary)
-3.1% [-7.6%, -0.5%] 136
All ❌✅ (primary) -2.1% [-4.9%, -0.6%] 68

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)
2.6% [2.5%, 2.6%] 2
Improvements ✅
(primary)
-2.5% [-7.1%, -0.7%] 72
Improvements ✅
(secondary)
-3.8% [-15.6%, -0.8%] 99
All ❌✅ (primary) -2.5% [-7.1%, -0.7%] 72

Binary size

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

Bootstrap: 629.118s -> 625.733s (-0.54%)
Artifact size: 316.16 MiB -> 271.45 MiB (-14.14%)

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Sep 5, 2023
@Kobzol Kobzol force-pushed the ci-single-cgu branch 2 times, most recently from 26b5d7f to a47a9eb Compare September 15, 2023 14:21
@Kobzol
Copy link
Contributor Author

Kobzol commented Sep 15, 2023

@bors try

Trying with LLVM 16.

@bors
Copy link
Contributor

bors commented Sep 15, 2023

⌛ Trying commit a47a9eb with merge 3cdcb31...

bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 15, 2023
[WIP] Build `rustc` with a single CGU on x64 Linux

Follow-up attempt to rust-lang#87650. I wonder if anything changed with the addition of LTO.

I also enabled a single CGU only for the actual build of the compiler on CI, so that we can better see the perf. effects on the bootstrap benchmark.
@rustbot rustbot added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Sep 15, 2023
@Kobzol
Copy link
Contributor Author

Kobzol commented Sep 15, 2023

@bors try

@bors
Copy link
Contributor

bors commented Sep 15, 2023

⌛ Trying commit dba68fa with merge a3b1fd2...

bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 15, 2023
[WIP] Build `rustc` with a single CGU on x64 Linux

Follow-up attempt to rust-lang#87650. I wonder if anything changed with the addition of LTO.

I also enabled a single CGU only for the actual build of the compiler on CI, so that we can better see the perf. effects on the bootstrap benchmark.
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented Sep 15, 2023

💔 Test failed - checks-actions

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented Sep 20, 2023

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

@rust-log-analyzer
Copy link
Collaborator

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
Prepare all required actions
Getting action download info
Download action repository 'actions/checkout@v4' (SHA:8ade135a41bc03ea155e62e844d188df1ea18608)
Download action repository 'actions/upload-artifact@v3' (SHA:a8a3f3ad30e3422c9c7b888a15615d19a852ae32)
Complete job name: PR - mingw-check-tidy
git config --global core.autocrlf false
shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
---
GITHUB_ENV=/home/runner/work/_temp/_runner_file_commands/set_env_ee7d33b2-1f86-4f3c-a858-bb1657e6f732
GITHUB_EVENT_NAME=pull_request
GITHUB_EVENT_PATH=/home/runner/work/_temp/_github_workflow/event.json
GITHUB_GRAPHQL_URL=https://api.github.com/graphql
GITHUB_HEAD_REF=ci-single-cgu
GITHUB_JOB=pr
GITHUB_PATH=/home/runner/work/_temp/_runner_file_commands/add_path_ee7d33b2-1f86-4f3c-a858-bb1657e6f732
GITHUB_REF=refs/pull/107651/merge
GITHUB_REF_NAME=107651/merge
GITHUB_REF_PROTECTED=false
---

Sending build context to Docker daemon  432.1kB

Step 1/10 : FROM ubuntu:22.04
Head "https://registry-1.docker.io/v2/library/ubuntu/manifests/22.04": Get "https://auth.docker.io/token?account=githubactions&scope=repository%3Alibrary%2Fubuntu%3Apull&service=registry.docker.io": EOF
DEPRECATED: The legacy builder is deprecated and will be removed in a future release.
            BuildKit is currently disabled; enable it by removing the DOCKER_BUILDKIT=0
            environment-variable.


Sending build context to Docker daemon  432.1kB

Step 1/10 : FROM ubuntu:22.04
Get "https://registry-1.docker.io/v2/library/ubuntu/manifests/sha256:aabed3296a3d45cede1dc866a24476c4d7e093aa806263c27ddaadbdce3c1054": EOF
DEPRECATED: The legacy builder is deprecated and will be removed in a future release.
            BuildKit is currently disabled; enable it by removing the DOCKER_BUILDKIT=0
            environment-variable.


Sending build context to Docker daemon  432.1kB

Step 1/10 : FROM ubuntu:22.04
Head "https://registry-1.docker.io/v2/library/ubuntu/manifests/22.04": EOF
DEPRECATED: The legacy builder is deprecated and will be removed in a future release.
            BuildKit is currently disabled; enable it by removing the DOCKER_BUILDKIT=0
            environment-variable.


Sending build context to Docker daemon  432.1kB

Step 1/10 : FROM ubuntu:22.04
Get "https://registry-1.docker.io/v2/library/ubuntu/manifests/sha256:aabed3296a3d45cede1dc866a24476c4d7e093aa806263c27ddaadbdce3c1054": EOF
##[error]Process completed with exit code 1.
Post job cleanup.

@bors bors merged commit 871407a into rust-lang:master Oct 1, 2023
17 of 22 checks passed
@rustbot rustbot added this to the 1.75.0 milestone Oct 1, 2023
@lnicola
Copy link
Member

lnicola commented Oct 1, 2023

How did this get merged by bors? There's no r+.

@lqd
Copy link
Member

lqd commented Oct 1, 2023

This draft PR contains a single commit ca59652 which was merged in #115554

@Kobzol Kobzol deleted the ci-single-cgu branch October 1, 2023 15:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc perf-regression Performance regression. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants