Skip to content

Moved LSP main loop to use tokio.#11036

Merged
anlumo merged 27 commits intomasterfrom
anlumo/lsp-tokio
Apr 2, 2026
Merged

Moved LSP main loop to use tokio.#11036
anlumo merged 27 commits intomasterfrom
anlumo/lsp-tokio

Conversation

@anlumo
Copy link
Copy Markdown
Contributor

@anlumo anlumo commented Mar 18, 2026

@anlumo anlumo requested review from LeonMatthes and ogoffart March 18, 2026 15:03
@anlumo anlumo self-assigned this Mar 18, 2026
Comment thread tools/lsp/Cargo.toml Outdated
Comment thread tools/lsp/preview/connector/native.rs Outdated
Comment thread tools/lsp/main.rs Outdated
Comment thread tools/lsp/main.rs Outdated
Comment thread tools/lsp/main.rs Outdated
Copy link
Copy Markdown
Member

@LeonMatthes LeonMatthes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO, I think this is going in a good direction, it's a much less invasive change, but allows us to use normal async, which was the goal. 👍

Comment thread tools/lsp/main.rs Outdated
Comment thread tools/lsp/main.rs Outdated
Comment thread tools/lsp/main.rs Outdated
@anlumo anlumo force-pushed the anlumo/lsp-tokio branch 3 times, most recently from f4a09ae to 965802b Compare March 20, 2026 09:46
@anlumo anlumo marked this pull request as ready for review March 23, 2026 17:57
@anlumo anlumo requested review from LeonMatthes and ogoffart March 23, 2026 17:58
@anlumo anlumo force-pushed the anlumo/lsp-tokio branch from 1827475 to 8a8f9f6 Compare March 23, 2026 18:29
Comment thread tools/lsp/preview/connector/native.rs
Comment thread tools/lsp/Cargo.toml
Copy link
Copy Markdown
Member

@LeonMatthes LeonMatthes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is going in a good direction. Some nitpicks left.

What I am concerned about most is our continued use of RefCell.
That seems incorrect in an async world (even before this change tbh).
Any ideas how we want to deal with this?

Comment thread tools/lsp/preview/connector/native.rs Outdated
Comment thread tools/lsp/preview/connector/native.rs
Comment thread tools/lsp/main.rs Outdated
Comment thread tools/lsp/main.rs Outdated
@anlumo
Copy link
Copy Markdown
Contributor Author

anlumo commented Mar 24, 2026

@LeonMatthes I agree that RefCell/Mutex/RwLock is bad code style and can cause a lot of troubles. However, the proper fix to this is to use channels, which means a major refactoring. This is exactly what I tried to do with the rewrite to async_lsp and a major reason for the large number of added lines in that PR. Maybe we can find a way to get smaller PRs that change to a new async architecture step by step?

@anlumo anlumo force-pushed the anlumo/lsp-tokio branch 2 times, most recently from 31df97b to 34f4f41 Compare March 25, 2026 15:25
@anlumo anlumo linked an issue Mar 25, 2026 that may be closed by this pull request
@anlumo anlumo force-pushed the anlumo/lsp-tokio branch 2 times, most recently from f78b2a7 to f053d13 Compare March 26, 2026 09:52
Copy link
Copy Markdown
Member

@LeonMatthes LeonMatthes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I'm most curious about is why not continue using the fine-grained locking on the Context and use a large-scale lock. What is the advantage of that?

Apart from that it's mostly nitpicks left. 👍

Comment thread tools/lsp/main.rs Outdated
Comment thread tools/lsp/main.rs Outdated
Comment thread tools/lsp/main.rs Outdated
Comment thread tools/lsp/main.rs Outdated
Comment thread tools/lsp/language.rs Outdated
Comment thread tools/lsp/preview/connector/native.rs
Comment thread tools/lsp/fmt/writer.rs Outdated
Comment thread tools/lsp/preview/connector/native.rs
Comment thread tools/lsp/language.rs Outdated
Comment thread tools/lsp/main.rs Outdated
Comment thread tools/lsp/main.rs Outdated
@anlumo anlumo force-pushed the anlumo/lsp-tokio branch 5 times, most recently from a7a03af to dfaf3e5 Compare March 27, 2026 12:46
@anlumo anlumo requested review from LeonMatthes and ogoffart March 27, 2026 15:22
@anlumo anlumo force-pushed the anlumo/lsp-tokio branch from dfaf3e5 to 0fdd100 Compare March 30, 2026 17:22
anlumo and others added 25 commits April 1, 2026 19:14
Co-authored-by: Leon Matthes <leon.matthes@slint.dev>
… by a lot.

Since we're single-threaded anyways, this shouldn't have any negative performance impact.
Copy link
Copy Markdown
Member

@LeonMatthes LeonMatthes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super cool that you managed to get rid of the Rc/RefCell situation all together 👍

As discussed this linearizes all requests at the moment, but as we discussed, that's not necessarily a bad thing, as this was all single-threaded anyhow.

@anlumo anlumo merged commit d451604 into master Apr 2, 2026
53 checks passed
@anlumo anlumo deleted the anlumo/lsp-tokio branch April 2, 2026 12:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Move LSP to use async

3 participants