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.

Sign up for GitHub

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

Jump to bottom

Too many workspaces are being opened for rust-analyzer #1778

Open
dimonomid opened this issue Jan 3, 2025 · 4 comments · May be fixed by #1779
Open

Too many workspaces are being opened for rust-analyzer #1778

dimonomid opened this issue Jan 3, 2025 · 4 comments · May be fixed by #1779

Comments

@dimonomid
Copy link

Hey there. First of all, thanks a lot for your work on this project! I use it pretty much daily and so it makes my life better every day, and overall I'm very happy with it.

These days I primarily use it for Rust. I used some old version until recently, but then I had to upgrade (since the Rust project our team works on has upgraded Rust and started using lock file version 4, which Rust coming with the old ycmd didn't support), and I noticed that rust-analyzer started to consume A TON more resources: so much that it's pretty much unbearable, very hard to get any work done this way, at least on my not-extremely-powerful Thinkpad.

What I found is that for every Rust crate I open, ycmd now apparently opens a new workspace in rust-analyzer, which in turn spawns a separate rust-analyzer-proc-macro-srv process etc. With the old ycmd, this wasn't happening: there wasn't even a concept of the workspace, and a particular instance of vim could only navigate to the code which the first Rust project I open has access to.

I understand that the introduction of workspaces is supposed to be an improvement, but unfortunately with the current implementation, it's not very usable. The main problem is that currently ycmd just opens way too many rust-analyzer workspaces.

I don't have a ready solution here, since I don't know enough about rust-analyzer and ycmd; and I'm sure you'll be able to arrive on a better solution that I can, but let me still share the issues that I've had, and some workarounds that I've done to unblock myself, in case it's helpful to find the right solution.

For example, we have a massive Rust project with many crates inside (about 300 atm). As I navigate code, I of course quickly open more and more of these crates, adding more and more rust-analyzer processes, each of which consumes a good chunk of CPU and RAM. And the thing is: all of these crates are actually being indexed by rust-analyzer with just a single workspace opened, there's no need to open a separate rust-analyzer workspace for every single sub-crate we have. So these extra rust-analyzer processes just do redundant work. So in an attempt to address this problem, I tried to add the following hack: in the GetWorkspaceForFilepath function, when looking for a dir containing Cargo.toml, instead of working our way up from the current file path, start from the root directory instead, and work our way down. I just hackishly added reverse(list(.....)) to this line. It did help to solve this particular part (with the many sub-crates), however there are other manifestations of the same problem:

Another manifestation is that as I navigate to the crates which are a part of stdlib, or fetched by Cargo, the same thing happens: for every crate, a new rust-analyzer workspace is being opened. And again it's not necessary: these crates are being indexed by the workspace which has access to them. To work around this issue, I've changed the GetProjectRootFiles to return rust-toolchain.toml instead of Cargo.toml, because our projects happen to have this rust-toolchain.toml at the root of the actual Rust workspace. In addition to that, since crates fetched by Cargo or which are part of stdlib don't have this file, I updated GetWorkspaceForFilepath to return None if the file not found, instead of just returning the path to the file.

Those two things unblock me so I can keep working now. For completeness, here's the hackish commit containing both of these changes, however I'm definitely not saying it should be merged in this form, or even close to that. Just sharing it to have the changes at a glance, in case it's helpful.

As I said above, I don't know what the right solution would be here. Thinking about it, I'd like to have something like this (no idea if rust-analyzer makes it possible though): whenever we open a new file, first check with rust-analyzer if the file is already indexed by any of the currently opened workspaces. If yes, then don't do anything, and just reuse the existing workspace. And only if the file is completely unreachable from any of the currently opened workspaces, then look for the Cargo.toml (and again, starting from the root dir, so that we use the biggest workspace possible, instead of using some sub-crate). I think this would be close to ideal: we do open multiple workspaces when working on multiple independent projects simultaneously, but we don't open redundant workspaces, so no resources are being wasted.

Last note fwiw, while bisecting, I found that the commit which introduces this problem is this one: Upgrade completers; so the concept of a workspace has already existed in ycmd, but I'm guessing rust-analyzer didn't support it before, and now it finally supports it, and the issues with how ycmd chooses workspaces have manifested now.
@puremourning
Copy link
Member

Thanks for the detailed report.

it seems like we weren't expecting rust analyser to support LSP workspaces:

  def GetProjectRootFiles( self ):
    # Without LSP workspaces support, RA relies on the rootUri to detect a
    # project.
    # TODO: add support for LSP workspaces to allow users to change project
    # without having to restart RA.
    return [ 'Cargo.toml' ]

But looking at that, I also think that it's kind of wrong anyway.... This will find the nearest Cargo.toml to the file opened in the editor, but In a repo with multiple crates, we'd expect there to be multiple Cargo.toml files, and we should set the project root to the furthest Cargo.toml file found.

checking a large-ish rust project I know:

ben@BenMBP2021 wasmtime % find . -name Cargo.toml | wc -l
      88

One option without might be to change our rust completer to use the manually specified project root for all files underneath that path. You'd have to add an extra conf file with something like this:

import os
def Settings(**kwargs):
  if kwargs[ 'language' ] == 'rust':
    return {
      'project_directory': os.path.dirname(os.path.abspath(__file__))
    }

And we'd have to change rust completer to override the default heuristics.

Another alternative is to use the furthest Cargo.toml rather than the nearest one, but that's also not foolproof, as demonstrated by wasmtime above: there's no obvious way to know when to stop searching.

Then again, we could search first for Cargo.lock, as that tends to be at the project root.

Does it improve if you change GetProjectRootFiles to return [ 'Cargo.lock', 'Cargo.toml' ] ?

isn't it reasonable for there to be more Cargo.toml files in subdirectories?

@puremourning
Copy link
Member

The general problem we're trying to solve is a hard one. Vim and ycmd clients don't have any concept of workspace "folders", but the LSP was designed by the same people that designed visual studio code which bases its entire existence around this concept, so we have to sort of guess what a project root is. It seems that this guesswork is suboptimal in this case.

If we can come up with better rust-specific heuristics we can implement that (we have done so, for example for Java). I'm loathe to expose more and more fiddly stuff to the extra conf files, or start querying servers though as this is not only complexity but difficult to teach and learn.

@puremourning puremourning linked a pull request Jan 3, 2025 that will close this issue
Open
@puremourning
Copy link
Member

I put up a really rough PR. Can you let me know if it helps at all? #1779

@dimonomid
Copy link
Author

Thanks for jumping on it.

Yes I do agree with the strategy of looking for the furthest Cargo.toml (or some other file), and I also agree with the idea of looking for Cargo.lock instead; this thing alone is not enough though, see below.

I put up a really rough PR. Can you let me know if it helps at all?

I just tested it (albeit I rebased on an older revision from Oct 13, since it uses the Rust version that I need); it is an improvement for sure, but it only addresses part of the problem: as I tried to elaborate in my initial message, there are two manifestations of the problem:

  • Projects with multiple crates (and multiple Cargo.toml files) inside;
  • Third-party crates which the project uses, but which live outside of the project tree (e.g. stdlib crates, or crates fetched by Cargo).

The #1779 does address the first one 👍 However if the user navigates to e.g. a stdlib crate, there is another Cargo.lock file (rust-analyzer/lib/rustlib/src/rust/Cargo.lock), so another workspace is being opened, which is not necessary: the parts of stdlib which are used by the main project are indexed by the project's workspace as well. The same for crates fetched by Cargo (not every library has a Cargo.lock file, but many do).

To work around this as well, in my hackish change I allowed a file to not belong to any project at all. I don't know about all the implications of it in ycmd, but it seems to work just the way I need it, and rust-analyzer is able to navigate them just fine (using the workspaces of the projects which use these crates).

The general problem we're trying to solve is a hard one. Vim and ycmd clients don't have any concept of workspace "folders", but the LSP was designed by the same people that designed visual studio code which bases its entire existence around this concept, so we have to sort of guess what a project root is. It seems that this guesswork is suboptimal in this case.

Yeah I def feel you there. I myself was working on a couple different Vim plugins to introduce some concept of a "project" back in the day. Kind of a pain not to have that built-in.

If we can come up with better rust-specific heuristics we can implement that (we have done so, for example for Java). I'm loathe to expose more and more fiddly stuff to the extra conf files, or start querying servers though as this is not only complexity but difficult to teach and learn.

I actually think that one way or the other, it can be useful to add some extra configurability. Maybe even to interoperate with some existing vim plugins which try to add the concept of the "project". But even with that, I think it's very useful to have files which don't belong to their own project, so that we don't start rust-analyzer workspaces which aren't strictly necessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

WIP: Improve rust project detection puremourning/ycmd-1
2 participants
@dimonomid @puremourning