search

nvim-lua / lsp-status.nvim

Utility functions for getting diagnostic status and progress messages from LSP servers, for use in the Neovim statusline
MIT License
621 stars 41 forks source link

Refactor code to improve performance #42

Closed joshuachp closed 3 years ago

joshuachp commented 3 years ago

Changed the status line function to just get a buffer variable for the status line and delegate the update of this variable to the lsp_handlers

joshuachp commented 3 years ago

Thanks for this! I think this is a good direction to take, but there are a few issues with the current implementation. First, we would ideally not call the (relatively expensive) get_lsp_statusline function on every message update. I'm playing around with an alternative to this. Second, this seems to break messages with other language servers for me - for example, with Sumneko's Lua LSP, the messages just freeze.

I'll try to fix things up this weekend and ping you for review. Thanks again!

I'm happy to help, could you elaborate on the issue? I found that the last message on some servers are not updated with ✔️ until you change function and so issue another update.

joshuachp commented 3 years ago

I've added the timer to defer the get_lsp_status call every 100ms. It uses the vim buffer variables as a global state to know when there is an update. Which probably isn't good but it's working.

Also I noticed the statusline text creates an array and then concatenates all the elements together, which is more work than concatenating each element to a sting.

wbthomason commented 3 years ago

Thanks again for this. I think the issue I was seeing is what you mention, that the last message is not updated until triggering another event.

I've added the timer to defer the get_lsp_status call every 100ms. It uses the vim buffer variables as a global state to know when there is an update. Which probably isn't good but it's working.

Thanks, I'll take another look at the design.

Also I noticed the statusline text creates an array and then concatenates all the elements together, which is more work than concatenating each element to a sting.

Yes, this is more expensive than a fixed, known number of concatenations, but is more convenient and more efficient than concatenating in a loop (ref: https://gitspartv.github.io/LuaJIT-Benchmarks/#test18).

wbthomason commented 3 years ago

Oh, minor nitpick: can I request that you rename this branch to refactor/throttle-messaging? refactor is a bit vague, and - while I think I could change the branch name on my end - I think it'll be easier from yours. Thanks!

wbthomason commented 3 years ago

One suggestion, though I haven't gone through the changes in depth yet: what if, instead of a constantly running timer, we test if the time since last update is within one interval-length of the current time, and issue a redrawstatus! if it is not? That would simplify the flow of the code a bit, and potentially save on computation (though probably not beyond the level of a micro-optimization).