Skip to content

feat: Index workspace symbols at startup rather than on the first symbol search. #18180

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

Merged
merged 1 commit into from
Sep 25, 2024

Conversation

kpreid
Copy link
Contributor

@kpreid kpreid commented Sep 24, 2024

This will eliminate potential many-second delays when performing the first search, at the price of making cache priming (“Indexing N/M” in the VS Code status bar) take a little longer in total. Hopefully this additional time is insignificant because a typical session will involve at least one symbol search.

Further improvement would be to do this as a separate parallel task (which will be beneficial if the workspace contains a small number of large crates), but that would require significant additional refactoring of the progress-reporting mechanism to understand multiple tasks per crate. Happy to tackle that in this PR if desired, but I thought I'd propose the minimal change first.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 24, 2024
@Veykril
Copy link
Member

Veykril commented Sep 25, 2024

This sounds fine to me but @davidbarsky might have opinions about this given this "slows down startup"

@davidbarsky
Copy link
Contributor

I'll fix #18181 and rebase this PR against 18181, but I don't expect this to be PR to be an issue.


As to why I care about startup times: at work, my team has some extremely detailed telemetry concerning people's usage of rust-analyzer and we've found that people want to start using rust-analyzer immediately after opening a Rust file, so faster startup times are a good thing. However, prior changes that increased cache priming times to reduce the number of uncached items also helped perceived despite nominally longer startup times.

The general tenor of written/survey feedback from people is that while they wish they didn't need to wait for rust-analyzer to start, they found that the steady-state, warm rust-analyzer is heads-and-shoulders better than any other supported language's LSP.

(I should probably write this down somewhere more permanent. Maybe a blog post?)

@kpreid
Copy link
Contributor Author

kpreid commented Sep 25, 2024

FWIW, my personal motivation for this change is broadly like:

  1. I open VS Code on a project. I know this will take a while, so I do something else meanwhile. (Or maybe it just reopened after a reboot, or something of that flavor. In any case, RA has started up but has had no user interactions.)
  2. I come back after it’s ready and the first thing I want to do on the project is navigate to a symbol (or a file, because I have VS Code set to mix file and symbol search) and I have to wait an eternal ~10 seconds for the indexing.
  3. If I go do something else while I wait for that, then the VS Code search popup will vanish and I will have no progress feedback and I will have to retype my search :(

And even if the symbol search isn't the first interaction overall, the delay on the first search is still a significant factor since jumping to a symbol is often an important task right in the middle of “having a thought”.

I expect that this aligns well with @davidbarsky’s goals.

@davidbarsky
Copy link
Contributor

It aligns with my goals, yeah.

I rebased your changes locally against my tracing changes and saw that parallel_prime_caches on my preferred test project (buck2) made parallel_prime_caches go from ~28 seconds to ~34 seconds, but symbol search (invoked by Command + T in VS Code was instant. I think this is a reasonable tradeoff.

@davidbarsky
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Sep 25, 2024

📌 Commit a050c5d has been approved by davidbarsky

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Sep 25, 2024

⌛ Testing commit a050c5d with merge 88fad49...

@bors
Copy link
Contributor

bors commented Sep 25, 2024

☀️ Test successful - checks-actions
Approved by: davidbarsky
Pushing 88fad49 to master...

@bors bors merged commit 88fad49 into rust-lang:master Sep 25, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants