Skip to content

CircleCI: improve caching for faster startup time #3612

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

Merged
merged 2 commits into from
Mar 7, 2019
Merged

Conversation

antoinerg
Copy link
Contributor

@antoinerg antoinerg commented Mar 6, 2019

This PR slightly improves the execution time of our CircleCi :

  1. I removed the steps to store and restore the cache because they were ineffective (the cache key would never match). We can reintroduce them later if we really want to save off a few seconds out of running npm i.

  2. Instead of doing a long git checkout process for every jobs, we pass the content of the plotly.js directory to subsequent jobs after build. This add rougly 5 seconds to the persist_to_workspace step. However, we reduce the startup time of all the following jobs by ~ 18 seconds:
    newplot 22

This will make running tests in parallel more efficient and save us N * 18 seconds of computation time each run.

@antoinerg antoinerg changed the title circleci: 🔪 unused cache key, copy git repo 🐎 CircleCI: improve caching for faster startup time Mar 6, 2019
@@ -14,10 +14,6 @@ jobs:
working_directory: ~/plotly.js
steps:
- checkout
- restore_cache:
keys:
- v{{ .Environment.CIRCLE_CACHE_VERSION }}-deps-{{ .Branch }}-{{ checksum "package-lock.json" }}
Copy link
Contributor Author

@antoinerg antoinerg Mar 7, 2019

Choose a reason for hiding this comment

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

Notice here we use the checksum of "package-lock.json" but when we save the cache, we use the checksum of "package.json". Those two cache names will never match so this is why I removed this "dead" code.

@etpinard
Copy link
Contributor

etpinard commented Mar 7, 2019

Nice find. Great work!

I'm curious though: can you try replacing our npm install step with npm ci, and see if we can 🔪 a few more seconds?

@antoinerg
Copy link
Contributor Author

I'm curious though: can you try replacing our npm install step with npm ci, and see if we can hocho a few more seconds?

npm ci seems faster: roughly 6 seconds versus 15-18 seconds for npm i. I will add it to the current PR.

@etpinard
Copy link
Contributor

etpinard commented Mar 7, 2019

npm ci seems faster: roughly 6 seconds versus 15-18 seconds for npm i

🎉 !

@antoinerg
Copy link
Contributor Author

Now using npm ci in 98c962d !

@etpinard
Copy link
Contributor

etpinard commented Mar 7, 2019

💃 💃

@antoinerg antoinerg merged commit 7e68a2e into master Mar 7, 2019
@antoinerg antoinerg deleted the ci-improve-cache branch March 7, 2019 20:05
@alexcjohnson
Copy link
Collaborator

cc @Marc-Andre-Rivet re: npm ci - nice find @etpinard @antoinerg 🏆

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

Successfully merging this pull request may close these issues.

3 participants