Conversation
Implements a high-performance LSP proxy in Rust to replace the Node.js version, eliminating the 50MB runtime dependency. The proxy forwards LSP messages between Zed and JDTLS while sorting completion items by parameter count. Key improvements: - 2.5x faster message processing (13µs vs 33µs average) - 771KB static binary vs 50MB Node.js runtime - Cross-platform CI builds for all supported architectures - HTTP server for extension requests with 5s timeout - Parent process monitoring to prevent orphaned JDTLS instances
Move Unix and Windows parent monitoring implementations into separate `platform` modules
|
Some additional info about the new proxy.
A comparison between Node and Rust powered by Opus 4.6 LSP Proxy Benchmark: Node.js vs RustOverviewThis report compares the performance of the existing Node.js LSP proxy ( Methodology
Test Environment
ResultsNode.js Proxy (3,700 messages)
Total overhead: 124,796 µs (~125 ms) Rust Proxy (5,277 messages)
Total overhead: 72,026 µs (~72 ms) Head-to-Head Comparison (Median)
Tail Latency Comparison (P99)
Analysis
Personal touch
|
playdohface
left a comment
There was a problem hiding this comment.
This all looks very reasonable to me, and works without issues on my system, including the debugger 🚀
Things that I had on my mind while reviewing this
- it might be good to include some way to log from inside the proxy for debugging (I think sending a "window/logMessage" is going to make it show up in Zeds Server Logs).
eprintln!will only be shown if we fail to start as far as i can tell - proxy::main could likely benefit from splitting, but for now it's perhaps better as one file, to stay in line with the old one.
Ah that's nice, let me include that
I was indeed following the old one as a template but we are no longer limited to a single file. |
|
@playdohface I've added logging following the proposal LSP 3.18 version. DEBUG is for now included with LOG but in case of approval we are ready. Verified logs emission by checking them under LSP logs --> Server Logs and filtering between the available log levels. I've also split main into submodules. Let me know if there's something else that could be added |
playdohface
left a comment
There was a problem hiding this comment.
Industrial grade logging 👍
Not sure if it needs to be that comprehensive but might as well
| if [ "${{ matrix.cross }}" = "true" ]; then | ||
| cross build --release --target ${{ matrix.target }} | ||
| else | ||
| cargo build --release --target ${{ matrix.target }} | ||
| fi |
There was a problem hiding this comment.
Think there might be something missing here?
There was a problem hiding this comment.
Something might indeed be off Are you hinting at cross usage? In that case it is required for cross compilation in Linux env for aarch64 support
| // Spawn JDTLS (use shell on Windows for .bat files) | ||
| let mut cmd = Command::new(bin); | ||
| cmd.args(child_args) | ||
| .stdin(Stdio::piped()) | ||
| .stdout(Stdio::piped()) | ||
| .stderr(Stdio::inherit()); | ||
|
|
||
| #[cfg(windows)] | ||
| if bin.ends_with(".bat") || bin.ends_with(".cmd") { | ||
| cmd = Command::new("cmd"); | ||
| cmd.arg("/C") | ||
| .arg(bin) | ||
| .args(child_args) | ||
| .stdin(Stdio::piped()) | ||
| .stdout(Stdio::piped()) | ||
| .stderr(Stdio::inherit()); |
There was a problem hiding this comment.
Can we not just conditionallly compile the entire command? Also, are we sure that the command will not spawn a window on Windows? Ref https://github.com/zed-industries/zed/blob/3864b736eaa332f9dbe88831e75242f050a2f699/crates/util/src/command.rs#L13-L25
There was a problem hiding this comment.
The additional check for bat or cmd is required in the instance the jdtls launcher is provided by the user.
About the window spawned, I haven't received any feedback about it but I'll try to see if the flag is required.
There was a problem hiding this comment.
I did some digging and the flag to prevent a window to be spawned is not required.
The child process will inherit the parent's console. As the proxy itself was spawned without a window the only way for the child to spawn a window would be to override the config.
There was a problem hiding this comment.
We could perhaps consider to just make the extension a "proper" Rust workspace? Not sure it is worth it though (or causes some other issues), but might be a bit nicer to work with.
There was a problem hiding this comment.
I agree, it is annoying to have two windows open, one with the extension as root and one with the proxy.
Let me see whether the extension will work correctly even with that enabled.
There was a problem hiding this comment.
Rust-analyzer without it wasn't able to recognize the active code behind cfg. I'm open to better options. The idea otherwise was to provide all possible values as comment and ask the maintainer to select the proper system.
This PR introduces a new proxy written entirely in Rust.
The proxy sits between Zed and JDTLS, forwarding LSP messages bidirectionally, sorting completion responses by parameter count, and exposing an HTTP server for extension-originated requests.
Motivation
Other changes included
justfilewith dev commands (proxy-build,proxy-install,ext-build,fmt,clippy,all)