Remove usage of --use-old-text for BOLT#114649
Merged
bors merged 1 commit intorust-lang:masterfrom Aug 9, 2023
Merged
Conversation
rustbot
added
the
S-waiting-on-review
Contributor
|
@bors r+ rollup=never |
Collaborator
bors
added
S-waiting-on-bors
Collaborator
Collaborator
|
☀️ Test successful - checks-actions |
Collaborator
|
Finished benchmarking commit (d190d97): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)ResultsThis 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.
CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 630.553s -> 632.651s (0.33%) |
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Oct 9, 2023
Optimize `librustc_driver.so` with BOLT This PR optimizes `librustc_driver.so` on 64-bit Linux CI with BOLT. ### Code One thing that's not clear yet to me how to resolve is how to best pass a linker flag that we need for BOLT (the second commit). It is currently passed unconditionally, which is not a good idea. We somehow have to: 1) Only pass it when we actually plan to use BOLT. How to best do that? `config.toml` entry? Environment variable? CLI flag for bootstrap? BOLT optimization is done by `opt-dist`, therefore bootstrap doesn't know about it by default. 2) Only pass it to `librustc_driver.so` (see performance below). Some discussion of this flag already happened on [Zulip](https://rust-lang.zulipchat.com/#narrow/stream/326414-t-infra.2Fbootstrap/topic/Adding.20a.20one-off.20linker.20flag). ### Performance Latest perf. results can be found [here](rust-lang#102487 (comment)). Note that instruction counts are not very interesting here, there are only regressions on hello world programs. Probably caused by a larger C++ libstd (?). Summary: - ✔️ `-1.8%` mean improvement in cycle counts across many primary benchmarks. - ✔️ `-1.8%` mean Max-RSS improvement. - ✖️ 34 MiB (+48%) artifact size regression of `librustc_driver.so`. - This is caused by building `librustc_driver.so` with relocations (which are required for BOLT). Hopefully, it will be [fixed](https://discourse.llvm.org/t/bolt-rfc-a-new-mode-to-rewrite-entire-binary/68674) in the future with BOLT improvements, but now trying to reduce this size increase is [tricky](rust-lang#114649). - Note that the size of this file was recently reduced in rust-lang#115554 by pretty much the same amount (33 MiB). So the size after this PR is basically the same as it was for the last ~year. - ✖️ 1.4 MiB (+53%) artifact size regression of `rustc`. - This is annoying and pretty much unnecessary. It is caused by the way relocations are currently applied in this PR, because they are applied both to `librustc_driver.so` (where they are needed) and for `rustc` (where they aren't needed), since both are built with a single cargo invocation. We might need e.g. some tricks in the bootstrap `rustc` shim to only apply the relocation flag for the shared library and not for `rustc`. ### CI time CI (try build) got slower by ~5 minutes, which is fine, IMO. It can be further reduced by running LLVM and `librustc_driver` BOLT profile gathering at the same time (now they are gathered separately for LLVM and `librustc_driver`). r? `@Mark-Simulacrum` Also CC `@onur-ozkan,` primarily for the bootstrap linker flag issue.
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Oct 11, 2023
Optimize `librustc_driver.so` with BOLT This PR optimizes `librustc_driver.so` on 64-bit Linux CI with BOLT. ### Code One thing that's not clear yet to me how to resolve is how to best pass a linker flag that we need for BOLT (the second commit). It is currently passed unconditionally, which is not a good idea. We somehow have to: 1) Only pass it when we actually plan to use BOLT. How to best do that? `config.toml` entry? Environment variable? CLI flag for bootstrap? BOLT optimization is done by `opt-dist`, therefore bootstrap doesn't know about it by default. 2) Only pass it to `librustc_driver.so` (see performance below). Some discussion of this flag already happened on [Zulip](https://rust-lang.zulipchat.com/#narrow/stream/326414-t-infra.2Fbootstrap/topic/Adding.20a.20one-off.20linker.20flag). ### Performance Latest perf. results can be found [here](rust-lang#102487 (comment)). Note that instruction counts are not very interesting here, there are only regressions on hello world programs. Probably caused by a larger C++ libstd (?). Summary: - ✔️ `-1.8%` mean improvement in cycle counts across many primary benchmarks. - ✔️ `-1.8%` mean Max-RSS improvement. - ✖️ 34 MiB (+48%) artifact size regression of `librustc_driver.so`. - This is caused by building `librustc_driver.so` with relocations (which are required for BOLT). Hopefully, it will be [fixed](https://discourse.llvm.org/t/bolt-rfc-a-new-mode-to-rewrite-entire-binary/68674) in the future with BOLT improvements, but now trying to reduce this size increase is [tricky](rust-lang#114649). - Note that the size of this file was recently reduced in rust-lang#115554 by pretty much the same amount (33 MiB). So the size after this PR is basically the same as it was for the last ~year. - ✖️ 1.4 MiB (+53%) artifact size regression of `rustc`. - This is annoying and pretty much unnecessary. It is caused by the way relocations are currently applied in this PR, because they are applied both to `librustc_driver.so` (where they are needed) and for `rustc` (where they aren't needed), since both are built with a single cargo invocation. We might need e.g. some tricks in the bootstrap `rustc` shim to only apply the relocation flag for the shared library and not for `rustc`. ### CI time CI (try build) got slower by ~5 minutes, which is fine, IMO. It can be further reduced by running LLVM and `librustc_driver` BOLT profile gathering at the same time (now they are gathered separately for LLVM and `librustc_driver`). r? `@Mark-Simulacrum` Also CC `@onur-ozkan,` primarily for the bootstrap linker flag issue.
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Oct 14, 2023
…ulacrum Optimize `librustc_driver.so` with BOLT This PR optimizes `librustc_driver.so` on 64-bit Linux CI with BOLT. ### Code One thing that's not clear yet to me how to resolve is how to best pass a linker flag that we need for BOLT (the second commit). It is currently passed unconditionally, which is not a good idea. We somehow have to: 1) Only pass it when we actually plan to use BOLT. How to best do that? `config.toml` entry? Environment variable? CLI flag for bootstrap? BOLT optimization is done by `opt-dist`, therefore bootstrap doesn't know about it by default. 2) Only pass it to `librustc_driver.so` (see performance below). Some discussion of this flag already happened on [Zulip](https://rust-lang.zulipchat.com/#narrow/stream/326414-t-infra.2Fbootstrap/topic/Adding.20a.20one-off.20linker.20flag). ### Performance Latest perf. results can be found [here](rust-lang#102487 (comment)). Note that instruction counts are not very interesting here, there are only regressions on hello world programs. Probably caused by a larger C++ libstd (?). Summary: - ✔️ `-1.8%` mean improvement in cycle counts across many primary benchmarks. - ✔️ `-1.8%` mean Max-RSS improvement. - ✖️ 34 MiB (+48%) artifact size regression of `librustc_driver.so`. - This is caused by building `librustc_driver.so` with relocations (which are required for BOLT). Hopefully, it will be [fixed](https://discourse.llvm.org/t/bolt-rfc-a-new-mode-to-rewrite-entire-binary/68674) in the future with BOLT improvements, but now trying to reduce this size increase is [tricky](rust-lang#114649). - Note that the size of this file was recently reduced in rust-lang#115554 by pretty much the same amount (33 MiB). So the size after this PR is basically the same as it was for the last ~year. - ✖️ 1.4 MiB (+53%) artifact size regression of `rustc`. - This is annoying and pretty much unnecessary. It is caused by the way relocations are currently applied in this PR, because they are applied both to `librustc_driver.so` (where they are needed) and for `rustc` (where they aren't needed), since both are built with a single cargo invocation. We might need e.g. some tricks in the bootstrap `rustc` shim to only apply the relocation flag for the shared library and not for `rustc`. ### CI time CI (try build) got slower by ~5 minutes, which is fine, IMO. It can be further reduced by running LLVM and `librustc_driver` BOLT profile gathering at the same time (now they are gathered separately for LLVM and `librustc_driver`). r? `@Mark-Simulacrum` Also CC `@onur-ozkan,` primarily for the bootstrap linker flag issue.
github-actions bot
pushed a commit
to rust-lang/miri
that referenced
this pull request
Oct 17, 2023
Optimize `librustc_driver.so` with BOLT This PR optimizes `librustc_driver.so` on 64-bit Linux CI with BOLT. ### Code One thing that's not clear yet to me how to resolve is how to best pass a linker flag that we need for BOLT (the second commit). It is currently passed unconditionally, which is not a good idea. We somehow have to: 1) Only pass it when we actually plan to use BOLT. How to best do that? `config.toml` entry? Environment variable? CLI flag for bootstrap? BOLT optimization is done by `opt-dist`, therefore bootstrap doesn't know about it by default. 2) Only pass it to `librustc_driver.so` (see performance below). Some discussion of this flag already happened on [Zulip](https://rust-lang.zulipchat.com/#narrow/stream/326414-t-infra.2Fbootstrap/topic/Adding.20a.20one-off.20linker.20flag). ### Performance Latest perf. results can be found [here](rust-lang/rust#102487 (comment)). Note that instruction counts are not very interesting here, there are only regressions on hello world programs. Probably caused by a larger C++ libstd (?). Summary: - ✔️ `-1.8%` mean improvement in cycle counts across many primary benchmarks. - ✔️ `-1.8%` mean Max-RSS improvement. - ✖️ 34 MiB (+48%) artifact size regression of `librustc_driver.so`. - This is caused by building `librustc_driver.so` with relocations (which are required for BOLT). Hopefully, it will be [fixed](https://discourse.llvm.org/t/bolt-rfc-a-new-mode-to-rewrite-entire-binary/68674) in the future with BOLT improvements, but now trying to reduce this size increase is [tricky](rust-lang/rust#114649). - Note that the size of this file was recently reduced in rust-lang/rust#115554 by pretty much the same amount (33 MiB). So the size after this PR is basically the same as it was for the last ~year. - ✖️ 1.4 MiB (+53%) artifact size regression of `rustc`. - This is annoying and pretty much unnecessary. It is caused by the way relocations are currently applied in this PR, because they are applied both to `librustc_driver.so` (where they are needed) and for `rustc` (where they aren't needed), since both are built with a single cargo invocation. We might need e.g. some tricks in the bootstrap `rustc` shim to only apply the relocation flag for the shared library and not for `rustc`. ### CI time CI (try build) got slower by ~5 minutes, which is fine, IMO. It can be further reduced by running LLVM and `librustc_driver` BOLT profile gathering at the same time (now they are gathered separately for LLVM and `librustc_driver`). r? `@Mark-Simulacrum` Also CC `@onur-ozkan,` primarily for the bootstrap linker flag issue.
cuviper
reviewed
Jan 9, 2025
| // However, it succeeds very non-deterministically. To avoid frequent artifact size swings, | ||
| // it is kept disabled for now. | ||
| // FIXME(kobzol): try to re-enable this once BOLT in-place rewriting is merged or after | ||
| // we bump LLVM. |
Member
Author
There was a problem hiding this comment.
Yes, I tried it a few weeks ago:
Sadly, it's still quite unstable. Asked the BOLT people about it, but we haven't got anywhere yet.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This flag has reduced the size of
libLLVM.soby ~50 MiB, but sadly it is quite non-deterministic and the size savings frequently fail, thus causing large artifact size swings. To avoid the swings, it would be better to just disable the flag for now.r? @nikic