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

[WIP] Try caching compiled JavaScript when building packages #13124

Closed
wants to merge 1 commit into from

Conversation

noisysocks
Copy link
Member

@noisysocks noisysocks commented Dec 28, 2018

Continuing on from yesterday's experimentation (#13103) into improving some of our build times.

This changes npm run build:packages to cache the compiled JavaScript that Babel generates. Doing so makes npm install, npm run dev, and npm run build run significantly quicker.

To test, run npm run build:packages. The first run will take a while but subsequent runs will be quicker.

$ time npm run build:packages
$ time npm run build:packages

@noisysocks noisysocks changed the base branch from try/optimize-ci to master December 28, 2018 05:51
@noisysocks noisysocks added the [Status] In Progress Tracking issues with work in progress label Dec 28, 2018
@noisysocks
Copy link
Member Author

@youknowriad: Do you think this is worth continuing with? I added caching to npm run build:packages in a way that lets us configure Travis CI to cache the cache directory.

Pros:

  • npm run build, npm run dev and npm install are all faster. npm run build:packages now takes ~7 seconds, down from ~26 seconds.
  • Travis CI jobs are all faster. When combined with Improve Travis CI build times #13103 it means that our JS unit tests job runs in less than 4 minutes.

Cons:

  • It makes the packages build script more complex.
  • It dumps build artefacts onto people's machines (~/.cache/wordpress-packages).

A compromise could be to only enable caching on Travis CI.

@noisysocks
Copy link
Member Author

cc. @gziolo as well 🙂

@gziolo
Copy link
Member

gziolo commented Jan 21, 2019

There is also a concurrent effort started by @aduth where packages are built async. #8093 was merged and reverted not that long ago. I would like to learn what went wrong, so we could better evaluate the proposed changes. Also this from @talldan:

I had a similar attempt at this over christmas (didn't realise this PR existed). I also didn't see any real improvement in build times, so didn't bother tidying things up:
#13104

It looks like you all have more expertise in this area than I do. I'm fine with all proposals which makes builds faster. With this PR in place we wouldn't have to see packages to be rebuilt every time you run npm run build when nothing has really changed. It's really annoying at the moment.

@noisysocks noisysocks added the [Type] Build Tooling Issues or PRs related to build tooling label Jan 23, 2019
@noisysocks
Copy link
Member Author

I've refreshed this this PR. I think it's worth continuing with.

The only real concern I have is that it means that we're dumping build artefacts into a hidden directory (~/.cache/wordpress-packages) on people's machines.

(Mind you, running ls -a | grep "^\." is pretty revealing: we certainly wouldn't be the only script that does this!)

@gziolo
Copy link
Member

gziolo commented Jan 23, 2019

See https://github.com/WordPress/gutenberg/blob/master/.travis.yml#L15-L19

This is what it is, I wouldn't worry about it :)

@gziolo
Copy link
Member

gziolo commented Jan 23, 2019

@omarreiss is it something that was ever discussed in the context of WordPress core? To cache build artifacts? Well, it might be specific to packages only...

@aduth
Copy link
Member

aduth commented Apr 24, 2019

What's the status of this pull request? Will you plan to revisit it?

For what it's worth, I've created a separate tracking issue for general Travis build performance ideas at #15159 . There were similar optimizations at #14860 which leverage Babel cache which seems to be similar in impact to what's been proposed here.

@aduth aduth added the [Status] Stale Gives the original author opportunity to update before closing. Can be reopened as needed. label Apr 24, 2019
@omarreiss
Copy link
Member

@gziolo I somehow missed your question. We didn't consider this yet for WordPress core, but definitely seems like we should consolidate solutions like this one to WordPress core. The optimizations for core so far focused primarily on improving the efficiency of rebuilds on grunt watch. The setup there is still quite behind on the Gutenberg one. cc @herregroen.

@noisysocks
Copy link
Member Author

What's the status of this pull request? Will you plan to revisit it?

This was a quick proof of concept for an idea that I think still has merit. I'll close the PR though as I am not able to give it the time it deserves.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Status] In Progress Tracking issues with work in progress [Status] Stale Gives the original author opportunity to update before closing. Can be reopened as needed. [Type] Build Tooling Issues or PRs related to build tooling
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants