-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Apply Profile-guided optimization to improve performance #9412
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
Comments
It would be feasibly, but the tradeoff between additional perf boost and additional burden of maintaining a more complex build process is not worth it at this stage. It’s more impactful to spend the effort on making rust-analyzer more performant directly. The primary blocker for that work is understanding rust-analyzer’s heap structure: #9309 |
I just tried this, it's an ~8% improvement in
Steps for posterity, since they weren't obvious: RUSTFLAGS="-C profile-generate" cargo build --release
target/release/rust-analyzer analysis-stats .
llvm-profdata merge *.profraw --output merged.profdata
RUSTFLAGS="-C profile-use=$PWD/merged.profdata" cargo build --release Probably not worth the hassle for now. |
Was some time since this issue and RA is maturing quite nicely, maybe we should look over this again? |
for a speed-up of 20%. For
(19% faster) My steps in #9412 (comment) still work, but seem to yield a smaller improvement. I'm not sure if In any case, 15-20% is a decent improvement. |
Great! This looks awesome, 15-20% is a huge improvement. |
15-20% is a drop in the ocean compared to the algorithmic improvements (that nobody had time/managed/knew how to make) 😅. |
@lnicola It has been quite some time since the last time PGO was evaluated, Are rust-analyzer in a better state where it may be suitable to distribute PGO-optimized builds? |
Are PGO builds now distributed? |
No. There are still bigger wins to be gained (which nobody tries currently AFAIK). |
Are you referring to this? #9309 |
@ChayimFriedman2 jfyi, i am using rust-analyzer every day, all day - every single percent shaved off of runtime of this tool is a huge win for my work day. I urge you to not ignore a 15% win that is basically coming for free. |
I think as of right now there is a bug with PGO + LTO in the rust compiler preventing both to be used simultaneously, so any exploration of eventual performance would probably be best after that is fixed. |
The problem with pgo in combination with LTO have now been fixed so PGO can be explored again. |
#19582 does this now |
#19582 is awesome, but is there a blocker from enabling it on windows? |
If we want to provide PGO-optimized binaries to users, I think we need to reopen this issue or create another one due to this PR: #19583 |
Seem the problem with zigbuild not working is maybe something that have to be changed in zig. |
I extended the PGO logic and changed CI to use PGO for Windows. If someone with a Windows machine would like to test the performance effect, the instructions are in #19585. |
|
@Kobzol you mentioned in #19582 (comment) that PGO on macOS is quite broken. Could you please elaborate on this a bit more? Is there any open issue for that with any details? I am asking it since I don't remember issues with PGO on macOS platform. |
It's possible I was being a bit too pessimistic, I remember having a lot of trouble to apply PGO on the compiler itself on macOS. That being said, it was some time ago, and it might have actually been on the LLVM side, not the rustc side. Would you like to try PGO on macOS for rust-analyzer? It should be similar to the PRs I already sent. |
Yeah, let's try it for macOS too. I'll create a PR for that. |
We enabled PGO for macOS platform too. The results are available here: #19611 (comment) |
Profile-guided optimization (PGO) shows great promise in improving the speed of software, last year tests where made on applying it on Rust itself improving build time by ~15%.
Would it be feasible to do something similar for Rust analyzer to improve its speed?
As i understand it the difficulties would be
The text was updated successfully, but these errors were encountered: