-
Notifications
You must be signed in to change notification settings - Fork 6.3k
Speed up CI builds with ccache (seeded from develop) #16356
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
base: develop
Are you sure you want to change the base?
Conversation
|
Thank you for your contribution to the Solidity compiler! A team member will follow up shortly. If you haven't read our contributing guidelines and our review checklist before, please do it now, this makes the reviewing process and accepting your contribution smoother. If you have any questions or need our help, feel free to post them in the PR or talk to us directly on the #solidity-dev channel on Matrix. |
| apt-get install -qqy --no-install-recommends \ | ||
| build-essential \ | ||
| cmake \ | ||
| ccache \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have a special workflow for updating buildpack images and this needs to be done in two steps (separate PRs). See buildpack-deps/README.md.
| run_build: | ||
| steps: | ||
| # TODO: remove the line below | ||
| # Before merging this PR we need to use an alternative develop branch to test the functionality, `develop-cache-test` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We want to be able to run without ccache on tags, so we still need to keep a non-cached variant of this command around.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think is the best solution to handle this case? Something like this would work or do we need a more generic solution?
- when:
condition:
and:
- equal: ["", << pipeline.git.tag >>]
steps:
- run:
name: Build (ccache)
command: |
export CCACHE_DIR="$HOME/.ccache"
export CCACHE_BASEDIR="$(pwd)"
export CCACHE_NOHASHDIR=1
mkdir -p "$CCACHE_DIR"
export CMAKE_OPTIONS="${CMAKE_OPTIONS:-} -DCMAKE_C_COMPILER_LAUNCHER=ccache -DCMAKE_CXX_COMPILER_LAUNCHER=ccache"
ccache -z
scripts/ci/build.sh
ccache -s
- when:
condition:
or:
- not:
equal: ["", << pipeline.git.tag >>]
steps:
- run:
name: Build (no ccache)
command: scripts/ci/build.shThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that should work.
I mean, I would have preferred something that directly passes this decision as a parameter from the top, but I that would be annoying, because you'd have to modify most jobs to have parameters. A global version like this does not have this problem.
.circleci/config.yml
Outdated
| export CCACHE_NOHASHDIR=1 | ||
| mkdir -p "$CCACHE_DIR" | ||
| export CMAKE_OPTIONS="${CMAKE_OPTIONS:-} -DCMAKE_C_COMPILER_LAUNCHER=ccache -DCMAKE_CXX_COMPILER_LAUNCHER=ccache" | ||
| ccache -z || true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we expect not to have this on any of the platforms?
Is that platform Windows? :P
.circleci/config.yml
Outdated
| equal: ["develop-cache-test", << pipeline.git.branch >>] | ||
| steps: | ||
| - save_cache: | ||
| key: v1-ccache-{{ arch }}-develop-cache-test-{{ .Revision }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I expect that we might want to invalidate cache whenever we suspect something weird is going on. It would be convenient to be able to do that without having to merge a PR. In an extreme case we might even not be able to merge one due to required jobs failing because of cache. In addition to the hard-coded version we could also keep a variable in CircleCI config that we can bump to get the same effect:
| key: v1-ccache-{{ arch }}-develop-cache-test-{{ .Revision }} | |
| key: v1-{{ .Environment.CCACHE_KEY }}-ccache-{{ arch }}-develop-cache-test-{{ .Revision }} |
.circleci/config.yml
Outdated
| - v1-ccache-{{ arch }}- | ||
| - run: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we usually add a warning here because not everyone may realize that changing the code between restore_cache and save_cache doe not invalidate the cache automatically.
| - v1-ccache-{{ arch }}- | |
| - run: | |
| - v1-ccache-{{ arch }}- | |
| # WARNING! If you edit anything between here and save_cache, remember to bump version or invalidate the cache manually. | |
| - run: |
.circleci/config.yml
Outdated
| - when: | ||
| condition: | ||
| not: | ||
| equal: ["develop-cache-test", << pipeline.git.branch >>] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The breaking branch needs special treatment as well. It's not a feature branch but currently would be treated like one. It's like develop, but for the next breaking version.
- improve comments and warnings - treat ccache as always existing - add CCACHE_KEY to easily invalidate cache - skip cache also on "breaking" branch
|
|
|
|
|
|
|
|
|
|
For the purpose of testing instead of creating the cache from
develop, the cache is created by this branchdevelop-cache-test. We can later create another PR starting from this branch to see if the builds are indeed using the cache generated bydevelop-cache-test, before merging this PR let's remember to fix the occurrences ofdevelop-cache-testin the code.