-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
POC: Rust LSP #7262
POC: Rust LSP #7262
Conversation
61cd0be
to
56c62bc
Compare
Current dependencies on/for this PR:
This comment was auto-generated by Graphite. |
56c62bc
to
729a4fc
Compare
CodSpeed Performance ReportMerging #7262 will improve performances by 2.9%Comparing Summary
Benchmarks breakdown
|
fafee18
to
02c6ea2
Compare
Signed-off-by: Micha Reiser <[email protected]>
02c6ea2
to
a670128
Compare
This is a very nice writeup. What did you end up deciding on? |
Thanks. We have yet to decide. We hope that @inquisitivepenguin can work and this soon. Are you working on something similar? |
@MichaReiser Yes, I'm working on a language server for Fe language. I recently prototyped a switch from lsp-server to tower-lsp and I've documented some of my thoughts so far here: ethereum/fe#979 Long story short, it seems that there's no way of making concurrency problems magically simple. Waving We are using salsa2022 in our compiler libraries, which alleviates some of the pressure of getting this stuff perfect. A lot of computations will be reused as long as inputs haven't changed. |
@micahscopes Thanks for sharing your findings and the pull request. It will help us evaluating tower-lsp and lsp-server.
I didn't expect that. There's an inherent complexity that the LSP protocol allows multiple in-flight requests. I'm mainly wondering if there's an opportunity to avoid some complexity introduced by async and await (and async runtimes) because the LSP-server can give us more fine-grained control of when we want to run something on different threads to avoid concurrency-safe data structures potentially. The reason why I feel that async (or at least tokio) might not be the right fit is because most of our operations aren't IO bound and, therefore, shouldn't run in a regular tokio worker. That said. I'm sure your PR will help me understand why the trade of more fine grained control doesn't outweigh the first-class language integration that you get with async await. |
Exactly, regardless of lsp-server vs tower-lsp, it's still necessary to face this complexity! Today I'm feeling curious about using tower-lsp along with separate thread(s) for long running tasks.
Using async/await wouldn't necessarily preclude that kind of fine-grained control, right? For example, if you wanted some state to be on a thread and didn't want to share it, you could still use channels to communicate to/from tower-lsp handlers, couldn't you? I.e. the channel endpoints would be the only "shared state" you allow in the tower-lsp object. But I hear your concern about the complexity of async/await/executors. Is it possible to create something maintainable and easy to fix if things go wrong? Does async/await still provide advantages even when using explicit channels and threads? By the way, you've probably seen this already but I found this to be very helpful: https://tokio.rs/tokio/tutorial/shared-state. |
In developing the Grit language server we faced some similar challenges with the async language server. We ended up moving all long-running or compute-intensive work to a separate thread which we communicate with via channels. So far this is working nicely, but you do need to do a bit of work to make sure state messages aren't sent to the client. |
Hy @morgante 👋 Oh that's interesting. Do I understand it correctly that you used the That sounds very clever, making use of the async-await ergonomics where it matters but avoid it "infecting" all the downstream infrastructure and keeping some control over scheduling. |
Exactly, with the caveat that we do still keep some very fast things (ex. looking up available patterns) in the main LSP thread. |
@morgante thank you for sharing your experience, it's encouraging to hear that you've made this work. Async/await appeals to me in that it simplifies keeping track of pending requests and related responses; no need for extra data structures to keep track of pending request IDs or any of that.
Can you elaborate on the part about preventing state messages from being sent to the client? I don't understand. |
Depending on how long async execution takes, documents will often have changed before the results are sent to the client. The |
<!-- Thank you for contributing to Ruff! To help us out with reviewing, please consider the following: - Does this pull request include a summary of the change? (See below.) - Does this pull request include a descriptive title? - Does this pull request include references to any relevant issues? --> ## Summary This PR introduces the `ruff_server` crate and a new `ruff server` command. `ruff_server` is a re-implementation of [`ruff-lsp`](https://github.com/astral-sh/ruff-lsp), written entirely in Rust. It brings significant performance improvements, much tighter integration with Ruff, a foundation for supporting entirely new language server features, and more! This PR is an early version of `ruff_lsp` that we're calling the **pre-release** version. Anyone is more than welcome to use it and submit bug reports for any issues they encounter - we'll have some documentation on how to set it up with a few common editors, and we'll also provide a pre-release VSCode extension for those interested. This pre-release version supports: - **Diagnostics for `.py` files** - **Quick fixes** - **Full-file formatting** - **Range formatting** - **Multiple workspace folders** - **Automatic linter/formatter configuration** - taken from any `pyproject.toml` files in the workspace. Many thanks to @MichaReiser for his [proof-of-concept work](#7262), which was important groundwork for making this PR possible. ## Architectural Decisions I've made an executive choice to go with `lsp-server` as a base framework for the LSP, in favor of `tower-lsp`. There were several reasons for this: 1. I would like to avoid `async` in our implementation. LSPs are mostly computationally bound rather than I/O bound, and `async` adds a lot of complexity to the API, while also making harder to reason about execution order. This leads into the second reason, which is... 2. Any handlers that mutate state should be blocking and run in the event loop, and the state should be lock-free. This is the approach that `rust-analyzer` uses (also with the `lsp-server`/`lsp-types` crates as a framework), and it gives us assurances about data mutation and execution order. `tower-lsp` doesn't support this, which has caused some [issues](ebkalderon/tower-lsp#284) around data races and out-of-order handler execution. 3. In general, I think it makes sense to have tight control over scheduling and the specifics of our implementation, in exchange for a slightly higher up-front cost of writing it ourselves. We'll be able to fine-tune it to our needs and support future LSP features without depending on an upstream maintainer. ## Test Plan The pre-release of `ruff_server` will have snapshot tests for common document editing scenarios. An expanded test suite is on the roadmap for future version of `ruff_server`.
…sh#10158) <!-- Thank you for contributing to Ruff! To help us out with reviewing, please consider the following: - Does this pull request include a summary of the change? (See below.) - Does this pull request include a descriptive title? - Does this pull request include references to any relevant issues? --> ## Summary This PR introduces the `ruff_server` crate and a new `ruff server` command. `ruff_server` is a re-implementation of [`ruff-lsp`](https://github.com/astral-sh/ruff-lsp), written entirely in Rust. It brings significant performance improvements, much tighter integration with Ruff, a foundation for supporting entirely new language server features, and more! This PR is an early version of `ruff_lsp` that we're calling the **pre-release** version. Anyone is more than welcome to use it and submit bug reports for any issues they encounter - we'll have some documentation on how to set it up with a few common editors, and we'll also provide a pre-release VSCode extension for those interested. This pre-release version supports: - **Diagnostics for `.py` files** - **Quick fixes** - **Full-file formatting** - **Range formatting** - **Multiple workspace folders** - **Automatic linter/formatter configuration** - taken from any `pyproject.toml` files in the workspace. Many thanks to @MichaReiser for his [proof-of-concept work](astral-sh#7262), which was important groundwork for making this PR possible. ## Architectural Decisions I've made an executive choice to go with `lsp-server` as a base framework for the LSP, in favor of `tower-lsp`. There were several reasons for this: 1. I would like to avoid `async` in our implementation. LSPs are mostly computationally bound rather than I/O bound, and `async` adds a lot of complexity to the API, while also making harder to reason about execution order. This leads into the second reason, which is... 2. Any handlers that mutate state should be blocking and run in the event loop, and the state should be lock-free. This is the approach that `rust-analyzer` uses (also with the `lsp-server`/`lsp-types` crates as a framework), and it gives us assurances about data mutation and execution order. `tower-lsp` doesn't support this, which has caused some [issues](ebkalderon/tower-lsp#284) around data races and out-of-order handler execution. 3. In general, I think it makes sense to have tight control over scheduling and the specifics of our implementation, in exchange for a slightly higher up-front cost of writing it ourselves. We'll be able to fine-tune it to our needs and support future LSP features without depending on an upstream maintainer. ## Test Plan The pre-release of `ruff_server` will have snapshot tests for common document editing scenarios. An expanded test suite is on the roadmap for future version of `ruff_server`.
Rust LSP
This PR adds a Rust-based LSP to ruff. You can start it by running
ruff lsp
. It receives messages overstdin
and writes responses overstdout
.stderr
is used for logging.Getting started
editors/vscode
)nom install
ineditors/vscode
F5
) or Run and Debug / Run Extension"ruff.lspBin": "<path>/target/debug/ruff"
Advantages
Development tools
ruff.trace.server
setting toverbose
to gettrace
leveltracing
in theOutput: Ruff
panel and see all messages between client and server in the newOutput: Ruff Trace
panel.Implemented functionality
Challenges and open questions
Tower-LSP or LSP-server
The most known crates for building an LSP server with rust are tower-lsp and lsp-server. This prototype uses tower-lsp, but I think it's worth considering lsp-server.
tower-lsp
async
. Usestokio
by defaultinitialize
messages. Not accepting messages aftershutdown
.lsp-server
async
Rust, which is less useful for code analysis because most tasks are CPU and not IO bound.Example
VS Code Extension migration
This prototype doesn't explore how to support the old Python-based LSP and the ruff-integrated LSP in a single extension. It's further unclear how the new LSP would support the
args
setting (The setting is already problematic today becauseformat
andcheck
don't support the same arguments).A possible approach could be that the extension detects the version of the ruff binary and either spawns
ruff-lsp
(Python LSP) orruff lsp
(Rust based LSP)Ruff binary discovery
This prototype doesn't explore how to discover the
ruff
binary for the current project (and using the bundled version for untrusted workspaces). It defaults toruff.lspBin
.Higher level
lint
andformat
abstractionsThe
ruff_workspace
crate provides most functionality needed by the LSP, but not with the right granularity and laziness. This either results in code duplication between the CLI and the LSP or unnecessary work:PyprojectConfig::resolve
andpython_files_in_path
.python_files_in_path
resolves all settings and python files included in the project. However, the LSP only needs the settings. Building up a list of all files in the project is wasted work.Setting
for a path would be resolved lazily rather than eagerly.python_files_in_path
)To sum it up. We lack high-level APIs for lazy linting or formatting of a file to avoid duplicating concerns performed by the CLI.
Concurrency
The fact that tower-lsp uses tokio forces the implementation to use thread-safe (
Send
andSync
) data structure for the session state. This makes accessing and mutating the configuration awkward and introduces much complexity. It would be nice if we could minimize the need for thread-safe data structures and locking.However, this problem isn't only caused by tower-lsp. I see us facing similar issues when implementing multi-file analysis where we suddenly need a way to snapshot the project state and run the analysis on the snapshot (to avoid new user modifications causing inconsistent states).
Notes
It isn't necessary to integrate the VS code extension into this repository. I added it to this repository to have a single PR that includes all changes (from plugin to ruff)