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

Compile the libstd we distribute with -Ccodegen-unit=1 #55264

Merged
merged 3 commits into from
Oct 26, 2018

Conversation

michaelwoerister
Copy link
Member

This PR

  • adds the single-codegen-unit-std option to config.toml which allows for setting the CGU count for libstd and libtest independently of the one for the rest of the compiler, and
  • sets the new option to true for all dist jobs in CI.

Fixes #54872.

r? @Mark-Simulacrum
cc @rust-lang/release

@Mark-Simulacrum
Copy link
Member

Hm, I'd prefer that we instead have this as codegen-unit-std=1 instead of single-codegen-unit-std=true; with that change, though, I think we should be able to use this.

@bors try to get at least some amount of timing information

@michaelwoerister
Copy link
Member Author

I'd prefer that we instead have this as codegen-unit-std=1 instead of single-codegen-unit-std=true

Sure, no problem.

@nikic
Copy link
Contributor

nikic commented Oct 22, 2018

Would it be possible to add the --set rust.single-codegen-unit-std=true flag for all DEPLOY=1 jobs in run.sh, instead of specifying it in each docker file? (This is probably a stupid question, I'm not particularly familiar with the build system.)

@Mark-Simulacrum
Copy link
Member

Yeah, that's probably the better way to do it. I think it should go somewhere here: https://github.com/rust-lang/rust/blob/master/src/ci/run.sh#L56

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 22, 2018
@bors
Copy link
Contributor

bors commented Oct 22, 2018

⌛ Trying commit c4ca23afbf1138f094fb813e7fe8eaaec73ef59c with merge 0c6e1283b0a9fe23e54018a5dcdd3ec6d84d1896...

@rust-highfive
Copy link
Collaborator

Your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
travis_fold:end:services

travis_fold:start:git.checkout
travis_time:start:00e7ba4e
$ git clone --depth=2 --branch=try https://github.com/rust-lang/rust.git rust-lang/rust
---
Resolving deltas: 100% (7937/7937), done.
travis_time:end:00e7ba4e:start=1540241159474675330,finish=1540241165516488837,duration=6041813507
$ cd rust-lang/rust
$ git checkout -qf 0c6e1283b0a9fe23e54018a5dcdd3ec6d84d1896
fatal: reference is not a tree: 0c6e1283b0a9fe23e54018a5dcdd3ec6d84d1896
The command "git checkout -qf 0c6e1283b0a9fe23e54018a5dcdd3ec6d84d1896" failed and exited with 128 during .
Your build has been stopped.

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

2 similar comments
@rust-highfive
Copy link
Collaborator

Your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
travis_fold:end:services

travis_fold:start:git.checkout
travis_time:start:00e7ba4e
$ git clone --depth=2 --branch=try https://github.com/rust-lang/rust.git rust-lang/rust
---
Resolving deltas: 100% (7937/7937), done.
travis_time:end:00e7ba4e:start=1540241159474675330,finish=1540241165516488837,duration=6041813507
$ cd rust-lang/rust
$ git checkout -qf 0c6e1283b0a9fe23e54018a5dcdd3ec6d84d1896
fatal: reference is not a tree: 0c6e1283b0a9fe23e54018a5dcdd3ec6d84d1896
The command "git checkout -qf 0c6e1283b0a9fe23e54018a5dcdd3ec6d84d1896" failed and exited with 128 during .
Your build has been stopped.

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@rust-highfive
Copy link
Collaborator

Your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
travis_fold:end:services

travis_fold:start:git.checkout
travis_time:start:00e7ba4e
$ git clone --depth=2 --branch=try https://github.com/rust-lang/rust.git rust-lang/rust
---
Resolving deltas: 100% (7937/7937), done.
travis_time:end:00e7ba4e:start=1540241159474675330,finish=1540241165516488837,duration=6041813507
$ cd rust-lang/rust
$ git checkout -qf 0c6e1283b0a9fe23e54018a5dcdd3ec6d84d1896
fatal: reference is not a tree: 0c6e1283b0a9fe23e54018a5dcdd3ec6d84d1896
The command "git checkout -qf 0c6e1283b0a9fe23e54018a5dcdd3ec6d84d1896" failed and exited with 128 during .
Your build has been stopped.

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@bors
Copy link
Contributor

bors commented Oct 22, 2018

💔 Test failed - status-travis

@rust-highfive
Copy link
Collaborator

Your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
travis_fold:end:services

travis_fold:start:git.checkout
travis_time:start:00e7ba4e
$ git clone --depth=2 --branch=try https://github.com/rust-lang/rust.git rust-lang/rust
---
Resolving deltas: 100% (7937/7937), done.
travis_time:end:00e7ba4e:start=1540241159474675330,finish=1540241165516488837,duration=6041813507
$ cd rust-lang/rust
$ git checkout -qf 0c6e1283b0a9fe23e54018a5dcdd3ec6d84d1896
fatal: reference is not a tree: 0c6e1283b0a9fe23e54018a5dcdd3ec6d84d1896
The command "git checkout -qf 0c6e1283b0a9fe23e54018a5dcdd3ec6d84d1896" failed and exited with 128 during .
Your build has been stopped.

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@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 Oct 22, 2018
@michaelwoerister
Copy link
Member Author

Thanks for pointing that out, @nikic. That's a much better way of setting the flag.

@michaelwoerister
Copy link
Member Author

@bors try

@bors
Copy link
Contributor

bors commented Oct 23, 2018

⌛ Trying commit 4545b3e with merge 00b7129...

bors added a commit that referenced this pull request Oct 23, 2018
Compile the libstd we distribute with -Ccodegen-unit=1

This PR
 - adds the `single-codegen-unit-std` option to `config.toml` which allows for setting the CGU count for `libstd` and `libtest` independently of the one for the rest of the compiler, and
 - sets the new option to `true` for all dist jobs in CI.

Fixes #54872.

r? @Mark-Simulacrum
cc @rust-lang/release
@bors
Copy link
Contributor

bors commented Oct 23, 2018

💔 Test failed - status-travis

@rust-highfive
Copy link
Collaborator

The job dist-x86_64-linux of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
travis_fold:end:services

travis_fold:start:git.checkout
travis_time:start:0a072a42
$ git clone --depth=2 --branch=try https://github.com/rust-lang/rust.git rust-lang/rust
---
[00:03:53]    Compiling toml v0.4.6
[00:03:53]    Compiling serde_json v1.0.31
[00:04:01]    Compiling bootstrap v0.0.0 (/checkout/src/bootstrap)
[00:04:34]     Finished dev [unoptimized] target(s) in 1m 06s
[00:04:34] failed to parse TOML configuration 'config.toml': invalid type: string "1", expected u32 for key `rust.codegen-units-std`
[00:04:34] Build completed unsuccessfully in 0:01:28
[00:04:34] make: *** [prepare] Error 1
[00:04:35] Command failed. Attempt 2/5:
[00:04:35]     Finished dev [unoptimized] target(s) in 0.26s
[00:04:35]     Finished dev [unoptimized] target(s) in 0.26s
[00:04:35] failed to parse TOML configuration 'config.toml': invalid type: string "1", expected u32 for key `rust.codegen-units-std`
[00:04:35] Build completed unsuccessfully in 0:00:00
[00:04:35] make: *** [prepare] Error 1
[00:04:37] Command failed. Attempt 3/5:
[00:04:37]     Finished dev [unoptimized] target(s) in 0.26s
[00:04:37]     Finished dev [unoptimized] target(s) in 0.26s
[00:04:37] failed to parse TOML configuration 'config.toml': invalid type: string "1", expected u32 for key `rust.codegen-units-std`
[00:04:37] Build completed unsuccessfully in 0:00:00
[00:04:37] make: *** [prepare] Error 1
[00:04:40] Command failed. Attempt 4/5:
[00:04:41]     Finished dev [unoptimized] target(s) in 0.26s
[00:04:41]     Finished dev [unoptimized] target(s) in 0.26s
[00:04:41] failed to parse TOML configuration 'config.toml': invalid type: string "1", expected u32 for key `rust.codegen-units-std`
[00:04:41] Build completed unsuccessfully in 0:00:00
[00:04:41] make: *** [prepare] Error 1
[00:04:45] Command failed. Attempt 5/5:
[00:04:45]     Finished dev [unoptimized] target(s) in 0.26s
[00:04:45]     Finished dev [unoptimized] target(s) in 0.26s
[00:04:45] failed to parse TOML configuration 'config.toml': invalid type: string "1", expected u32 for key `rust.codegen-units-std`
[00:04:45] Build completed unsuccessfully in 0:00:00
[00:04:45] make: *** [prepare] Error 1
[00:04:45] The command has failed after 5 attempts.
travis_time:end:0c1d07fc:start=1540297112928164652,finish=1540297398733819004,duration=285805654352
---
travis_time:end:14670954:start=1540297399169557079,finish=1540297399177338390,duration=7781311
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:2adff231
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb --batch -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:07e41598
travis_time:start:07e41598
$ cat ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
cat: ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers: No such file or directory
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:02dd237c
$ dmesg | grep -i kill

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@michaelwoerister
Copy link
Member Author

@bors try

@bors
Copy link
Contributor

bors commented Oct 23, 2018

⌛ Trying commit 03551e1 with merge d803782...

bors added a commit that referenced this pull request Oct 23, 2018
Compile the libstd we distribute with -Ccodegen-unit=1

This PR
 - adds the `single-codegen-unit-std` option to `config.toml` which allows for setting the CGU count for `libstd` and `libtest` independently of the one for the rest of the compiler, and
 - sets the new option to `true` for all dist jobs in CI.

Fixes #54872.

r? @Mark-Simulacrum
cc @rust-lang/release
@bors
Copy link
Contributor

bors commented Oct 23, 2018

☀️ Test successful - status-travis
State: approved= try=True

@michaelwoerister
Copy link
Member Author

@Mark-Simulacrum: Changes applied and try build succeeds :)

