search

lukechilds / zsh-nvm

Zsh plugin for installing, updating and loading nvm
MIT License
2.19k stars 111 forks source link

Optimize lazy loading performance #76

Closed Riatre closed 1 year ago

Riatre commented 3 years ago

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.

niklaas commented 3 years ago

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

mikob commented 3 years ago

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 commented 2 years ago

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

andronocean commented 2 years ago

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.

mmelikyan commented 1 year ago

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 commented 1 year ago

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