Skip to content
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

Send textDocument/didChange for groups of edits, not one per each change. #362

Closed
yyoncho opened this issue May 17, 2018 · 5 comments
Closed
Labels

Comments

@yyoncho
Copy link
Member

yyoncho commented May 17, 2018

Possible improvement for lsp-mode as per https://github.com/joaotavora/eglot .

@yyoncho
Copy link
Member Author

yyoncho commented Dec 11, 2018

I did performance testing using php/rust/java and it seems there is no any performance implication related to sending not grouping the updates. Since VScode is sending the updates one by one, it looks like the server are optimized to do not perform build or any heavy operation on key press but they already group the updates on server side. In my tests the fontlock was taking 50% of the CPU time while the LSP code less than 1%.

@MaskRay what do you think from CCLS point of view, do you rebuild on each didChange notification?

@alanz
Copy link
Contributor

alanz commented Dec 12, 2018

An earlier issue #112 deals with trying to batch the changes, with the conclusion that it is not possible to do it based on the change notification process in emacs.

So it is up to the server to "batch up" changes, and only do the diagnostic processing per batch.

For example, in the haskell-ide-engine we wait for 350ms without a change message before triggering diagnostics.

@yyoncho
Copy link
Member Author

yyoncho commented Dec 12, 2018

@alanz

I wonder whether it will pay off to implement the other option listed in #112

Adjust the edit locations so that they all refer to the original document, so they can be applied according to the specification.

My conclusion for now is like yours - Won't fix - due to the fact that the servers do buffer the updates and also the sending itself is fast (~1000 in second on 5 year old PC).

@yyoncho
Copy link
Member Author

yyoncho commented Dec 12, 2018

I am closing this for now. Later, we may do complete performance testing and reevaluate this decision.

@yyoncho yyoncho closed this as completed Dec 12, 2018
@MaskRay
Copy link
Member

MaskRay commented Dec 13, 2018

Yeah this should be done on the server side and AFAIK ccls clangd haskell-ide-engine have implemented debounce.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants