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.

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

Memory usage quadrupled after salsa migration #19402

Open
alexkirsz opened this issue Mar 20, 2025 · 4 comments
Open

Memory usage quadrupled after salsa migration #19402

alexkirsz opened this issue Mar 20, 2025 · 4 comments
Labels
C-bug Category: bug

Comments

@alexkirsz
Copy link
Contributor

alexkirsz commented Mar 20, 2025

rust-analyzer version: 2025-03-17

rustc version: nightly/2025-01-04

editor or extension: Zed

relevant settings: N/A

After we updated to the latest release of RA, we've seen its memory usage more than quadruple when running on our codebase. Before the update, RA was routinely taking between 5 to 6GB of memory. After the update, it balloons up to 22GB a few seconds after starting, and can increase up to 30GB during use.

I've bisected the regression to 74620e6, which is the port to the new salsa.

I'm currently trying to capture allocations profiles using Instruments on macOS, but the instrumentation makes RA extremely slow, so it takes quite a while to run. I'll try on a subset of our codebase next, which hopefully should reproduce the issue while being faster to run, and update this issue with the results.

Particularities of our setup:

  1. We have about 700 crates, 2800 counting external dependencies.
  2. We use Bazel instead of cargo, which means we use rust-project.json (or more specifically, the ongoing discover config integration).
@alexkirsz alexkirsz added the C-bug Category: bug label Mar 20, 2025
@alexkirsz
Copy link
Contributor Author

Here are two Instruments.app Allocations traces. I had to run rust-analyzer on a small subset of our workspace (~80 crates), as otherwise running the instrumentation would have taken hours on the full workspace. Even on ~10% of the workspace, Instruments is no longer able to save/reopen the .trace files it creates.

On this subset, I'm only seeing a 2x increase in memory usage, but hopefully it is representative of the underlying issue(s).

The first trace is running on 394374e, the second is running on 74620e6 (after #18964). The traces are very large (10GiB), so I've gzipped them.

  1. 705,48MiB
  2. 1,60GiB

Please let me know if there are better ways to run memory/heap allocations profiling for RA!

@davidbarsky
Copy link
Contributor

Oh boy, 30 gigabytes is suboptimal. As least temporarily, can you try disabling cache priming in your editor? Here's the configuration you'll need in Zed:

 "lsp": {
    "rust-analyzer": {
      "initialization_options": {
        "cachePriming": {
          "enable": false
        }
      }
    }
  },

Slightly longer term, I think I have a sense as to what's going on: I think cache priming ("Indexing") doesn't increment the revision counter, so we never actually garbage collect anything. cc: @Veykril—how wrong am I about this theory?

@alexkirsz
Copy link
Contributor Author

alexkirsz commented Mar 20, 2025

@davidbarsky After disabling cache priming, rust-analyzer now only takes 5.5GiB of memory. It's also much faster to start, so I think I'm never going back 😄

Thanks for the quick reply. I had built an older version of RA for the team, but this completely unblocks us for now.

EDIT: Okay, the "much faster to start" part might have been placebo, I'll need to run more comparisons, but it does feel noticeably faster than before the salsa migration.

@davidbarsky
Copy link
Contributor

@davidbarsky After disabling cache priming, rust-analyzer now only takes 5.5GiB of memory. It's also much faster to start, so I think I'm never going back 😄

I'm guessing you already discovered this, but disabling cache priming means that startup is faster, but that faster startup time is paid for by certain operations like symbol search or go-to-def being slightly slower because, well—the indexes they rely on haven't been built yet. We might need to fix stick a Database::trigger_lru_eviction

EDIT: Okay, the "much faster to start" part might have been placebo, I'll need to run more comparisons, but it does feel noticeably faster than before the salsa migration.

See the disclaimer above, but I've noticed that new Salsa is slightly faster on some projects even with cache priming. My suggestion to disable cache priming was intended to buy us time in order to run garbage collection during cache priming.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: bug
Projects
None yet
Development

No branches or pull requests

2 participants