Code Monkey home page Code Monkey logo

Comments (38)

augustobmoura avatar augustobmoura commented on May 29, 2024 24

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

from asdf-nodejs.

daniel-nagy avatar daniel-nagy commented on May 29, 2024 8

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

from asdf-nodejs.

lukaselmer avatar lukaselmer commented on May 29, 2024 8

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.

from asdf-nodejs.

augustobmoura avatar augustobmoura commented on May 29, 2024 8

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

from asdf-nodejs.

danhper avatar danhper commented on May 29, 2024 7

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.

from asdf-nodejs.

frnco avatar frnco commented on May 29, 2024 7

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...

from asdf-nodejs.

frnco avatar frnco commented on May 29, 2024 4

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.

from asdf-nodejs.

ctsstc avatar ctsstc commented on May 29, 2024 4

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

from asdf-nodejs.

CherryDT avatar CherryDT commented on May 29, 2024 3

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.

from asdf-nodejs.

frnco avatar frnco commented on May 29, 2024 2

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.

from asdf-nodejs.

OndroNR avatar OndroNR commented on May 29, 2024 2

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

from asdf-nodejs.

danhper avatar danhper commented on May 29, 2024 1

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.

from asdf-nodejs.

igelstorm avatar igelstorm commented on May 29, 2024 1

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.

from asdf-nodejs.

igelstorm avatar igelstorm commented on May 29, 2024 1

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.

from asdf-nodejs.

lnikkila avatar lnikkila commented on May 29, 2024 1

^ 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...

from asdf-nodejs.

Stratus3D avatar Stratus3D commented on May 29, 2024 1

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

from asdf-nodejs.

lnikkila avatar lnikkila commented on May 29, 2024 1

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.

from asdf-nodejs.

mikemorris avatar mikemorris commented on May 29, 2024 1

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.

from asdf-nodejs.

ctsstc avatar ctsstc commented on May 29, 2024 1

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

npm i -g @vue/cli
...
+ @vue/[email protected]
added 1189 packages from 662 contributors in 385.805s

Gallons of slow post install calls :(
image

from asdf-nodejs.

sjml avatar sjml commented on May 29, 2024 1

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.

from asdf-nodejs.

augustobmoura avatar augustobmoura commented on May 29, 2024 1

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

from asdf-nodejs.

mareksuscak avatar mareksuscak commented on May 29, 2024

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

from asdf-nodejs.

davewasmer avatar davewasmer commented on May 29, 2024

@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.

from asdf-nodejs.

Stratus3D avatar Stratus3D commented on May 29, 2024

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.

from asdf-nodejs.

webhive avatar webhive commented on May 29, 2024

Any progress with it?

from asdf-nodejs.

joemsak avatar joemsak commented on May 29, 2024

@frnco's suggestions appears to have worked for me

from asdf-nodejs.

vic avatar vic commented on May 29, 2024

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

from asdf-nodejs.

lukaselmer avatar lukaselmer commented on May 29, 2024
npm i -g firebase-tools
[...]
+ [email protected]
added 544 packages from 272 contributors in 249.83s

from asdf-nodejs.

MrQubo avatar MrQubo commented on May 29, 2024

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.

from asdf-nodejs.

MrQubo avatar MrQubo commented on May 29, 2024

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.

from asdf-nodejs.

smorimoto avatar smorimoto commented on May 29, 2024

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

from asdf-nodejs.

waynehoover avatar waynehoover commented on May 29, 2024

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

from asdf-nodejs.

zxaos avatar zxaos commented on May 29, 2024

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?

from asdf-nodejs.

wavebeem avatar wavebeem commented on May 29, 2024

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

from asdf-nodejs.

CherryDT avatar CherryDT commented on May 29, 2024

Thank you, much appreciated!

from asdf-nodejs.

Stratus3D avatar Stratus3D commented on May 29, 2024

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

from asdf-nodejs.

CherryDT avatar CherryDT commented on May 29, 2024

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

from asdf-nodejs.

CherryDT avatar CherryDT commented on May 29, 2024

I did now. Thank you, it worked!

from asdf-nodejs.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.