Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Startup performance regression #966

Closed
joliss opened this issue Jan 13, 2016 · 10 comments · Fixed by #2317
Closed

Startup performance regression #966

joliss opened this issue Jan 13, 2016 · 10 comments · Fixed by #2317
Labels
performance This relates to anything regarding the speed of using nvm. pull request wanted This is a great way to contribute! Help us out :-D

Comments

@joliss
Copy link
Contributor

joliss commented Jan 13, 2016

A performance regression seems to have been introduced in 1458de7, raising startup time (time . ~/.nvm/nvm.sh) from 65ms to 340ms on my system.

I don't have time to dig into the cause right now, but just wanted to report this. :)

P.S. Here's the bash command I used to narrow down on the commit.

@ljharb
Copy link
Member

ljharb commented Jan 13, 2016

Thanks - it's known, and somewhat intentional - it's more important to ensure that people aren't using the "prefix" npm config than it is to ensure there's no performance issues.

I don't know of a more reliable way to evaluate npm config get prefix without running npm, which is the slowest part.

@ljharb ljharb added pull request wanted This is a great way to contribute! Help us out :-D performance This relates to anything regarding the speed of using nvm. labels Jan 13, 2016
@wadey
Copy link

wadey commented Mar 18, 2016

Could we make it possible to set an env var, such as NVM_NO_NPM_PREFIX, which if set could skip the check?

@ljharb
Copy link
Member

ljharb commented Mar 18, 2016

@wadey My concern is then that people will just set that env var while using a prefix (and that random blogs and tutorials will recommend the same) - which isn't acceptable. If someone wants to use a prefix, I don't want them using nvm.

However, it's very unfortunate that everyone has to suffer the npm slowdown just for the minority that have a prefix set :-(

@megalithic
Copy link

Any decision on this ever come about? An ENV var at least gives those of us that care enough a way to work around it. cc @ljharb

@jeffwidman
Copy link

jeffwidman commented May 9, 2016

@wadey My concern is then that people will just set that env var while using a prefix (and that random blogs and tutorials will recommend the same) - which isn't acceptable.

I help maintain several popular open-source projects, so I feel your pain of the deluge of support tickets due to misconfiguration/not reading the README.

However, it's very unfortunate that everyone has to suffer the npm slowdown just for the minority that have a prefix set :-(

Another way to look at it is that some folks who live in their terminals simply won't use nvm if these safety checks make it this slow. I have a fairly slim .zshrc/prezto setup that used to load in <0.5s... I installed/sourced nvm and it felt like molasses, which is how I found this issue.

I'd really like an ENV var called NVM_DISABLE_SAFETY_CHECKS that bypasses these slow npm calls on shell startup. To be clear, I have no problem if they're called when I run nvm use ... or the first time I actually use node in a particular shell session. I just don't want to wait in all the other shell sessions where I'm not using node.

What about adding an issue template (now that Github supports them) that says:

Before reporting an issue, run echo $NVM_DISABLE_SAFETY_CHECKS and make sure it doesn't return True. If it does, you'll need to run export NVM_DISABLE_SAFETY_CHECKS=False to re-enable the safety checks before your bug report will be accepted.

Sane defaults makes sense, but locking it down so it's not performant just to prevent someone shooting themselves in the foot is like creating a "No child left behind" program that effectively is a "No child allowed ahead" program.

I'm happy to help pitch in here, and perhaps there's a way to make nvm performant without disabling the npm checks. I can't come up with another feasible solution though.

For now, I've resorted to nvm unalias default, which requires that I always nvm use node whenever I actually want to use node.

@ljharb
Copy link
Member

ljharb commented May 14, 2016

@jeffwidman instead of removing the default alias, you can do . "$NVM_DIR/nvm.sh" --no-use in your profile file if you're using a shell that supports this (ie, not dash/sh, but bash/zsh should be OK)

@keithws
Copy link

keithws commented Aug 22, 2016

I also found that my shell startup slowed down after installing nvm. Without nvm, my .bash_profile takes 800ms to source. With nvm, it takes ~2600ms. A nearly two second delay for every shell startup really starts to hurt.

I just curled down the latest nvm, 0.31.4, and the issue remains. Here is my nvm debug output:

nvm --version: v0.31.4
$SHELL: /bin/bash
$HOME: /Users/jsmith
$NVM_DIR: '$HOME/.nvm'
$PREFIX: ''
$NPM_CONFIG_PREFIX: ''
nvm current: v6.3.1
which node: $NVM_DIR/versions/node/v6.3.1/bin/node
which iojs: 
which npm: $NVM_DIR/versions/node/v6.3.1/bin/npm
npm config get prefix: $NVM_DIR/versions/node/v6.3.1
npm root -g: $NVM_DIR/versions/node/v6.3.1/lib/node_modules

Any plans to offer a solution?

@ljharb
Copy link
Member

ljharb commented Aug 22, 2016

@keithws at the moment, the solution is directly above your comment: #966 (comment)

@keithws
Copy link

keithws commented Aug 23, 2016

Unfortunately, that solution then breaks node everywhere else until I specify nvm use node as @jeffwidman pointed out. As someone who likes to jump into the REPL quite often, that makes the solution about as bad as the problem.

Since my startup time seems to be much longer than @joliss, 2600ms vs 340ms, could it be another issue? Note, npm config get prefix completes in roughly 330ms on my system. Should I open a new issue?

@ljharb
Copy link
Member

ljharb commented Aug 23, 2016

@keithws sure, but I've tried profiling things before and there's nothing that can easily be sped up or removed - just a number of 2-300ms things that add up.

You could try #966 (comment) and then follow it up with nvm use & in your profile file, perhaps?

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
performance This relates to anything regarding the speed of using nvm. pull request wanted This is a great way to contribute! Help us out :-D
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants