Use LTO to optimize Rust tools (cargo, miri, rustfmt, clippy, Rust Analyzer)#139588
Use LTO to optimize Rust tools (cargo, miri, rustfmt, clippy, Rust Analyzer)#139588bors merged 1 commit intorust-lang:masterfrom
Conversation
rustbot
added
S-waiting-on-review
|
@bors try |
Apply LTO when building rust-analyzer Trying if LTO/PGO can help RA's performance, and by how much. CC `@Veykril` I realized that we don't even do LTO for Rust Analyzer, that could be a very low hanging fruit to improve its performance 😅 try-job: dist-x86_64-linux
|
☀️ Try build successful - checks-actions |
|
Tried the LTO optimized version on the Looks like a solid 2% win. Not bad! |
Kobzol
force-pushed
the
rust-analyzer-opt
branch
from
3501da3 to
257d63f
Compare
|
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
rustbot
added
the
S-waiting-on-perf
Apply LTO when building rust-analyzer Trying if LTO/PGO can help RA's performance, and by how much. CC `@Veykril` I realized that we don't even do LTO for Rust Analyzer, that could be a very low hanging fruit to improve its performance 😅 try-job: dist-x86_64-linux
|
☀️ Try build successful - checks-actions |
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (1c4fbb0): comparison URL. Overall result: ✅ improvements - no action neededBenchmarking 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. @bors rollup=never Instruction countThis is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.
Max RSS (memory usage)Results (primary -2.6%, secondary 3.2%)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.
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: 780.203s -> 780.244s (0.01%) |
rustbot
removed
the
S-waiting-on-perf
Kobzol
changed the title
|
CI duration seems to be fine. The small win should be from LTO optimizing Cargo. @rustbot ready |
Kobzol
force-pushed
the
rust-analyzer-opt
branch
from
257d63f to
9a26863
Compare
This comment was marked as off-topic.
This comment was marked as off-topic.
|
Ah -- do note that we're not running the main r-a test suite yet (only proc-macro-srv), but I believe other tool tests are run. |
jieyouxu
left a comment
jieyouxu
left a comment
There was a problem hiding this comment.
Other tools seem fine, waiting to hear back re. rust-analyzer because we don't run main r-a test suite in r-l/r CI yet
|
FYI @rust-lang/rust-analyzer: are you okay with building r-a with LTO? |
|
Yeah, LTO seems reasonable to us! |
|
On a related note, does this also affect the proc-macro server? I imagine not as I don't think it is considered a tool by boostrap? |
|
It is also compiled with |
|
Since r-a is on board with LTO, let's give it a try. Thanks! @bors r+ rollup=never |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions |
What is this?This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.Comparing e1b06f7 (parent) -> ed3a4aa (this PR) Test differencesNo test diffs found Job duration changes
How to interpret the job duration changes?Job durations can vary a lot, based on the actual runner instance |
|
Finished benchmarking commit (ed3a4aa): comparison URL. Overall result: ✅ improvements - no action needed@rustbot label: -perf-regression Instruction countThis is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.
Max RSS (memory usage)Results (secondary 2.2%)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.
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: 779.161s -> 779.659s (0.06%) |
| // Rustc tools (miri, clippy, cargo, rustfmt, rust-analyzer) | ||
| // could use the additional optimizations. | ||
| if self.mode == Mode::ToolRustc && | ||
| // rustdoc is performance sensitive, so apply LTO to it. |
There was a problem hiding this comment.
I have no object to the change, but the new comment should probably merge in this one.
Fix comment in bootstrap Didn't notice it in rust-lang#139588. r? `@jieyouxu`
Fix comment in bootstrap Didn't notice it in rust-lang#139588. r? ``@jieyouxu``
Fix comment in bootstrap Didn't notice it in rust-lang#139588. r? ```@jieyouxu```
Fix comment in bootstrap Didn't notice it in rust-lang#139588. r? ````@jieyouxu````
Rollup merge of rust-lang#139707 - Kobzol:fix-comment, r=jieyouxu Fix comment in bootstrap Didn't notice it in rust-lang#139588. r? ````@jieyouxu````
Fix comment in bootstrap Didn't notice it in rust-lang/rust#139588. r? ````@jieyouxu````
9 participants
Trying if LTO/PGO can help RA's performance, and by how much. As @Noratrieb suggested, we could actually LTO optimize all the important tools.
CC @Veykril I realized that we don't even do LTO for Rust Analyzer, that could be a very low hanging fruit to improve its performance 😅
try-job: dist-x86_64-linux