rust-analyzer icon indicating copy to clipboard operation
rust-analyzer copied to clipboard

internal: port base-db to new Salsa

Open davidbarsky opened this issue 1 year ago • 9 comments

Trying out new Salsa in rust-analyzer by porting over base-db. I haven't fully ported it over, but given its interactions with the VFS/external files (and the plan outlined on Zulip)), I figured I'd post this for early review and feedback.

davidbarsky avatar Oct 15 '24 20:10 davidbarsky

Hmm, I'm not sure why the license check fails with

thread 'tidy::test' panicked at xtask/src/tidy.rs:182:9:
different set of licenses!
New Licenses:
  null

Missing Licenses:

Similarly, I don't know why the cross build is failing with:

error[E0463]: can't find crate for `salsa`
  --> crates/base-db/src/new_db.rs:11:5
   |
11 | use salsa::{Accumulator, Durability, Event};
   |     ^^^^^ can't find crate

I can't reproduce that issue locally.

davidbarsky avatar Oct 17 '24 15:10 davidbarsky

The license check might be failing due to the git dependency (unsure how the logic there works in that check), the cross one failing is weird

Veykril avatar Oct 17 '24 16:10 Veykril

For the license check, that should be fixed by https://github.com/salsa-rs/salsa/pull/601.

davidbarsky avatar Oct 17 '24 21:10 davidbarsky

Is there a summary of the differences between Salsa 2 and Salsa 3, for those of us who don't follow the news?

ChayimFriedman2 avatar Oct 18 '24 01:10 ChayimFriedman2

Is there a summary of the differences between Salsa 2 and Salsa 3, for those of us who don't follow the news?

Like, new Salsa and salsa-2022 or the new Salsa and the Salsa rust-analyzer uses?

davidbarsky avatar Oct 18 '24 03:10 davidbarsky

The Salsa rust-analyzer uses.

ChayimFriedman2 avatar Oct 18 '24 03:10 ChayimFriedman2

From a programming model standpoint, I wrote this document as a guide/reference. From a "what features are now possible" standpoint, new Salsa makes the following possible, or at the very least, a lot easier:

  1. Persistent/saved state of nameres. I want to try out persistent caching to make startup faster.
  2. Interned values will actually be cleaned up in new Salsa! a. New Salsa also gives out Copy values, not Clone, which pairs nicely with the new trait solver's requirements.
  3. Easy, pervasive parallelism. This would be a big deal for autocomplete!
  4. Salsa-native cycle recovery, which might make it possible to simplify some of the fixed point iterations that exist within rust-analyzer.

Not all the functionality above is currently implemented, but I feel comfortable betting that it will be shortly—a bunch of a folks are working on implementing cycle recovery, serialized state, and GC'ing interned values as I write this comment.

Anyways, porting ra over is a bit annoying, but... that's the job.

davidbarsky avatar Oct 18 '24 13:10 davidbarsky

Also don't forget forking of the database, allowing speculative execution which simplifies completion context analysis (and also makes it more robust/less flaky)

Veykril avatar Oct 18 '24 13:10 Veykril

:umbrella: The latest upstream changes (presumably #17954) made this pull request unmergeable. Please resolve the merge conflicts.

bors avatar Oct 21 '24 10:10 bors

I'm going to close this PR in favor of work I've been doing on a macro imitating the existing query_group macro: https://github.com/davidbarsky/db-ext-macro/. Few notes:

  1. That macro live in upstream Salsa; it's just easier to develop there because rust-analyzer reloads the workspace frequently. I'll open a PR against upstream Salsa by Monday.
  2. The macro mimics the original API query_group API pretty closely!
  3. I need to add support for cycle and interned annotations and figure out why the keys in an #[input]-annotated struct are being read as unused (correctly, I might add).

davidbarsky avatar Nov 01 '24 19:11 davidbarsky