@Mark-Simulacrum
Copy link
Member

@rust-timer build d803782

If this shows performance wins then we might want to enable this on all builds, not just dist builds, since it could make the compiler fast(er) sufficiently to make the wins on test compilations significant.

@rust-timer
Copy link
Collaborator

Success: Queued d803782 with parent d74b402, comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit d803782

@michaelwoerister
Copy link
Member Author

Performance looks pretty good!

@Mark-Simulacrum
Copy link
Member

Can you make the change to set the flag on all CI builds vs. just dist builds? I would expect that to be mildly faster with tests so maybe worth it? r=me with that

@nikic
Copy link
Contributor

nikic commented Oct 24, 2018

Interestingly, there's also a few percent of max-rss improvement. I wonder where that comes from, wouldn't have expected codegen units to affect this. Is LLVM optimizing away allocations here?

@michaelwoerister
Copy link
Member Author

@bors r=Mark-Simulacrum

@bors
Copy link
Contributor

bors commented Oct 25, 2018

📌 Commit c4aa361e3d0771b18ad2e832c4f28eb345acd686 has been approved by Mark-Simulacrum

@bors 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 Oct 25, 2018
@michaelwoerister
Copy link
Member Author

Rebasing & re-approving in the hope that that will make the PR show up in the bors queue properly.

