Load dictionary asynchronously#3
Open
Iron-E wants to merge 9 commits into
Open
Conversation
* NOne -> None (common in Rust code) * defualt -> default (common in C# code) * Locatoin -> Location
It was previously just a big string, which needed to be parsed. This is computationally expensive compared to just returning a table.
The README says this loads in the background, but I found that what it really does is "defer" execution, and then do it in bulk. This leads to a 1s+ long stutter when opening Neovim. This commit refactors this, so that abbreviations are actually loaded in the background. I can now type smoothly without interruptions.
If there is a problem, I think we should report it properly as an `error` in `load_abbreviations`. However, we should also not make a bad user experience, and so we should use `vim.notify` to tell the user something bad happened (instead of letting the error propagate).
This has the same responsiveness characteristics, but is easier to handle (and takes much less code).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This MR updates the dictionary loading logic so that it runs asynchronously.
Motivation
The README says:
However, when starting Neovim, there is a noticeable stutter (naive profiling shows about 1-2s) while loading the abbreviations. Here is my Lazy config:
{ "swaits/thethethe.nvim", config = true, event = "InsertEnter", }With this MR, the loading happens truly asynchronously, and responsiveness is not affected (that I can tell).
Changes
The bulk of the performance improvements come from three areas:
Converting the dictionary to Lua.
Not having to parse the raw string into a table prevents extra work each startup. Though, I am unfamiliar with the reason why a string was chosen originally, so it could be there was a strong reason to use a string.
Using
coroutines to arrange creation ofiabbrevs.This is what actually makes it async.
On this MR,
iabbrevs (nownvim_set_keymap, see point 3) isvim.scheduled. This allows the event loop to processother events in-between ours, making the editor more responsive during initialization.
Each scheduled
iabbrevcontains an instruction to schedule the nextiabbrev, until all are loaded. This way,we do not fire off a large number of
vim.schedulecalls right away.To facilitate this, I used
coroutines, since this makes it easier to arrange this type of behavior.Switching to
nvim_set_keymapfromvim.cmd.The
nvim_set_keymapAPI call supports an"ia"mode, wihch is equivalent toiabbrevAFAICT.I looked on the README for a compatibility promise for Neovim versions (e.g.
"0.8+"), but didn't notice one. There is a chance this has repercussions on backwards compatibility, but without knowing the supported versions, I can't say for sure.Let me know if you have any comments/concerns 🙂
I would be happy to make this a toggle if needed, so that users can opt-in to the async behavior.