This repository has been archived by the owner on Dec 29, 2022. It is now read-only.
Implement Tokio-based LSP client (for testing) #1223
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Xanewok
force-pushed
the
lsp-client-lib-rebase
branch
3 times, most recently
from
96085ac to
c339b73
Compare
|
Cool! Though to be fair using an extra thread to manage stdio was never a big deal, considering we're starting up a whole RLS process. It's a good place to start pushing in tokio style async code though. |
Xanewok
force-pushed
the
lsp-client-lib-rebase
branch
from
c339b73 to
9148b8a
Compare
bors
added a commit
that referenced
this pull request
Jan 22, 2019
Translate cmd tests to use async LSP client This translates our cmd-based tests to use the LSP client introduced in #1223. Some anecdotal benchmarks (Xubuntu 18.04, Ryzen 2600) before and after translating: ``` $ time cargo test cmd_ real 0m3,498s user 0m8,379s sys 0m2,710s $ time cargo test cmd_ --release real 0m3,059s user 0m5,585s sys 0m2,512s $ time cargo test client_ real 0m3,465s user 0m7,197s sys 0m1,926s $ time cargo test client_ --release real 0m3,053s user 0m4,977s sys 0m1,825s ``` It seems that synchronization overhead caused by running multiple RLS and rustc instances in-process still outweighs spawning separate processes per test 🎉 That means we don't lose performance (and actually benefit) from switching over. I plan on translating the remaining tests in tests_old as a separate PR. r? @alexheretic
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Based on #1210.
The original motivation for this work was to tackle the recent spurious failures and make sure there are no timing/racing issues, at least on the testing harness side for
rlsspawning integration tests, so this is an attempt that does not spawn a thread and without explicit sleeping, but instead runs on the current thread (Tokio runtime), uses async I/O to read the messages and can block the current thread until specified message arrives.Thanks to RLS lib being exposed, this also can send typed requests; due to notification model we block on the response and return it as part of the request function call (so it's an actual RPC) and we can naturally add timeouts per each message.
At first I wanted to only work with
mioinstead but I found myself trying to write a lot of scaffolding that's been already implemented as parts of Tokio andmio-named-pipes, so I figured I'll pull in Tokio instead (many of the transienttokio-*deps aren't needed, there's work going at tokio-rs/tokio#808 to make these optional).I'd say this was a good excuse to finally look at the async (ecosystem) in Rust! Given enough polish maybe it'd make sense to extract the async LSP client into separate crate?
@alexheretic @nrc I'd very much appreciate taking a look at it!
Also a shout out to @alexcrichton for mio-named-pipes and tokio-process, which made this use-case a breeze 😁
EDIT: Ported only a single test, can rewrite rest of this as a part of this PR or later if looks good enough.