@bors r=simulacrum

@bors
Copy link
Contributor

bors commented Oct 26, 2018

📌 Commit 5dedf0c has been approved by simulacrum

kennytm added a commit to kennytm/rust that referenced this pull request Oct 26, 2018
…simulacrum

Compile the libstd we distribute with -Ccodegen-unit=1

This PR
 - adds the `single-codegen-unit-std` option to `config.toml` which allows for setting the CGU count for `libstd` and `libtest` independently of the one for the rest of the compiler, and
 - sets the new option to `true` for all dist jobs in CI.

Fixes rust-lang#54872.
bors added a commit that referenced this pull request Oct 26, 2018
Rollup of 21 pull requests

Successful merges:

 - #54816 (Don't try to promote already promoted out temporaries)
 - #54824 (Cleanup rustdoc tests with `@!has` and `@!matches`)
 - #54921 (Add line numbers option to rustdoc)
 - #55167 (Add a "cheap" mode for `compute_missing_ctors`.)
 - #55258 (Fix Rustdoc ICE when checking blanket impls)
 - #55264 (Compile the libstd we distribute with -Ccodegen-unit=1)
 - #55271 (Unimplement ExactSizeIterator for MIR traversing iterators)
 - #55292 (Macro diagnostics tweaks)
 - #55298 (Point at macro definition when no rules expect token)
 - #55301 (List allowed tokens after macro fragments)
 - #55302 (Extend the impl_stable_hash_for! macro for miri.)
 - #55325 (Fix link to macros chapter)
 - #55343 (rustbuild: fix remap-debuginfo when building a release)
 - #55346 (Shrink `Statement`.)
 - #55358 (Remove redundant clone (2))
 - #55370 (Update mailmap for estebank)
 - #55375 (Typo fixes in configure_cmake comments)
 - #55378 (rustbuild: use configured linker to build boostrap)
 - #55379 (validity: assert that unions are non-empty)
 - #55383 (Use `SmallVec` for the queue in `coerce_unsized`.)
 - #55391 (bootstrap: clean up a few clippy findings)
@bors bors merged commit 5dedf0c into rust-lang:master Oct 26, 2018
im-0 pushed a commit to im-0/fedora-rpm.rust that referenced this pull request Dec 9, 2019
Upstream started this in rust-lang/rust#55264,
which was released in Rust 1.32.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants