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

Don't load plugin-related things when the relevant plugin isn't installed #294

Merged
merged 2 commits into from
Apr 10, 2022

Conversation

jvoisin
Copy link
Contributor

@jvoisin jvoisin commented Mar 31, 2022

Function calls are expensive in vim-script.
This commit speeds up the processing of colors/nord.vim
in neovim 0.6.1 by almost 50% on my machine, where
none of the supported plugins are installed.

Some plugins provide no easy/straightforward way to be detected, so not all blocks are behind conditions, but it's still a significant improvement, and can always be improved in the future.

…lled

Function calls are expensive in vim-script.
This commit speeds up the processing of colors/nord.vim
in neovim 0.6.1 by almost 50% on my machine, where
none of the supported plugins are installed.
@svengreb
Copy link
Member

svengreb commented Apr 5, 2022

Hi @jvoisin 👋, thanks for your contribution 👍
I'm currently busy with the preparations for the roadmap and future plan of the whole Nord project, but I'll try to review the changes within the next week(s).

The Neovim terminal colors and some plugins are exclusively for Neovim
so they can be warpped in a `nvim` conditional block too.

GH-294
Copy link
Member

@svengreb svengreb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only loading the highlighting groups conditionally makes sense and reducing overhead, and therefore improving the performance, is always good 😄
I've also moved some other Neovim exclusive features & plugins into a nvim block guard even though they will be deprecated and removed from this port when the official Neovim port is available in a stable version.

Thanks again for the contribution 🤘🏼

@svengreb svengreb merged commit a4bf0a6 into nordtheme:develop Apr 10, 2022
@jvoisin jvoisin deleted the conditional branch April 10, 2022 11:17
svengreb added a commit that referenced this pull request Apr 11, 2022
This commit is a continuation of GH-294, see message and PR discussion
of commit `a4bf0a63e8e1e62e884eee9ec04d577105f58aa7` for details.

[1]: a4bf0a6

Related to GH-294
GH-296


Co-authored-by: Julien Voisin <[email protected]>
Co-authored-by: Sven Greb <[email protected]>
@alappe
Copy link

alappe commented Apr 11, 2022

I get

Error detected while processing …/nord-vim/colors/nord.vim:
line  729:
E117: Unknown function: exist
line  862:
E117: Unknown function: exist

in NVIM v0.7.0-dev+1436-gf0d07dcb7.

@jvoisin
Copy link
Contributor Author

jvoisin commented Apr 11, 2022

#301 should fix it, sorry about breaking things :/

@alappe
Copy link

alappe commented Apr 11, 2022

No problem, thanks for fixing it :)

@svengreb
Copy link
Member

@alappe Thanks for the report, this has been fixed in #297. Also see my comment in the PR about what can be learned from this.

arcticicestudio pushed a commit that referenced this pull request Apr 23, 2022
The changes introduced in PR GH-294 [1] did not take into account that
the order how plugins are loaded are not always constant and can also
change based on how users order plugins in their configurations.
When a supported plugin is loaded after Nord the global `loaded_*`
variable might not be available yet, causing the styles to be skipped
due to the conditional block guard. Also each plugin manager handles the
plugin loading order differently which is also a problem when checking
for global variable existence.

[1]: #294
[2]: #306

Fixes GH-306
svengreb added a commit that referenced this pull request Apr 23, 2022
The changes introduced in PR GH-294 [1] did not take into account that
the order how plugins are loaded are not always constant and can also
change based on how users order plugins in their configurations.
When a supported plugin is loaded after Nord the global `loaded_*`
variable might not be available yet, causing the styles to be skipped
due to the conditional block guard. Also each plugin manager handles the
plugin loading order differently which is also a problem when checking
for global variable existence.

The loading time of the Nord plugin is still totally fine so improving
the stability for only a minimal performance boost is no negative trade
at all (tested via `vim --startuptime timing.out`):

  4.956ms: sourcing ~/.local/share/vim/plugged/nord-vim/colors/nord.vim

[1]: #294
[2]: #306

Fixes GH-306
arcticicestudio pushed a commit that referenced this pull request May 14, 2022
…lled (#294)

Guard plugin and Neovim specific features in conditional blocks

Function calls are expensive in Vim script so to reduce the overhead of
processing the theme most plugin and Neovim specific features are now
wrapped in conditional blocks, e.g. Neovim highlighting groups are only
loaded when the global `nvim` variable is set while other plugins are
checked for their own `loaded_*` variable (common plugin pattern).

Some plugins provide no easy/straightforward way to be detected, so not
all blocks are behind conditions, but it's still a significant
improvement, and can always be improved in the future.


Co-authored-by: Julien Voisin <[email protected]>
Co-authored-by: Sven Greb <[email protected]>

GH-294
arcticicestudio pushed a commit that referenced this pull request May 14, 2022
This commit is a continuation of GH-294, see message and PR discussion
of commit `a4bf0a63e8e1e62e884eee9ec04d577105f58aa7` for details.

[1]: a4bf0a6

Related to GH-294
GH-296


Co-authored-by: Julien Voisin <[email protected]>
Co-authored-by: Sven Greb <[email protected]>
arcticicestudio pushed a commit that referenced this pull request May 14, 2022
The changes introduced in PR GH-294 [1] did not take into account that
the order how plugins are loaded are not always constant and can also
change based on how users order plugins in their configurations.
When a supported plugin is loaded after Nord the global `loaded_*`
variable might not be available yet, causing the styles to be skipped
due to the conditional block guard. Also each plugin manager handles the
plugin loading order differently which is also a problem when checking
for global variable existence.

The loading time of the Nord plugin is still totally fine so improving
the stability for only a minimal performance boost is no negative trade
at all (tested via `vim --startuptime timing.out`):

  4.956ms: sourcing ~/.local/share/vim/plugged/nord-vim/colors/nord.vim

[1]: #294
[2]: #306

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

Successfully merging this pull request may close these issues.

4 participants