-
Notifications
You must be signed in to change notification settings - Fork 3
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
Minor doc improvements #70
base: master
Are you sure you want to change the base?
Conversation
20cf5bb
to
4ede7b2
Compare
like keystrokes, mouse movement, clicks, etc. and also communicate heavily | ||
with the LSP server to send the current document text and what kind of event | ||
happened. | ||
These additional tasks cause more CPU usage of the Geany process and especially |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm curious - have you seen any significantly higher Geany process CPU usage? I wouldn't expect
this plugin needs to react on various events like keystrokes, mouse movement, clicks, etc.
to be very expensive. What may be more expensive is json serialization/deserialization and all the communication with the server. I could potentially improve things a bit by not requesting some updates that often (or making the interval configurable).
I also have #3 open but I actually don't know if it is a problem or not. I kind of like showing the response from the server immediately - it feels snappier but at the cost of possibly higher CPU usage.
In a similar manner, I also request updates like symbol under cursor highlighting, document symbols, semantic tokens, etc. immediately after a keystroke or caret movement so the user visible response is quick. Only if there are subsequent keystrokes, I wait for 300ms before issuing new requests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In any case, any suggestions how to improve CPU usage are welcome.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm curious - have you seen any significantly higher Geany process CPU usage? I wouldn't expect
this plugin needs to react on various events like keystrokes, mouse movement, clicks, etc.
to be very expensive. What may be more expensive is json serialization/deserialization and all the communication with the server. I could potentially improve things a bit by not requesting some updates that often (or making the interval configurable).
I didn't measure the CPU usage or anything and apart from "pylsp", I had no performance issues.
Still, I assume the additional event handling the plugin does and, as you say, the communication with the server process will require additional CPU usage.
In a similar manner, I also request updates like symbol under cursor highlighting, document symbols, semantic tokens, etc. immediately after a keystroke or caret movement so the user visible response is quick. Only if there are subsequent keystrokes, I wait for 300ms before issuing new requests.
This is good, maybe it could be even configurable. It's probably totally fine for daily use, while testing the whole LSP stuff, I had a terminal next to Geany with stdout and stderr and saw a lot of communication which felt a bit scary but in normal usage you won't see it :).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In any case, any suggestions how to improve CPU usage are welcome.
As said above, the delay between keystrokes could be made configurable for conservative people like me.
But the idea of this PR was no critism at all, just to have a note in the Readme to point users to potentially increased CPU usage.
I think this is totally fine for users if they are aware of it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feel free to drop this PR altogether and/or reword the text as you like.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feel free to drop this PR altogether and/or reword the text as you like.
I think it's a good idea to mention higher CPU usage, I would just suggest to updat this part
this plugin needs to react on various events like keystrokes, mouse movement, clicks, etc.
and mention only keystrokes because these will result in expensive operations on the server.
Also
... to send the current document text...
is not as bad. When a LSP server supports delta updates,
"textDocumentSync" : {
"change" : 2
}
in "Server Initialize Responses", it only sends a few bytes. It's true, however, that some "less advanced" servers don't support it and then the whole document text has to be sent.
But again, the expensive part is what the server does afterwards - clangd, for instance, recompiles the current file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As said above, the delay between keystrokes could be made configurable for conservative people like me.
I've created a separate issue for it here #72
with the LSP server to send the current document text and what kind of event | ||
happened. | ||
These additional tasks cause more CPU usage of the Geany process and especially | ||
the LSP server process, so they require also more power which might be |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and especially LSP server process
From what I have seen, this is the main source of higher battery drain. Especially with clangd
which recompiles the file on every key stroke (you can see this in stderr when you enable it in the config file). Also pylsp
is ultra-slow.
Some servers may also spend too much time on certain tasks - for instance, semantic tokens and document symbols may be expensive. It might help to disable these in the config file if it's an issue. No matter what, though, the server will always have to re-parse the current source on modifications.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I gave up on pylsp
, after I got it working also with proper virtualenv integration and everything, it was sooooo slow that I had to wait 5-10s for a calltip, autocompletion was often even slower and the pylsp
process burned one CPU core all the time.
I switched to your suggestion: lsp-proxy + jedi-lsp + ruff-lsp.
This will probably work well enough.
Thanks for your suggestions!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it was sooooo slow that I had to wait 5-10s for a calltip
This is much worse than I thought - with smaller files it seems to work reasonably well. But it's definitely the slowest server I tried.
like keystrokes, mouse movement, clicks, etc. and also communicate heavily | ||
with the LSP server to send the current document text and what kind of event | ||
happened. | ||
These additional tasks cause more CPU usage of the Geany process and especially |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feel free to drop this PR altogether and/or reword the text as you like.
I think it's a good idea to mention higher CPU usage, I would just suggest to updat this part
this plugin needs to react on various events like keystrokes, mouse movement, clicks, etc.
and mention only keystrokes because these will result in expensive operations on the server.
Also
... to send the current document text...
is not as bad. When a LSP server supports delta updates,
"textDocumentSync" : {
"change" : 2
}
in "Server Initialize Responses", it only sends a few bytes. It's true, however, that some "less advanced" servers don't support it and then the whole document text has to be sent.
But again, the expensive part is what the server does afterwards - clangd, for instance, recompiles the current file.
with the LSP server to send the current document text and what kind of event | ||
happened. | ||
These additional tasks cause more CPU usage of the Geany process and especially | ||
the LSP server process, so they require also more power which might be |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it was sooooo slow that I had to wait 5-10s for a calltip
This is much worse than I thought - with smaller files it seems to work reasonably well. But it's definitely the slowest server I tried.
I thought it might be useful to explain additional CPU usage by the plugin and especially the LSP which might be relevant if using a laptop without ac.
Also added a note in the default config that
languageID
s could maybe found in the LSP's documentation or at least source code.I hope at some point there will be a better way than this :).