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

Exploration: Drop yarn in favour of npm #8718

Closed
wants to merge 9 commits into from

Conversation

oskosk
Copy link
Contributor

@oskosk oskosk commented Feb 1, 2018

Changes proposed in this Pull Request:

  • Removes references and usage of yarn as the benefits by which yarn was chosen are implemented in npm now.

Testing instructions:

Proposed changelog entry for your changes:

@zinigor
Copy link
Member

zinigor commented Feb 2, 2018

This seems to work great! One thing I can suggest here is to add npm caching to Travis configuration. It's currently set up to cache files that yarn creates in the home folder.

@Viper007Bond
Copy link
Contributor

Viper007Bond commented Feb 5, 2018

Yarn is still faster than npm for me: https://github.com/Viper007Bond/npm-yarn-benchmark

 -----------------------------------------------------------------------
 -------------------------- RESULTS (seconds) --------------------------
 -----------------------------------------------------------------------
|                          |      jetpack
|     npm_with_empty_cache |      400.842
|      npm_with_all_cached |      427.144
|    yarn_with_empty_cache |      353.973
|     yarn_with_all_cached |      334.688
 -----------------------------------------------------------------------
$ node --version && npm --version && yarn --version
v9.4.0
5.6.0
1.3.2

EDIT: That was with 10 runs.

@zinigor
Copy link
Member

zinigor commented Feb 6, 2018

It's strange, my results are the opposite:

 ----------------------------------------------------------------------- 
 -------------------------- RESULTS (seconds) -------------------------- 
 ----------------------------------------------------------------------- 
|                          |      jetpack 
|     npm_with_empty_cache |       28.700 
|      npm_with_all_cached |       26.797 
|    yarn_with_empty_cache |       49.687 
|     yarn_with_all_cached |       34.550 
 ----------------------------------------------------------------------- 
$ node --version && npm --version && yarn --version
v8.9.4
5.6.0
1.3.2

@Viper007Bond
Copy link
Contributor

Okay, maybe my Internet and laptop just suck then. Good to know. 👍

@oskosk
Copy link
Contributor Author

oskosk commented Feb 6, 2018

@Viper007Bond thanks for the code there. I got a copy of the benchmark tool but it didn't run on my mac (awk kept complaining about a division by zero).

But I'm interested about what results you get if you don't rm the lock files on each run. This could be improving the numbers as the tools won't be needing to check which version is the one they should get absence of the lock file.

@Viper007Bond
Copy link
Contributor

I got a copy of the benchmark tool but it didn't run on my mac (awk kept complaining about a division by zero).

You probably don't have /usr/bin/time (which is different than bash's time). Try sudo apt install time and then see if that fixes it.

@Viper007Bond
Copy link
Contributor

Also I ran this on my WP.com sandbox as I didn't trust Windows Subsystem for Linux speed. Here's the results (with the default 3 runs):

 -----------------------------------------------------------------------
 -------------------------- RESULTS (seconds) --------------------------
 -----------------------------------------------------------------------
|                          |      jetpack
|     npm_with_empty_cache |       48.357
|      npm_with_all_cached |       30.983
|    yarn_with_empty_cache |       37.717
|     yarn_with_all_cached |       20.660
 -----------------------------------------------------------------------

@oskosk
Copy link
Contributor Author

oskosk commented Feb 7, 2018

