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

Global installs extremely slow #46

Closed
davewasmer opened this issue Jul 19, 2017 · 38 comments
Closed

Global installs extremely slow #46

davewasmer opened this issue Jul 19, 2017 · 38 comments

Comments

@davewasmer
Copy link

It seems that doing any npm install -g is an order of magnitude slower with asdf-nodejs vs. a standard npm install.

My guess is that it is coming from the post install hook the seems to run on every dependency and every nested dependency for global installs. The post install hook seems to invoke bin/get-bin-names.js for each package, which just parses package.json to find bin files to shim. I'm guessing a big chunk of that time is spent starting up a node process for each package.

I'd suggest we either:

  • convert get-bin-names.js to a bash script that greps through the package.json to find the bin files (given the narrow acceptable values for the bin property in a package.json, grepping seems like it might be possible, or
  • convert bin/postinstall.sh to node and consolidating it with bin/get-bin-names.js, so we can avoid the hit for starting up a node process for each package

Happy to PR the latter approach (the former might stretch my bash-fu skills a bit) if there's any interest.

@mareksuscak
Copy link

mareksuscak commented Oct 8, 2017

I'm also unable to install and override the global npm package at all (i.e. replace with the latest version). I'm getting "File name too long" error for many packages. This is a deal breaker for me.

Created a separate issue to track work #56

OS: High Sierra w/ APFS

@danhper
Copy link
Member

danhper commented Oct 21, 2017

Sorry for the delay here.
The postinstall script indeed runs for every nested dependency, making install very slow.
Right now, the postinstall hook simply runs asdf reshim nodejs, but running this for every single dependency seems like a lot of overhead.
@davewasmer Can we manage to avoid starting up a new process for every single dependency?
Thank you for your help.

@davewasmer
Copy link
Author

@tuvistavie I've since moved on from asdf unfortunately, so I no longer have time to devote to a PR here. But I would suggest one of the two options I described above.

@Stratus3D
Copy link
Member

I'm experiencing this today. Very painful. While I have not addressed this issue I did address something related: c8f4518. The reshim command works better if a version is set.

@webhive
Copy link

webhive commented Mar 29, 2018

Any progress with it?

@igelstorm
Copy link

igelstorm commented Jun 6, 2018

I just encountered this problem. The code seems to have moved on since the original post - bin/get-bin-names.js is no longer run in the postinstall script, only asdf reshim, but this, too, is slow to run, which adds up to a lot of time when installing packages with a lot of dependencies.

As a blunt workaround, I commented out the line in the postinstall script that runs asdf reshim - i.e. in ~/.asdf/plugins/nodejs/bin/postinstall:

#!/usr/bin/env bash

# asdf reshim nodejs "$ASDF_INSTALL_VERSION"

I then ran asdf reshim nodejs <version> once, manually, after I was finished installing. I don't know if this approach has any unintended side effects, but it seemed to work for me, and might be useful to anyone else who has this issue right now and needs to get around it.

@igelstorm
Copy link

Some more insight into the problem: running asdf reshim nodejs without a version is a lot slower than giving it a version. It seems that the ASDF_INSTALL_VERSION environment variable isn't populated when the postinstall script is run on my machine, so it runs asdf reshim nodejs without a version number even though this clearly isn't the intended behaviour. I don't know nearly enough about asdf to know why this env var might be blank, so I don't think I can attempt to fix this.

@frnco
Copy link

frnco commented Jul 7, 2018

I settled for using a different env var in the file ~/.asdf/plugins/nodejs/bin/postinstall

Switched from

asdf reshim nodejs "$ASDF_INSTALL_VERSION"`

To

asdf reshim nodejs "$npm_config_node_version"

There may be a way to call ~/.asdf/plugins/nodejs/bin/postinstall through asdf, kinda like asdf-exec, but I have no idea. Would still run for every single package though.

For limiting to a single reshim something like Custom Shim Templates would allow it, but I don't think it's worth it. I'd either stick with postinstall or drop auto-reshimming entirely and allow people to decide if and how they want to automate it.

@joemsak
Copy link

joemsak commented Oct 16, 2018

@frnco's suggestions appears to have worked for me

sukima added a commit to sukima/asdf-nodejs that referenced this issue Nov 7, 2018
Per issue asdf-vm#46 the `ASDF_INSTALL_VERSION` variable is not available in global NPM installs. Instead the `npm_config_node_version` is provided by NPM during this situation.

This change offers a fallback in these cases. Since a version will be available this will significantly speed up the postinstall during global NPM installs.

Fixes asdf-vm#46
@daniel-nagy
Copy link

This is still kinda ridiculous for some packages. It takes almost 6 minutes to install the polymer-cli on my MacBook Pro.

time npm install -g polymer-cli

real	5m42.131s

@lnikkila
Copy link

lnikkila commented Dec 10, 2018

^ I only started noticing this after updating to Node 10.14.1 from 9.11.2, using the latest version of asdf and asdf-nodejs. Not quite sure what changed. Rolling back to 9.x makes the installs fast again.

EDIT: I believe something changed between npm 5.6.0 and 6.1.0 that caused this to happen, Node 10.2.1 is fast but 10.3.0 is slow. I scoured through changelogs but didn’t spot anything suspicious...

@Stratus3D
Copy link
Member

I'm not sure what needs to be done here, but it sounds like there is still a problem.

@lnikkila
Copy link

lnikkila commented Dec 10, 2018

Initially I thought maybe the newer npm stopped exposing the $npm_config_node_version environment variable used in d18287b but that doesn’t seem to be the case, I added some logging to the postinstall hook to verify it’s still there.

However I noticed the hook is now being run for every dependency, whereas with an older npm I can see it’s only run a few times. Maybe it was previously being run for only those dependencies that had their own postinstall scripts? With polymer-cli as an example, npm 5.6.0 runs the script ~5 times whereas npm 6.1.0 runs it at least hundreds of times.

The documentation for scripts says:

If you want to run a specific script at a specific lifecycle event for ALL packages, then you can use a hook script. Place an executable file at node_modules/.hooks/{eventname}, and it'll get run for all packages when they are going through that point in the package lifecycle for any packages installed in that root. Hook scripts are run exactly the same way as package.json scripts. That is, they are in a separate child process, with the env described above.

This does support my theory that the reshim hook is now being run for every single package installed during that npm install call, and the previous behaviour where the hook script was only being run a few times (maybe just for those dependencies that already had their own postinstall scripts) might’ve been a bug in npm. However I couldn’t spot any changelog entries that would support this.

The postinstall script might not be the best place to reshim since it’s called multiple times, and asdf-nodejs is mainly concerned about when the npm install command has finished. Adding some logic to the npm shim script might work better for knowing when installation has really finished, but it’s not a very reliable method for knowing which npm calls install packages.

Maybe some kind of hybrid approach would be a better option, for instance a postinstall hook could create a flag (such as an empty marker file under ~/.asdf/plugins/nodejs) after a package is installed and the npm shim could then check that flag after the real npm exits and run a reshim once before exiting itself...

In my opinion dropping auto-reshimming might be a valid option too in case this gets too difficult, but at the cost of ease of use.

Looking at asdf-vm/asdf#409 maybe some asdf-level solution might be a better idea, those are good suggestions in my opinion.

@mikemorris
Copy link

Looking at asdf-vm/asdf#409 maybe some asdf-level solution might be a better idea, those are good suggestions in my opinion.

^ This looks like a promising direction, some sort of single-run postinstall reshim implementation.

@vic
Copy link
Contributor

vic commented Jan 19, 2019

Could you people test using asdf's master 0.6.4-dev and report back if the new shim executor feels faster?

@lukaselmer
Copy link
Contributor

Could you people test using asdf's master 0.6.4-dev and report back if the new shim executor feels faster?

No, it's still super slow.

@lukaselmer
Copy link
Contributor

npm i -g firebase-tools
[...]
+ firebase-tools@6.3.0
added 544 packages from 272 contributors in 249.83s

@MrQubo
Copy link

MrQubo commented Oct 14, 2019

I want to put my two cents here. There was this unmerged PR #93 which tried to solve this issue. This PR spawns reshim in a background so it doesn't slows down installation of packages. It also adds throttling so reshim isn't called once for every package. The problem, which was the reason of not merging my PR, is that installed packages aren't available immediately after installation, which may have consequences if you try to install some package and use it afterwards in some automatic script.

In my opinion the best solution would be to add new kind of hook into npm, which would be called only once per whole installation, instead of once per every package.

Seems like #118 is also quite good solution for reducing installation times.

@MrQubo
Copy link

MrQubo commented Oct 14, 2019

Another solution I can think of is to patch npm-cli.js on installation and add reshim there. There's already isGlobalNpmUpdate variable in there which is exactly what we would need to check for.

@smorimoto
Copy link
Member

smorimoto commented Oct 28, 2019

@daniel-nagy Related issue for polymer-cli:
Polymer/tools#2583

@ctsstc
Copy link

ctsstc commented Mar 25, 2020

Just added vue cli and it took over 6 minutes to install.

npm i -g @vue/cli
...
+ @vue/cli@4.2.3
added 1189 packages from 662 contributors in 385.805s

Gallons of slow post install calls :(
image

@waynehoover
Copy link

waynehoover commented Apr 3, 2020

took 6m40s just to install gatsby. Sorry to say I think i'm going to switch from asdf to just nvm and rbenv now.

@ctsstc
Copy link

ctsstc commented Jun 2, 2020

Maybe it's a blessing in disguise and nothing should be installed globally lol

@zxaos
Copy link

zxaos commented Jun 3, 2020

Maybe it's a blessing in disguise and nothing should be installed globally lol

You want to install updated versions of npm/yarn into each project?

@danhper
Copy link
Member

danhper commented Aug 9, 2020

This has been an issue for quite a while. I personally think that auto-reshim is not really needed.
Rather than having ASDF_SKIP_RESHIM, I would simply remove the auto-reshim feature altogether and add a note about reshim in the docs.

@wavebeem
Copy link

wavebeem commented Jan 8, 2021

Any chance this reshim behavior will get removed? I love the idea of asdf, but between having to install GPG just to install Node, and dealing with npm i -g being so slow, it's pretty disappointing how much extra work you have to do to make Node work well with asdf

@sjml
Copy link

sjml commented Jan 21, 2021

Do other standard plugins do automatic reshimming? At least the ones that I use regularly do not, but I haven't examined all of them. If it's non-standard behavior, especially given the unnecessary slowdown it causes with asdf-nodejs, it seems to make more sense to remove it. (Anyone using asdf quickly becomes familiar with the need to reshim after installing something that includes a binary.)

Even setting ASDF_SKIP_RESHIM=1 only helps a little bit, since it still triggers and runs the check (and prints the caveat) for every dependency.

@augustobmoura
Copy link
Member

I made some advancements in #197, @Stratus3D can you take a look? I think this a really big issue to be solved

@frnco
Copy link

frnco commented Mar 8, 2021

I made some advancements in #197, @Stratus3D can you take a look? I think this a really big issue to be solved
I don't think it's that big but I'm happy to see people doing PRs instead of just asking for stuff.

I'm not sure what needs to be done here, but it sounds like there is still a problem.

Let me add to that: I'm not even sure this actually IS a problem, I mean I've used Slackware for a few years after 2001, and I've used Gentoo (I actually have a PowerPC that I use as a server running it in my living room), and compiling stuff takes FOREVER, and if you tell that to the Gentoo folks the first thing they'll ask is how many HOURS.

With ASDF the time saved is already gigantic, if anyone really needs more, AT LEAST send a PR, or do it in a fork or whatever. This thing is free, try to see if you can figure it out, have a blast! I don't get why this issue is even open I mean it's from 2017, seriously, it's obviously not even enough of a problem to most people...

@augustobmoura
Copy link
Member

augustobmoura commented Mar 9, 2021

Let me add to that: I'm not even sure this actually IS a problem, I mean I've used Slackware for a few years after 2001, and I've used Gentoo (I actually have a PowerPC that I use as a server running it in my living room), and compiling stuff takes FOREVER, and if you tell that to the Gentoo folks the first thing they'll ask is how many HOURS.

This doesn't even makes sense, Gentoo and Slackware are not the same as NodeJS, +6min to install a package that in base npm takes 30-60s is a HUGE difference, and in bigger packages it can take even more, asdf it's introducing linear time to the installation with a considerable big coefficient

I don't dare to quote your "free stuff no whining" argument because every time a read it I get a bit more salty. Just because a tool is open source doesn't mean that issues should be ignored, this is why the issues tab exists for starters. This issue has been triaged and have space for improvement, there's many ways we can amortize this and I'm actively researching a way to fully prevent it

I don't get why this issue is even open I mean it's from 2017, seriously, it's obviously not even enough of a problem to most people...

A issue is not "closed" just because it is old or because people don't bother with it that much, sometimes the issue is a intended behavior and won't be "fixed", not in this case though. A issue is a change that can be made while still applicable, there's plenty of examples of older issues that were not fixed to this day in both Slackware and Gentoo that you defended. Your argument is deeply flawed and mostly a disconnected rant, if I were a repository moderator I would certainly flag it as off-topic or irrelevant

@frnco
Copy link

frnco commented Mar 9, 2021

This doesn't even makes sense, Gentoo and Slackware are not the same as NodeJS, +6min to install a package that in base npm takes 30-60s is a HUGE difference, and in bigger packages it can take even more, asdf it's introducing linear time to the installation with a considerable big coefficient

I don't dare to quote your "free stuff no whining" argument because every time a read it I get a bit more salty. Just because a tool is open source doesn't mean that issues should be ignored, this is why the issues tab exists for starters. This issue has been triaged and have space for improvement, there's many ways we can amortize this and I'm actively researching a way to fully prevent it

Actually you're the one I think is right in this one. I just pointed out you were the only one to actually get your hands dirty, which I think is how this should be done. And I never said "free stuff no whining" for the reason I don't believe that. What I believe is "free stuff so if you want something try to help, don't just expect others to do stuff for you". And you perfectly fit on that, you're nice, I loved to see a PR for that and I totally support your effort. Maybe I put it in a really bad way, I'm sorry for that. I was actually trying to point out how you were the only person to actually do something about it, and how we need more of that.

A issue is not "closed" just because it is old or because people don't bother with it that much, sometimes the issue is a intended behavior and won't be "fixed", not in this case though. A issue is a change that can be made while still applicable, there's plenty of examples of older issues that were not fixed to this day in both Slackware and Gentoo that you defended. Your argument is deeply flawed and mostly a disconnected rant, if I were a repository moderator I would certainly flag it as off-topic or irrelevant

Now you have a point here, and I guess many people would agree to that. I just happen to be among the people who thinks this would fit in a roadmap or something, not on an issue list, since it has been ignored for years without any major consequences whatsoever. Not that it matters, neither of us is actually the maintainer for this repo and the maintainer is the one who decides what's left open and what gets closed. And BTW I used and loved both Slackware and Gentoo, but currently only use gentoo for one server that is a PowerPC, meaning I can't run Arch on it. But you can probably find old issues on the Arch repo, and on many repositories of stuff I love, and even I may have old stuff on issues. I never said I followed this perfectly. I just pointed out, "if this was left alone for 4 years and people still use nevertheless, is it ACTUALLY an issue?". You think so, and you did a PR, great, I hope you actually fix this, it would certainly be an improvement. Still, if/when you decide it's sufficient, how much "room for improvement" would be insufficient to keep this open? What's the criteria for closing it? That's what I was trying to question.

And again, I absolutely support PRs for this or anything else that can help people. I just think it's weird that forgotten stuff from years ago comes back to life in the notifications when there were obviously a bazillion npm installs during this time that nobody considered bad enough to come back here and do something about it (Until now of course), and I think it shows that this is not an issue, but something on a roadmap/backlog/whatever.

@CherryDT
Copy link

Not everyone who is annoyed by that opens or comments on an issue or even realizes it's asdf's fault this is happening in the first place, and also not everyone remembers updating their global packages regularly, unfortunately.

I happened to notice because I hadn't used asdf before and it started happening together with me starting to use asdf. It is infuriating every time I install or update something globally.

I agree this is a problem and warrants an "issue" status. I mean, if I sell you pants but as side effect you walk 10x slower because the material is so stiff, it's not a thing for a roadmap to be fixed some time in the future, it greatly reduces the value of the product... - Of course since this isn't a commercial product, I cannot expect a speedy fix (and I don't understand enough about this to create a PR myself), but I still think it should be recognized as a problem.

@augustobmoura
Copy link
Member

augustobmoura commented Jul 19, 2021

Ok, so I pushed a new mechanism for auto-reshimming that doesn't rely on running postinstall hooks on every package. Instead, the new mechanism wraps around the npm command and reshims only on global installs and uninstalls, and only after every package is installed, any other command is just a pass-through to npm.

I also added a step for removing the old postinstall script files automatically. So you guys will probably see something like this in your next npm install -g on already installed versions: "This nodejs installation is using an older auto-reshimming mechanism, we are now removing this mechanism from all nodejs installations...". That pretty much solves the issue with slowness on installations

This new mechanism is still pretty much in testing and I would really appreciate any feedback on it. If the slowness still persists or you guys are still seeing those pesky postinstall messages, please let me know

@CherryDT
Copy link

Thank you, much appreciated!

@Stratus3D
Copy link
Member

@augustobmoura thanks! Sorry I never got around to reviewing your PR!

@CherryDT
Copy link

CherryDT commented Oct 8, 2021

Hm, it's still happening for me... I already used asdf update --main and I also tried reshimming, and even installing a new node version... Am I missing something?

npm i -g standard takes 5 minutes...

image

@OndroNR
Copy link
Contributor

OndroNR commented Oct 8, 2021

@CherryDT did you also update plugins asdf plugin update --all

@CherryDT
Copy link

CherryDT commented Oct 8, 2021

I did now. Thank you, it worked!

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

No branches or pull requests