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

Redesign rust-analyzer::flycheck #18186

Open
alibektas opened this issue Sep 25, 2024 · 2 comments
Open

Redesign rust-analyzer::flycheck #18186

alibektas opened this issue Sep 25, 2024 · 2 comments
Assignees
Labels
A-flycheck issues with flycheck a.k.a. "check on save"

Comments

@alibektas
Copy link
Member

alibektas commented Sep 25, 2024

The current implementation of flycheck is flawed in that it instantiates at most a single "flycheck instance" for each workspace. With the introduction of rust-analyzer.toml files, more fine-grained configuration has become possible. Therefore, we need more flexibility in flycheck instances. At the same time, we want to avoid having many `flycheck' threads, as this is clearly inefficient.

Another issue that also needs to be addressed is how hard it is to test any kind of flycheck activity. The new model should make testing less of a problem for us.

Some key points about the new approach

  • A thread pool will be used to avoid unnecessary initialization/destruction of threads. The number of threads will be configurable.
  • Threads are used to execute whatever flycheck op is waiting to be executed. To map the results of a flycheck' instance to the corresponding workspace, it was previously sufficient to keep a workspace id'. The new model could use a hash of workspace_id * target * other things I can't think of to compute an id.
  • More points to come in the next few days.
@alibektas alibektas self-assigned this Sep 25, 2024
@alibektas alibektas added the A-flycheck issues with flycheck a.k.a. "check on save" label Sep 25, 2024
@Veykril
Copy link
Member

Veykril commented Sep 26, 2024

Dumping some quick notes/thoughts:

Current architecture:

  • "spawn" one flycheck worker per loaded workspace, we eagerly built the command at this stage
  • on initial project load finishing, instruct all workers to check their full workspace
  • Whenever a file is saved
    • figure out which crates the file belongs to
      • then, instruct all flycheck workers of workspaces that contain any of the crates to flycheck either:
        • the relevant package belonging to the crate (if the workspace c onfig is not set)
        • or the full workspace otherwise
    • if no flycheck was triggered, isntruct all workers to check their full workspace

Notably, this has the downside that we can only check a single package (or target even) per workspace. We also can bring down the system if there are a lot of workspaces and we check all of them at once.

Also, we do want to keep the property of having at most one flycheck worker run for a given workspace (multiple is non-sensical as they will block on another for the cargo lock anyways).

So maybe a job queue isn't actually needed and we can keep the per workspace workers, from the looks of it we mainly need to make them more flexible wrt. the command they invoke?

@joshka
Copy link
Contributor

joshka commented Feb 7, 2025

Add

Taking a big picture look at this, I think I'd look at this as solving the big hairy goal of how to optimize the developer inner loop. To me, that means where possible, low latency results (on every keypress, not on save) for all the activities that a developer needs to focus on, while simultaneously prioritizing the most important ones (e.g. run impacted unit tests first, run recently failing tests first, run fast tests first, run clippy concurrently), in combination with showing the most important information up front (i.e. I rarely ever want to see clippy warnings when there's errors, and I probably never want to see these if there's unit test failures / build errors).

Put more plainly, I'd like to stay in flow, while having my machine do all the things which are required as part of delivering quality software (build, test, lint, etc.)

Maybe this could be expanded out to be a worthy GSoC project for someone?

So maybe a job queue isn't actually needed and we can keep the per workspace workers, from the looks of it we mainly need to make them more flexible wrt. the command they invoke?

I think we probably do need a queue, but it's more likely a priority queue than a linear one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-flycheck issues with flycheck a.k.a. "check on save"
Projects
None yet
Development

No branches or pull requests

3 participants