time is there :(

osk@pizzazz-2 npm-yarn-benchmark (master)$ which time
/usr/bin/time
osk@pizzazz-2 npm-yarn-benchmark (master)$ time

real	0m0.000s
user	0m0.000s
sys	0m0.000s

@Viper007Bond
Copy link
Contributor

Hmm, maybe an old version that doesn't support the needed flags? /shrug

 viper007bond@AlexSurface  ~  /usr/bin/time
Usage: /usr/bin/time [-apvV] [-f format] [-o file] [--append] [--verbose]
       [--portability] [--format=format] [--output=file] [--version]
       [--quiet] [--help] command [arg...]
 ✘ viper007bond@AlexSurface  ~  /usr/bin/time --version
GNU time 1.7

@oskosk
Copy link
Contributor Author

oskosk commented Feb 7, 2018

yeah, time here is crap.

@oskosk
Copy link
Contributor Author

oskosk commented Feb 9, 2018

I didn't realize that you updated the benchmark.sh to install Jetpack dependencies, went to clone the repo blindly and ran it on my sandbox...

...5 years have passed now, the benchmark has finished and I can confirm that it looks like yarn did it faster here.

I would still like to check something because the times for npm using cache and npm not using cache were exactly the same. I don't know what happened there.

@withinboredom
Copy link
Contributor

withinboredom commented Feb 15, 2018

So, I decided to write up a bit more of benchmark in C# using the native bit of windows:

Benchmarking: Yarn vs NPM vs WSL vs Windows
-------------------------------------------
Running 3 times per library, test results will be stored in C:\Users\withinboredom\source\repos\npm-yarn-benchmark\npm-yarn-benchmark\reports

automattic/jetpack =>
     npm with clean cache using cmd
  0    time => 98.26s, kernel => 0.02s, user => 0s, io => 98s
  1    time => 92.2s, kernel => 0.02s, user => 0s, io => 92s
  2    time => 97.15s, kernel => 0.03s, user => 0s, io => 97s
  avg  time => 95.87s, kernel => 0.02s, user => 0s, io => 95.85
     npm  using cmd
  0    time => 92.45s, kernel => 0s, user => 0s, io => 92s
  1    time => 89.1s, kernel => 0.03s, user => 0s, io => 89s
  2    time => 88.6s, kernel => 0s, user => 0s, io => 89s
  avg  time => 90.05s, kernel => 0.01s, user => 0s, io => 90.04
     yarn with clean cache using cmd
  0    time => 113.66s, kernel => 0.05s, user => 0.02s, io => 114s
  1    time => 114.51s, kernel => 0.02s, user => 0s, io => 114s
  2    time => 109.33s, kernel => 0.02s, user => 0s, io => 109s
  avg  time => 112.5s, kernel => 0.03s, user => 0.01s, io => 112.47
     yarn  using cmd
  0    time => 74.16s, kernel => 0.02s, user => 0s, io => 74s
  1    time => 74.75s, kernel => 0.02s, user => 0s, io => 75s
  2    time => 84.46s, kernel => 0s, user => 0s, io => 84s
  avg  time => 77.79s, kernel => 0.01s, user => 0s, io => 77.78

It looks like it's massively io-bound (which makes sense, it does a little bit of downloading, a little bit of copying, etc, etc). npm is io-bound regardless of cache. It doesn't "think" too much. Windows helps exacerbate this due to it's "filesystem safety" that lacks the concept of file descriptors or symlinks in *nix.

Anyway, if you see npm being consistently faster, you have some fast disks because that's the only thing holding it back! Yarn tends to "think about it" and once the files are cached, gets the job done faster than npm. For *nix users, we're talking a few seconds here, a few seconds there. For people with slow drives, or crappy filesystems, you could be talking about minutes, which should (in theory) make yarn the default choice for now.

@enejb
Copy link
Member

enejb commented Mar 22, 2018

I think we should remove the dependency on yarn.
My reasons for that are the following:

  • There is nothing in yarn that can't be done with npm (at least for the featured that we use)
  • Build speeds are comparable. (in order of magnitude)
  • It is one less dependency to keep up with.
  • NPM is easier to install. It comes with node. In order to install it you need brew on mac. https://yarnpkg.com/lang/en/docs/install/
  • It is what Calypso, WordPress Core and Gutenberg uses so chances are other devs are more familiar + have it installed already.

Let's ditch yarn 🎉

@enejb
Copy link
Member

enejb commented Mar 22, 2018

This PR probably needs updating to include some of the new yarn scripts that were added for docker

@samouri
Copy link

samouri commented Mar 23, 2018

It is what Calypso, WordPress Core and Gutenberg uses

We are currently considering switching to yarn on Calypso: Automattic/wp-calypso#23566 😆

In light of that, I've got a couple question for Jetpackers:

  1. How has your experience with the yarn lockfile been? How much time do you spend on the lockfile when adding/removing a dependency?
  2. Do you take advantage of of offline functionality via yarn install --offline?

@Viper007Bond
Copy link
Contributor

How has your experience with the yarn lockfile been? How much time do you spend on the lockfile when adding/removing a dependency?

What do you mean? I was under the impression that was all handled magically when you use the add/remove commands in yarn.

As for offline mode, I'm never offline, but the cache is hugely useful (although I guess npm has that now too?).

@samouri
Copy link

samouri commented Mar 26, 2018

What do you mean? I was under the impression that was all handled magically when you use the add/remove commands in yarn.

Thats enough evidence for me 😆.
While according to the docs npm should update the lockfile after every command just like yarn does, in reality it has never actually worked for me.

@Viper007Bond
Copy link
Contributor

Viper007Bond commented Mar 26, 2018

Disclaimer: Requires testing for confirmation. I may be wrong. 😉

@oskosk
Copy link
Contributor Author

oskosk commented May 24, 2018

Closing

@oskosk oskosk closed this May 24, 2018
@Viper007Bond Viper007Bond deleted the update/use-npm-instead-of-yarn branch May 26, 2018 15:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants