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

Experiment with not caching the ~/.npm directory during Filesize/Tests CircleCI jobs #10209

Merged
merged 3 commits into from
Oct 19, 2022

Conversation

benjamn
Copy link
Member

@benjamn benjamn commented Oct 19, 2022

Looking at a recent job, restoring and later saving the cached ~/.npm directory takes 13 and 14 seconds, respectively. This job took a total of 1 minute 11 seconds, so restoring/saving the (large) ~/.npm directory accounts for ~38% of the total time.

If removing the caching altogether does not slow anything else down by that much (~27 total seconds), then the caching was not really delivering on its intended goals, and should be removed.

New results after running CI on this commit/PR. Now only 1 minute 1 second (noticeably less time without any caching)!

Notes:

  • The "Spin up environment" step went from taking 2s to 19s, because the circleci/node@16.13.1 Docker image needed to be re-pulled. This seems like something that could be addressed separately, if we care.

  • The npm ci step takes a little longer, 9s instead of 4s, but that's presumably because everything has to be installed without a cache. Still, this is much faster than restoring ~/.npm from cache.

  • By itself, this PR does not (yet) speed up the finishing time of CI checks, since running the tests (the other CI job) always takes longer than running the Filesize job does now (or did before).

Looking at a recent job, restoring and later saving the 400MB+ npm cache
takes 13 and 14 seconds, respectively:
https://app.circleci.com/pipelines/github/apollographql/apollo-client/17326/workflows/4db5bc1c-718e-4f0b-b6ec-7089b9aa8b55/jobs/94670

This job took a total of 1 minute 11 seconds, so restoring/saving the
cache accounts for ~38% of the total time.

If removing the caching altogether does not slow anything else down by
that much (27 total seconds), then the caching was not really delivering
on its intended goals, and should be removed.

Update after running CI on this commit:
https://app.circleci.com/pipelines/github/apollographql/apollo-client/17327/workflows/0c21e96a-736e-48f2-b2a9-55dea5405342/jobs/94672

Now only 1 minute 1 second (noticeably less time without any caching)!

The "Spin up environment" step went from taking 2s to 19s, because the
circleci/node@16.13.1 Docker image needed to be re-pulled. This seems
like something that could be addressed separately, if we care.

The `npm ci` step takes a little longer, 9s instead of 4s, but that's
presumably because everything has to be installed without a cache.
Still, this is much faster than restoring `node_modules` from cache.
@benjamn
Copy link
Member Author

benjamn commented Oct 19, 2022

Good news: the pulling of the circleci/node Docker image in the "Spin up environment" step seems to have been a one-time slowdown (per branch?), so the latest Filesize job completed in just 43 seconds (down from 71 seconds originally)!

@benjamn benjamn changed the title Experiment with not caching the Filesize Circle CI job Experiment with not caching the ~/.npm directory during Filesize/Tests CircleCI jobs Oct 19, 2022
@benjamn
Copy link
Member Author

benjamn commented Oct 19, 2022

The relative impact for the Tests job is smaller, but 3m43s is faster than 4m11s by about 28 seconds, as before: $(4 \times 60 + 11) - (3 \times 60 + 43) = 28 = 71 - 43$.1

Footnotes

  1. No real reason for using $\LaTeX$ math here. I just never get to use it for anything.2

  2. And footnotes work too!

Comment on lines 10 to 16
# - restore_cache:
# keys:
# # When lock file changes, use increasingly general patterns to
# # restore cache
# - npm-v3-{{ .Branch }}-{{ checksum "package-lock.json" }}
# - npm-v3-{{ .Branch }}-
# - npm-v3-
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to leave these sections commented out for future reference, or just remove the lines and let Git history be our memory?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I generally prefer git history, but not a strong opinion.

Copy link
Member

@alessbell alessbell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like another clear CI win! 🚀

@benjamn benjamn merged commit b4dd365 into main Oct 19, 2022
@benjamn benjamn deleted the try-not-caching-filesize-circle-ci-job branch October 19, 2022 17:37
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants