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

fix(startup): save startup time, thanks @rem, closes #40 #42

Closed
wants to merge 1 commit into from

Conversation

janl
Copy link
Contributor

@janl janl commented Oct 2, 2015

Thanks @remy!

@boennemann
Copy link
Member

Whenever you initiate the update notifier and it's not within the interval threshold, it will asynchronously check with npm in the background for available updates, then persist the result. The next time the notifier is initiated the result will be loaded into the .update property. This prevents any impact on your package startup performance. The check process is done in a unref'ed child process. This means that if you call process.exit, the check will still be performed in its own process.

I think this is already taken care of/this change doesn't make any difference.

@janl
Copy link
Contributor Author

janl commented Oct 2, 2015

I could measure ~70ms time improvement consistently on my MBP

@boennemann
Copy link
Member

I just wanted to add, except for the time it takes to load the library itself into memory. But this also means the notifier is only ever called when the command doesn't fail, which isn't that cool, given that a lot of fails could result from the CLI being outdated.

@janl
Copy link
Contributor Author

janl commented Oct 2, 2015

good point, ignore.

@janl janl closed this Oct 2, 2015
@janl janl mentioned this pull request Oct 2, 2015
@boennemann
Copy link
Member

Addendum II: The defaults operation could be quite expensive. We could probably speed this up by just picking name and version first, and then applying the defaults.

@boennemann boennemann deleted the fastboot branch October 8, 2015 14:53
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.

2 participants