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

[perftest] Use LTO for compiling rustdoc #102885

Closed
wants to merge 2 commits into from

Conversation

Kobzol
Copy link
Contributor

@Kobzol Kobzol commented Oct 10, 2022

r? @ghost

@rustbot rustbot added T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) A-rustdoc-json Area: Rustdoc JSON backend T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Oct 10, 2022
@Kobzol
Copy link
Contributor Author

Kobzol commented Oct 10, 2022

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

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

bors commented Oct 10, 2022

⌛ Trying commit 3b4f19def7bdb4735789ccee560d0ad68c5d824a with merge dac891620058ebee5be0f07de78fa232da86a7de...

@bors
Copy link
Contributor

bors commented Oct 10, 2022

💔 Test failed - checks-actions

@bors bors added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Oct 10, 2022
@rust-log-analyzer

This comment has been minimized.

@Kobzol
Copy link
Contributor Author

Kobzol commented Oct 11, 2022

@bors try

@bors
Copy link
Contributor

bors commented Oct 11, 2022

⌛ Trying commit d0cdda0fd25f04894e54a4a430b348f43f690577 with merge 7ba8fc4f935a67c23318821d69a711ed1243205f...

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@Kobzol
Copy link
Contributor Author

Kobzol commented Oct 12, 2022

Hmm, looks like we're missing bitcode for jemalloc:

Couldn't add bitcode from jemalloc_sys-f8a71c72a88bc04e.jemalloc_sys.3fd4af2a-cgu.0.rcgu.o

I wonder why this wasn't a problem for librustc_driver, which AFAIK sets up jemalloc in the same way.
It's possibly by the fact that librustdoc uses extern crate jemalloc; (?).

I noticed that librustc_driver also uses extern crate tracing;, which caused a problem here, but it wasn't a problem for the driver. @bjorn3 did you encounter any issues with using LTO for crates included from the sysroot (extern crate X;) when you were working on the librustc_driver LTO PR?

@bjorn3
Copy link
Member

bjorn3 commented Oct 12, 2022

@bjorn3 did you encounter any issues with using LTO for crates included from the sysroot (extern crate X;) when you were working on the librustc_driver LTO PR?

I don't think I tested that.

@Kobzol
Copy link
Contributor Author

Kobzol commented Oct 12, 2022

I asked because librustc_driver also uses extern crate tracing;, but I haven't seen it causing any problems.

@bors
Copy link
Contributor

bors commented Oct 14, 2022

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

@Kobzol
Copy link
Contributor Author

Kobzol commented Oct 25, 2022

@bors try

@bors
Copy link
Contributor

bors commented Oct 25, 2022

⌛ Trying commit 0b38d6de615621b4cdb348d90621d89994cc04e6 with merge b7339bc24f5637c9784f27c199e176a1891af429...

@bors
Copy link
Contributor

bors commented Oct 25, 2022

💔 Test failed - checks-actions

@Kobzol
Copy link
Contributor Author

Kobzol commented Oct 25, 2022

@bors try

@bors
Copy link
Contributor

bors commented Oct 25, 2022

⌛ Trying commit 9a473cc with merge 13e91afe8ca2f1641d8d6cbcffe6e2860b18cc33...

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented Oct 25, 2022

☀️ Try build successful - checks-actions
Build commit: 13e91afe8ca2f1641d8d6cbcffe6e2860b18cc33 (13e91afe8ca2f1641d8d6cbcffe6e2860b18cc33)

@rust-timer
Copy link
Collaborator

Queued 13e91afe8ca2f1641d8d6cbcffe6e2860b18cc33 with parent 85d089b, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (13e91afe8ca2f1641d8d6cbcffe6e2860b18cc33): comparison URL.

Overall result: ✅ improvements - no 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.

@bors rollup=never
@rustbot label: +S-waiting-on-review -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.

mean1 range count2
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-1.5% [-1.5%, -1.5%] 1
All ❌✅ (primary) - - 0

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.

mean1 range count2
Regressions ❌
(primary)
10.2% [2.1%, 20.8%] 22
Regressions ❌
(secondary)
4.1% [3.5%, 5.2%] 3
Improvements ✅
(primary)
-4.2% [-5.3%, -3.0%] 4
Improvements ✅
(secondary)
-3.5% [-3.5%, -3.5%] 1
All ❌✅ (primary) 8.0% [-5.3%, 20.8%] 26

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.

mean1 range count2
Regressions ❌
(primary)
10.7% [1.2%, 42.0%] 92
Regressions ❌
(secondary)
6.2% [1.8%, 16.9%] 34
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 10.7% [1.2%, 42.0%] 92

Footnotes

  1. the arithmetic mean of the percent change 2 3

  2. number of relevant changes 2 3

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Oct 25, 2022
@Kobzol
Copy link
Contributor Author

Kobzol commented Oct 26, 2022

This probably hit the modifications of the perf. CI machine, rerunning perf.

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Oct 26, 2022
@Kobzol
Copy link
Contributor Author

Kobzol commented Nov 4, 2022

After discussing this with @lqd and @nnethercote, I think that this is actually not needed. librustc_driver is now LTO optimized, and the rest of the doc code is in librustdoc, which is a single crate, so thin LTO probably won't have a big effect on it. So I'm inclined to close this.

@bjorn3 Do you agree with my assessment of the rustdoc LTO "situation"?

@bjorn3
Copy link
Member

bjorn3 commented Nov 4, 2022

Rustdoc uses several external crates like a markdown parser. Those could benefit from LTO. I don't know how big the effect would be though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rustdoc-json Area: Rustdoc JSON backend S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. S-waiting-on-perf Status: Waiting on a perf run to be completed. 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) T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants