search

Shopify / ruby-lsp

An opinionated language server for Ruby
https://shopify.github.io/ruby-lsp/
MIT License
1.63k stars 168 forks source link

Improve workspace/symbol request's performance #2660

Open gmcabrita opened 2 months ago

gmcabrita commented 2 months ago

I have checked that this feature is not already implemented

Use case

The performance of the global go to symbol in ruby-lsp is noticeably worse when not excluding any gems than when excluding every gem.

Here are some short videos showing the difference:

https://github.com/user-attachments/assets/c5d5d922-df3b-46c7-b9f7-6dd65c9359cc

https://github.com/user-attachments/assets/ece1805e-0cba-4592-a51c-cf57f9c66a6e

Currently, if I want to exclude every gem, I'm forced to list them all in my editor's config file: https://github.com/gmcabrita/dotfiles/blob/f7b8d85449b4f6da3acbc7428548ceb4ca29f999/.config/zed/settings.json#L137-L585.

I also have to update that list as time goes on and new gems are added to an existing project, or I jump to a new project that has gems I am not currently ignoring.

Providing a setting to exclude all gems without having to list them would be a nice improvement.

Description

Possibly support one of these in the editor settings, ensuring that no gems are ever processed/indexed:

{
  "excludedGemPatterns": ["*"]
}

or 

{
  "excludeAllGems": true
}

Implementation

No response

st0012 commented 1 month ago

If I understand it correctly, global go to symbol is equivalent to workspace symbols in VS Code. If that's the case, we already started excluding dependencies from the request with/without indexing them (https://github.com/Shopify/ruby-lsp/pull/2131). Can you please upgrade to the latest ruby-lsp to confirm it?

Also, excluding all gems from indexing will seriously degrade the functionality of other features like go-to-definition, autocompletion...etc. so it's not something we'll encourage users to do and it's rarely the right solution to the issue.

gmcabrita commented 1 month ago

If I understand it correctly, global go to symbol is equivalent to workspace symbols in VS Code.

Yes, this is for the workspace/symbol method.

If that's the case, we already started excluding dependencies from the request with/without indexing them (https://github.com/Shopify/ruby-lsp/pull/2131). Can you please upgrade to the latest ruby-lsp to confirm it?

I checked with v0.20.0 and the performance difference is still pretty much the same.

Also, excluding all gems from indexing will seriously degrade the functionality of other features like go-to-definition, autocompletion...etc. so it's not something we'll encourage users to do and it's rarely the right solution to the issue.

I understand that this degrades the go-to definition and autocomplete functionality for external gems. Personally, the performance hit of including all these gems is so annoying that I don't care if I have autocomplete on them or not. I suspect other developers are affected as well, they probably just don't care about editor latency as much as I do or they would have excluded all the gems like I did.

I understand if this is not a setting that ruby-lsp wants to support. Performance for workspace/symbol should still be worked on even if this setting is supported, since it's just a band-aid.

st0012 commented 1 month ago

I agree that we can possibly improve the performance of the workspace/symbol request so it has less delay. Therefore, I'll change the title of this issue to be explicit about this end goal.

But I also need to be honest that given our team's current backlog, this will be low on our list for at least a while. That said, if you want to investigate what's causing the slowdown, PRs are welcome 🙂