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

Build core, alloc, std with 1 CGU #122522

Closed
wants to merge 1 commit into from

Conversation

erikdesjardins
Copy link
Contributor

Locally, this makes helloworld ~24% smaller:

Stats for benchmark `helloworld`
--------------------
Target `helloworld` (artifact `helloworld`)
┌─────────────────────┬───────────────┬──────────────┬─────────┬──────────┐
│ Section             │ Size (before) │ Size (after) │    Diff │ Diff (%) │
├─────────────────────┼───────────────┼──────────────┼─────────┼──────────┤
│ .strtab             │     98.78 KiB │    52.35 KiB │  -47543 │   -47.0% │
│ .text               │    276.83 KiB │   250.88 KiB │  -26576 │    -9.4% │
│ .eh_frame           │     42.02 KiB │    22.45 KiB │  -20048 │   -46.6% │
│ .symtab             │     37.69 KiB │    18.75 KiB │  -19392 │   -50.2% │
│ .gcc_except_table   │     11.00 KiB │     4.33 KiB │   -6824 │   -60.6% │
│ .rodata             │     28.42 KiB │    24.43 KiB │   -4083 │   -14.0% │
│ .eh_frame_hdr       │      8.16 KiB │     4.19 KiB │   -4064 │   -48.6% │
│ .rela.dyn           │     20.37 KiB │    17.06 KiB │   -3384 │   -16.2% │
│ .data.rel.ro        │     10.56 KiB │     9.05 KiB │   -1544 │   -14.3% │
│ .got                │      1.98 KiB │     1.66 KiB │    -328 │   -16.2% │
│ .dynstr             │      1.04 KiB │     1.02 KiB │     -25 │    -2.3% │
│ .dynsym             │      1.59 KiB │     1.57 KiB │     -24 │    -1.5% │
│ .gnu.version_r      │         288 B │        272 B │     -16 │    -5.6% │
│ .bss                │         200 B │        216 B │     +16 │    +8.0% │
│ .gnu.version        │         136 B │        134 B │      -2 │    -1.5% │
│ .tbss               │          81 B │         80 B │      -1 │    -1.2% │
│ <16 unchanged rows> │      1.28 KiB │     1.28 KiB │       0 │     0.0% │
│─────────────────────│───────────────│──────────────│─────────│──────────│
│ Total               │    540.41 KiB │   409.71 KiB │ -133838 │   -24.2% │
└─────────────────────┴───────────────┴──────────────┴─────────┴──────────┘

It also makes ./x build --stage 1 ~10% slower after a change to the stdlib (2:45 -> 3:00).

I'm not sure if that tradeoff is worthwhile.

Let's do a perf run first.

r? @ghost

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Mar 15, 2024
@erikdesjardins
Copy link
Contributor Author

(I hope this isn't just a duplicate of #115554, and the only reason it seems like an improvement is because that flag doesn't get set unless you're on CI. Although that change didn't have any binary size improvements on compile benchmarks so it seems like it only applies to rustc itself, not std.)

@lqd
Copy link
Member

lqd commented Mar 15, 2024

Did you run your benchmark on a local build with the rust.codegen-units-std=1 config? This should be the setting we currently use to build the std on the main targets.

rust.codegen-units=1 is indeed set on CI so that local builds are not slowed down too much. IIRC #115554 didn’t have binary size improvements because we were already building the std with 1 CGU.

@erikdesjardins
Copy link
Contributor Author

Ah, that's what I was missing. Yeah, this has no effect.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants