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

Optimize lazy loading performance #76

Merged
merged 1 commit into from
Feb 24, 2023
Merged

Conversation

Riatre
Copy link
Contributor

@Riatre Riatre commented Feb 8, 2021

zsh-nvm is pretty slow to load even with NVM_LAZY_LOAD=1 due to the
presence of a lots of global binaries. I have ~110 global binaries
installed, zprof shows that _zsh_nvm_lazy_load alone takes 113.23ms,
causing noticable delay during shell startup.

Further profiling shows that when there are a lot of global binaries,
running basename on each of them (because xargs -n1) is pretty slow,
the same goal could be achieved via zsh builtin expansion flags or
modifiers and it's much faster. Running which in a for loop also has a
similar but less significant drawback.

After the change zsh-nvm loads in 6.03ms under same conditions.

Additionally, added proper quoting to guard against binaries with
whitespaces in their name. This has near to zero real-world impact but
still, it is the correct thing to do.

May supersede #75.

zsh-nvm is pretty slow to load even with NVM_LAZY_LOAD=1 due to the
presence of a lots of global binaries. I have ~110 global binaries
installed, zprof shows that `_zsh_nvm_lazy_load` alone takes 113.23ms,
causing noticable delay during shell startup.

Further profiling shows that when there are a lot of global binaries,
running `basename` on each of them (because `xargs -n1`) is pretty slow,
the same goal could be achieved via zsh builtin expansion flags or
modifiers and it's much faster. Running `which` in a for loop also has a
similar but less significant drawback.

After the change zsh-nvm loads in 6.03ms under same conditions.

Additionally, added proper quoting to guard against binaries with
whitespaces in their name. This has near to zero real-world impact but
still, it is the correct thing to do.
@niklaas
Copy link

niklaas commented Mar 26, 2021

I'll be very happy if this is merged. Lazy loading nvm takes >500ms on my machine.

@mikob
Copy link

mikob commented Jul 10, 2021

This works for me with NVM_LAZY_LOAD=true. I had to find other places nvm was loading and take them out (e.g. ~/.zshenv, ~/.profile

@callumacrae
Copy link

This is a really good change! Got almost exactly the same timings as you - 113ms before, 6ms after

@andronocean
Copy link

Wow, this is insanely fast now. On my system _zsh_nvm_lazy_load dropped from 77ms to 2.8ms, even without a lot of global packages. Phenomenal!

On a tangential note: I was having a bug (#87 ) sourcing zsh-nvm within an anonymous function, due to the for cmd in $cmds forloop. The solution is to simply declare the cmd variable as local first:

  # Create function for each command
  local cmd
  for cmd in $cmds; do

It'd be great if you could double-check that that makes sense and add the fix to the PR if so.

phildenhoff added a commit to phildenhoff/zsh-nvm that referenced this pull request Apr 30, 2022
@mmelikyan
Copy link

I know this is a stale PR and I'm unsure why it was never merged but it allows for significant performance benefits that are very useful in my day-to-day development. Meanwhile I'll use Riatre's fork, if anyone else wants to remove the lag from their terminal startup

git clone https://github.com/Riatre/zsh-nvm.git ~/.oh-my-zsh/custom/plugins/zsh-nvm

then follow instructions for adding to omz plugins in zshrc

@lukechilds lukechilds merged commit a0f0a2c into lukechilds:master Feb 24, 2023
@lukechilds
Copy link
Owner

Sorry guys, I have limited time to spend maintaining my OSS projects these days. I've merged this in, thanks @Riatre!

lukechilds added a commit that referenced this pull request Mar 1, 2023
lukechilds added a commit that referenced this pull request Mar 1, 2023
henderea pushed a commit to henderea/zsh-nvm that referenced this pull request Jun 13, 2023
henderea pushed a commit to henderea/zsh-nvm that referenced this pull request Jun 13, 2023
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.

7 participants