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

refactor: use nvim_set_hl to speed up startup time #76

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

cljoly
Copy link
Contributor

@cljoly cljoly commented May 8, 2022

On my machine, prior to this change, the colorscheme adds about 8ms to
the startup time of neovim. Using the newly introduced API
nvim_set_hl, it’s a little under 3ms, so that’s more than a two times
speedup!

Special care is taken to handle the fmt argument, as that’s the least
friendly to nvim_set_hl.

This change strives to be backward compatible. Once neovim version 0.7.0
will be spread widely enough, further performance gain are within reach
by:

  • removing the fallback that calls string.format
  • defining the group_settings so that those can be passed to nvim_set_hl directly

@cljoly
Copy link
Contributor Author

cljoly commented May 9, 2022

Using https://github.com/rhysd/vim-startuptime with vim-startuptime -vimpath nvim, before the change on my machine I get:

  AVERAGE       MAX       MIN
------------------------------
64.994300 65.748000 64.236000: ~/.config/nvim/init.lua
 8.342700  8.888000  7.954000: ~/.local/share/nvim/site/pack/paqs/start/onedark.nvim/colors/onedark.lua

and after:

  AVERAGE       MAX       MIN
------------------------------
60.578100 62.654000 59.836000: ~/.config/nvim/init.lua
 6.268300  6.810000  5.928000: loading packages
 3.631400  3.860000  3.472000: loading rtp plugins
 2.773000  2.908000  2.604000: reading ShaDa
 2.659900  2.859000  2.508000: ~/.local/share/nvim/site/pack/paqs/start/onedark.nvim/colors/onedark.lua

@cljoly
Copy link
Contributor Author

cljoly commented May 10, 2022

The latest commit will need to be squashed if this PR is merged, but in the meantime, it makes testers live easier because they can just pull from the branch.

fmt was incorrectly handled previously, specifically the iteration was incorrect in Lua. The latest commit fixes that.

@cljoly
Copy link
Contributor Author

cljoly commented May 12, 2022

After daily driving this change, I have not noticed any more discrepancies compared to master.

@navarasu
Copy link
Owner

navarasu commented May 14, 2022

This plugin initially started with nvim_set_hl. But we switched to vim highlight to solve some color highlight issue. Let me check that once

@cljoly
Copy link
Contributor Author

cljoly commented May 14, 2022

Looking at the 0.7 changelog, it seems that nvim_set_hl has been significantly improved, so hopefully that’ll work now.

@navarasu
Copy link
Owner

Sure. Let me verify it this week and will merge if no issues

@cljoly
Copy link
Contributor Author

cljoly commented May 15, 2022

Cool, let me know if you need help!

@cljoly
Copy link
Contributor Author

cljoly commented Jun 4, 2022

Did you encounter any issues in the end?

@cljoly
Copy link
Contributor Author

cljoly commented Jun 18, 2022

ping @navarasu , in case the notification just got lost.

On my machine, prior to this change, the colorscheme adds about 8ms to
the startup time of neovim. Using the newly introduced API
`nvim_set_hl`, it’s a little under 3ms, so that’s more than a two times
speedup!

Special care is taken to handle the `fmt` argument, as that’s the least
friendly to `nvim_set_hl`.

This change strives to be backward compatible. Once neovim version 0.7.0
will be widely spread enough, further performance gain are within reach
by:
* removing the fallback that calls `string.format`
* defining the `group_settings` so that those can be passed to `nvim_set_hl` directly
@cljoly
Copy link
Contributor Author

cljoly commented Oct 2, 2022

Rebased on the most recent master branch.

@carmen-gh
Copy link

let's get this merged...

@abhinavnatarajan
Copy link

Bump!

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

Successfully merging this pull request may close these issues.

4 participants