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

Lazy Workspace/Project Discovery #17537

Open
davidbarsky opened this issue Jul 3, 2024 · 2 comments
Open

Lazy Workspace/Project Discovery #17537

davidbarsky opened this issue Jul 3, 2024 · 2 comments
Labels
A-project-model project model and workspace related issues C-Architecture Big architectural things which we need to figure up-front (or suggestions for rewrites :0) ) E-medium

Comments

@davidbarsky
Copy link
Contributor

Few notes:

Anyway! Project discovery should be entirely lazy. This change makes the following easier:

  • Monorepos. Projects are often discovered incrementally as the user navigates around a monorepo and it doesn't make sense to do Cargo-style project discovery at startup.
  • Standalone files, like rustlings. Users would be able to just open a Rust file, and through a rust-analyzer.toml in the rustlings repo, IDE functionality would just work for them.
  • Cargo scripts, which have a similar dynamic to that of rustlings/monorepos but scaled down from the latter.

To make this change happen, the currently eager (that is, they occur on startup/workspace folder change) ProjectManifest::discover_all + cargo metadata-style operations would become lazy, rust-analyzer.workspace.DiscoverCommand-style operations that only happen after startup. This would mean several things:

  • Project discovery/indexing wouldn't start until a user opens a Rust file.
  • cargo_metadata::MetadataCommand::new() would become the default mode for flycheck/src/json_workspace.rs (see this comment. That file would no longer be JSON workspace-specific, but it would also make "project discovery" a first-class concept in rust-analyzer.
  • rust-analyzer.linkedProjects would technically be lazily evaluated, but if any value is set, it would effectively be "eagerly" evaluated.
  • The current "rust-analyzer only searches two directory levels down for a Cargo.toml" behavior can be removed in favor of "run cargo-metadata in the parent of the rust file"-esque behavior, which newer Rust users often struggled with and complained about.

To support this change, I think three things need to happen:

  • feature: teach rust-analyzer to discover linked_projects #17246 needs to land.
  • The crate graph should be lifted into a standalone, Salsa database.
    • Salsa's interning infrastructure should be used with the crate data as the "key". This is necessary in order to support different feature flags/versions across projects.
  • A nice performance bonus, the VFS should be able to load all a project's files in a single go.
    • Today, rust-analyzer doesn't have a meaningful distinction between the user-facing "startup" and "steady-state, using-the-IDE" phases. It is always already to incrementaly update and rebuild the crate graph, which it does many times during project loading. I think it is worthwhile to have this distinction because it'd then be possible to load all relevant files in a single turn extremely quickly.
    • This is particularly important on network-backed file systems, like EdenFS. I've observed 180x speedups through some naive usage of Rayon.
@davidbarsky davidbarsky added E-medium C-Architecture Big architectural things which we need to figure up-front (or suggestions for rewrites :0) ) A-project-model project model and workspace related issues labels Jul 3, 2024
@Veykril
Copy link
Member

Veykril commented Jul 3, 2024

cargo_metadata::MetadataCommand::new() would become the default mode for flycheck/src/json_workspace.rs

I'd expect these to still be different from another, unless I misunderstand this phrase makes it sound like we are unify-ing cargo and rust-project.json like projects.

The crate graph should be lifted into a standalone, Salsa database.

I don't think this is necessary for this change. It is necessary to support standalone files, but that is a separate issue that can follow afterwards.

A nice performance bonus, the VFS should be able to load all a project's files #17491 (comment).

Likewise I don't think this is necessary either, this is a separate issue as well. We can implement lazy discovery without this change.

@davidbarsky
Copy link
Contributor Author

cargo_metadata::MetadataCommand::new() would become the default mode for flycheck/src/json_workspace.rs

I'd expect these to still be different from another, unless I misunderstand this phrase makes it sound like we are unify-ing cargo and rust-project.json like projects.

They are different, sorry! I'm trying to say that these would go through similar codepaths/mechanisms, as opposed to being fully distinct today.

The crate graph should be lifted into a standalone, Salsa database.

I don't think this is necessary for this change. It is necessary to support standalone files, but that is a separate issue that can follow afterwards.

It's not, strictly speaking, necessary for #17246, but it would simply the currently complicated state machine.

A nice performance bonus, the VFS should be able to load all a project's files #17491 (comment).

Likewise I don't think this is necessary either, this is a separate issue as well. We can implement lazy discovery without this change.

Same thing: it's not really required, but I really think it makes a lot of the subtle bugs would crop up substantially easier to reason about as a result.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-project-model project model and workspace related issues C-Architecture Big architectural things which we need to figure up-front (or suggestions for rewrites :0) ) E-medium
Projects
None yet
Development

No branches or pull requests

2 participants