Performance of rust_analyzer #962

Activity

1. purkhusid commented on Oct 4, 2021

   

   purkhusid

   on Oct 4, 2021

   ContributorAuthor

   More actions

   The issue is very likely with this here for loop:

   rules_rust/rust/private/rust_analyzer.bzl

   Line 219 in 238b998

   for info in depset(
2. hlopko commented on Oct 11, 2021

   

   hlopko

   on Oct 11, 2021

   Member

   More actions

   Thanks for reporting! I thought about it a bit more, and I came to the conclusion that there is no easy way out here. Right now we do the depset flattening in the analysis phase of Bazel, and that seems to be dominating the profile. What we can do:

   1. move the flattening to the execution phase, this way we can let Bazel execute other actions needed by the rust analyzer action sooner (by potentially removing the depset flattening from the critical path). End to end time may be shorter. Bazel actually has to do depset flattening (or equivalent logic) when constructing the collection of all action inputs. And by flattening the depset in analysis we make the depset flattening in execution a bit faster. We'll need more data. However we know this approach will not scale forever.
   2. change the current eager approach to the lazy one. It seems your build is big enough that a single user rarely needs the whole index to do their job. Since we build the workspace-wide rust-project.json so rust-analyzer can consume it in one go, from the perspective of a user we do unnecessary work in both Bazel and rust-analyzer. If instead rust-analyzer could ask Bazel for the build information about the currently-opened-file and its dependencies, remember them, invalidate them when necessary, and also reuse the running Bazel instance, at the end the user experience may be better (and for certain repository size the experience will be better). I also mentioned this in Rust Analyzer: Generate rust-project.json without explicit target list #907 (review).

   Your thoughts?
3. purkhusid commented on Oct 11, 2021

   

   purkhusid

   on Oct 11, 2021

   ContributorAuthor

   More actions

   If rust-analyzer can be made lazy then that would be the ideal approach IMO, this needs upstream changes in rust-analyzer though?

   I also wonder if we get more flexibility if we move into the execution phase since we might be able to have better caching during the execution, e.g. a lookup table for each crate so that we don't process them more than once.

   I'm not too familiar with the internals of rust-analyzer to be of much help there but I can definitely test out any changes to the rules on our repo and see if there are improvemtents.
4. hlopko commented on Oct 12, 2021

   

   hlopko

   on Oct 12, 2021

   Member

   More actions

   Yup, we will need to talk to rust-analyzer folks and see if they would be willing to let us contribute the support for Bazel. I'm happy to ask them once we have landed #907. I don't promise I will have time to contribute the code to rust-analyzer though :)

   You raise good points about benefits of the computation in the execution phase. Those will apply when #907 lands.

   I'll make sure we link any discussions related to rust analyzer support either through issues in rules_rust repo, or in Discussions in the repo.
5. UebelAndre

   added

   rust-analyzer

   on Nov 25, 2021
6. hlopko commented on Nov 29, 2021

   

   hlopko

   on Nov 29, 2021

   Member

   More actions

   Could you check if #1010 made any difference in the performance?
7. purkhusid commented on Nov 29, 2021

   

   purkhusid

   on Nov 29, 2021

   ContributorAuthor

   More actions

   @hlopko This seems to have improved the performance significantly, it still takes some 10s of seconds for my repo but it's not too bad compared to how it was. Thanks for following up on this!
8. purkhusid

   closed this as completedon Nov 29, 2021