blahgeek/emacs-lsp-booster

Got it working with eglot

jdtsmith opened this issue ยท 15 comments

Update: I turned this into a tiny package which you should use instead of the gist.

I threw together a small proof-of-concept to use this booster-wrapper with eglot. Works so far in limited testing. People with slow/chatty servers should give a try.

Thank you so much! I will add a link to your gist in readme.

Hopefully some people can test it and ideally get some benchmarks together. Once it's validated I'd be happy to have it included with your code however you like. BTW, Eglot sends "content headers" and expects

The remote endpoint is expected to understand JSONRPC messages with basic HTTP-style enveloping headers such as "Content-Length:".

Since that's on the send from emacs direction only, I presume that's fine, but I'm not sure how standard it is to include these headers prior to the JSON payload.

The "Content-Length" header is required and is properly handled by this wrapper.

The "Content-Type" header is optional according to the spec: https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#headerPart This wrapper should be able to read json with or without this header, and will always write json without this header. It should be OK if both sides are compliant to the spec.

Thanks a lot for this! Is there an easy way to benchmark its performance? (Not so familiar with Eglot/LSP backends)

Did you see the builtin low level test benchmark results @blahgeek has implemented? E.g. of this example completions response. How that translates to real world performance increase is hard to say, since poor performance probably relies on jamming up the IO buffers so things start to drag. See #3 for some ideas on that.

There is a potential issue: in the original jsonrpc--json-read, it sets :false-object :json-false which translates json "false" to :json-false symbol; while emacs-lsp-booster translates it into nil (like what lsp-mode would want). I would need to add an option in emacs-lsp-booster to support this

My eglot gist simply calls the original function that jsonrpc sets up, which is (for most people):

(json-parse-buffer :object-type 'plist
                           :null-object nil
                           :false-object :json-false)

Update: sorry I misunderstood. Yes I suppose the :json-false symbol needs to be retained when false is translated. Let me know when you've added an option and I'll update the gist. Maybe --false-object=:json-false would work?

BTW, down the road if this approach pans out, it would be worth approaching the eglot/lsp-mode people to see about adding official support. Eglot can read from network ports, so it might be necessary to add that capability. For remote servers, doing the bytecode translation on the remote end would improve throughput.

I pushed a new version. emacs-lsp-booster --json-false-value :json-false -- <cmd...> should work. See emacs-lsp-booster --help

Great, thanks for the quick turnaround. Updated the gist and it seems to work well. I also noticed you started adding some testing flags; very nice.

BTW, in the log it says false_value: Symbol("json-false"). By symbol do you mean keyword (which is just a symbol with a leading :)? jsonrpc requires a keyword, not a "plain" symbol.

BTW, in the log it says false_value: Symbol("json-false"). By symbol do you mean keyword (which is just a symbol with a leading :)? jsonrpc requires a keyword, not a "plain" symbol.

Ah right, it's a bug, it should be "keyword". Thanks for catching that. Fixed in latest commit.

As long as it is sending :json-false and not json-false it will work.

Just tested new build and it works fine, but the only :json-false in my server's event log is from the client initialize request, so it doesn't seem I'm really testing it. Maybe others can give a look.

I turned this into a tiny package: eglot-booster instead of the gist. It's now a minor-mode which can be enabled/disabled during the same Emacs session (great for comparing before/after boost).

Please give a try and report any issues there. We can modify the README to point to eglot-booster soon.

We can modify the README to point to eglot-booster soon.

done.

closing this issue.