CI: Cache config.cache across runs to speed up build #104800

hugovk · 2023-05-23T15:05:56Z

Run ./configure with the --config-cache to create a cache file on Ubuntu and macOS jobs, and use https://github.com/actions/cache to re-use it between runs:

uses: actions/cache@v3
with:
  path: config.cache
  key: ${{ github.job }}-${{ hashFiles('configure') }}-${{ hashFiles('configure.ac') }}

When the key changes, the action will upload a fresh cache file.

So we have a unique cache file per job ID (github.job, for example: build_macos, build_ubuntu_ssltests), and we'll also get a fresh cache file whenever the configure or configure.ac files change.

Seconds to download cache + configure:

Job	Without cache	With cache	Times faster	Seconds saved
check_generated_files	22	4	5.5	18
build_macos	96	17	5.6	79
build_ubuntu	23	4	5.8	19
build_ubuntu_ssltests (1.1.1t)	24	4	6.0	20
build_ubuntu_ssltests (3.0.8)	25	8	3.1	17
build_ubuntu_ssltests (3.0.8)	25	7	3.6	18
test_hypothesis	24	4	6.0	20
build_asan	29	5	5.8	24

Total:	268	53	5.1	215
	link	link

itamaro · 2023-05-23T15:47:55Z

where is the cache file stored? is it using per-repo or per-org storage?
wondering about that, thinking how it will behave in forks.

I'm not familiar with config.cache - are the configure and configure.ac files the only relevant inputs?
what's the impact on total CI runtime when there's a cache hit? cache miss?

I also tried thinking about cross-branch and cross-PR interactions, but since the key uses the hash of the input files then if the hashes are identical across branches and PRs then it should be ok to reuse the cached file.

one concern is implicit inputs that are not captured by the current key:

the "job name" is a proxy to flags that affect the result, but it is possible that flags change and the job name stays the same
the versions of the github actions used may change and implicitly affect the result, but afaik that is not captured in the cache key

hugovk · 2023-05-23T16:03:13Z

where is the cache file stored? is it using per-repo or per-org storage? wondering about that, thinking how it will behave in forks.

https://github.com/actions/cache does it per-org, so it won't be shared with forks. Forks will have their own copies.

I'm not familiar with config.cache - are the configure and configure.ac files the only relevant inputs?

Let's ask @erlend-aasland :)

what's the impact on total CI runtime when there's a cache hit? cache miss?

I ran this PR twice: once with no cache, and second time with cache present.

See the "Post Restore config.cache" steps at the end for uploading a cache in the first run: they take either 0 or 1 seconds.
See the "Restore config.cache" steps at the start for restoring a cache in the second run: they take either 0 or 1 seconds.

I also tried thinking about cross-branch and cross-PR interactions, but since the key uses the hash of the input files then if the hashes are identical across branches and PRs then it should be ok to reuse the cached file.

one concern is implicit inputs that are not captured by the current key:

the "job name" is a proxy to flags that affect the result, but it is possible that flags change and the job name stays the same

Could you give an example?

the versions of the github actions used may change and implicitly affect the result, but afaik that is not captured in the cache key

I think the action will invalidate the cache itself if they change something, but I couldn't find anything to confirm. I also don't think the action's version is exposed, anyway.

arhadthedev · 2023-05-23T16:29:16Z

Could you give an example?

check_generated_files: --config-cache --with-pydebug --enable-shared
build_macos: --config-cache --with-pydebug --prefix=/opt/python-dev --with-openssl="$(brew --prefix openssl@1.1)"
build_ubuntu, build_ubuntu_ssltests, test_hypothesis: --config-cache --with-pydebug --with-openssl=$OPENSSL_DIR
build_asan: --config-cache --with-address-sanitizer --without-pymalloc

Also, environment variables:

build_macos:
- HOMEBREW_NO_ANALYTICS: 1
- HOMEBREW_NO_AUTO_UPDATE: 1
- HOMEBREW_NO_INSTALL_CLEANUP: 1
- PYTHONSTRICTEXTENSIONBUILD: 1
build_ubuntu, test_hypothesis:
- OPENSSL_VER: 1.1.1t
- PYTHONSTRICTEXTENSIONBUILD: 1
build_ubuntu_ssltests:
- OPENSSL_VER: ${{ matrix.openssl_ver }}
- MULTISSL_DIR: ${{ github.workspace }}/multissl
- OPENSSL_DIR: ${{ github.workspace }}/multissl/openssl/${{ matrix.openssl_ver }}
- LD_LIBRARY_PATH: ${{ github.workspace }}/multissl/openssl/${{ matrix.openssl_ver }}/lib
build_asan:
- OPENSSL_VER: 1.1.1t
- PYTHONSTRICTEXTENSIONBUILD: 1
- ASAN_OPTIONS: detect_leaks=0:allocator_may_return_null=1:handle_segv=0

itamaro · 2023-05-23T16:34:06Z

@arhadthedev provided examples :)

the result can also be affected by the exact OS version (I don't know how granular runner.os is), which toolchain is used (gcc/clang), toolchain versions, and host architecture. any changes in any of these should invalidate the cache.

regarding the implementation - hashFiles can operate on multiple files, so use hashFiles(file1, file2) instead.
also for a given commit, the hash of these files is constant - would it be possible to extract the hash computation to perform it once and share between jobs?

…ions changed

hugovk · 2023-05-23T18:03:13Z

@arhadthedev provided examples :)

👍

the result can also be affected by the exact OS version (I don't know how granular runner.os is), which toolchain is used (gcc/clang), toolchain versions, and host architecture. any changes in any of these should invalidate the cache.

runner.os: "The operating system of the runner executing the job. Possible values are Linux, Windows, or macOS."

https://docs.github.com/en/actions/learn-github-actions/contexts#runner-context

I added the workflow file to hashFiles, so it will invalidate and make a new cache when things defined in the file change, for example the env vars and CLI flags.

There's a runner.arch: "The architecture of the runner executing the job. Possible values are X86, X64, ARM, or ARM64" but we're using defaults and don't expect these will change?

regarding the implementation - hashFiles can operate on multiple files, so use hashFiles(file1, file2) instead.

Combined.

also for a given commit, the hash of these files is constant - would it be possible to extract the hash computation to perform it once and share between jobs?

Possibly but I'm not sure if it's worth the complexity: we can just have hashFiles computed each time.

itamaro · 2023-05-23T18:44:02Z

LG!

I added the workflow file to hashFiles, so it will invalidate and make a new cache when things defined in the file change, for example the env vars and CLI flags.

this will be over-conservative, invalidating for any change in the workflow file, even if it doesn't affect the result (which is probably most changes). probably a fair trade off (preferring correctness over CI latency).

Possibly but I'm not sure if it's worth the complexity: we can just have hashFiles computed each time.

agreed. I'd suggest at least refactoring the list of files to be a shared constant.

erlend-aasland

LGTM. With the workflow file in the cache key, we're pretty safe regarding any environment change.

erlend-aasland · 2023-05-23T21:43:58Z

I guess we want this CI speedup in the other branches as well.

hugovk · 2023-05-24T09:59:10Z

I'd suggest at least refactoring the list of files to be a shared constant.

Do you know how to do this? I tried a few things, but didn't get it working.

itamaro · 2023-05-24T14:38:56Z

Do you know how to do this? I tried a few things, but didn't get it working.

I think workflow-level environment variable can work for this

https://docs.github.com/en/actions/learn-github-actions/variables

hugovk · 2023-05-24T14:48:24Z

I tried that like:

env:
  CONFIG_CACHE_HASH_FILES: ${{ hashFiles('configure', 'configure.ac', '.github/workflows/build.yml') }}

and:

          key: ${{ github.job }}-${{ runner.os }}-${{ env.CONFIG_CACHE_HASH_FILES }}

But it said "The workflow is not valid ", "Unrecognized function: 'hashFiles'"

I also tried:

env:
  CONFIG_CACHE_HASH_FILES: ('configure', 'configure.ac', '.github/workflows/build.yml')

and:

          key: ${{ github.job }}-${{ runner.os }}-${{ hashFiles(env.CONFIG_CACHE_HASH_FILES) }}

But it couldn't read the env var:

Cache not found for input keys: check_generated_files-Linux-

Let me try again with format():

https://github.com/orgs/community/discussions/25761#discussioncomment-4626908

hugovk · 2023-05-24T15:22:16Z

Hmm, no luck, because it formats into strings, and we need a sort of tuple of strings.

We might be able to do something like:

env:
  CONFIG_CACHE_HASH_FILE1: 'configure'
  CONFIG_CACHE_HASH_FILE2: 'configure.ac'
  CONFIG_CACHE_HASH_FILE3: '.github/workflows/build.yml'

And then format it something like this:

          key: ${{ github.job }}-${{ runner.os }}-${{ hashFiles(format('{0}', env.CONFIG_CACHE_HASH_FILE1))-${{ hashFiles(format('{0}', env.CONFIG_CACHE_HASH_FILE2))-${{ format('{0}', env.CONFIG_CACHE_HASH_FILE3)) }}

But it feels like a backward refactor...

itamaro · 2023-05-24T15:28:38Z

But it feels like a backward refactor...

yeah, I agree, this doesn't improve anything. let's leave it as explicit lists of files, I might give it another try separately after this merges.

erlend-aasland · 2023-05-25T18:08:29Z

Backport through all security branches?

miss-islington · 2023-05-26T07:45:35Z

Sorry @hugovk, I had trouble checking out the 3.12 backport branch.
Please retry by removing and re-adding the "needs backport to 3.12" label.
Alternatively, you can backport using cherry_picker on the command line.
cherry_picker 1080c4386dd3beb9ee808acdf3c3f01835f73860 3.12

…-104800) (cherry picked from commit 1080c43) Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>

bedevere-bot · 2023-05-26T08:51:28Z

GH-104967 is a backport of this pull request to the 3.12 branch.

bedevere-bot · 2023-05-26T08:56:28Z

GH-104968 is a backport of this pull request to the 3.11 branch.

…-104800).\n(cherry picked from commit 1080c43)\n\n\nCo-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>

miss-islington · 2023-05-26T08:57:05Z

Thanks @hugovk for the PR 🌮🎉.. I'm working now to backport this PR to: 3.7.
🐍🍒⛏🤖

miss-islington · 2023-05-26T08:57:05Z

Thanks @hugovk for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

miss-islington · 2023-05-26T08:57:05Z

Thanks @hugovk for the PR 🌮🎉.. I'm working now to backport this PR to: 3.9.
🐍🍒⛏🤖

miss-islington · 2023-05-26T08:57:05Z

Thanks @hugovk for the PR 🌮🎉.. I'm working now to backport this PR to: 3.10.
🐍🍒⛏🤖

miss-islington · 2023-05-26T08:57:07Z

Sorry, @hugovk, I could not cleanly backport this to 3.7 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 1080c4386dd3beb9ee808acdf3c3f01835f73860 3.7

miss-islington · 2023-05-26T08:57:08Z

Sorry @hugovk, I had trouble checking out the 3.9 backport branch.
Please retry by removing and re-adding the "needs backport to 3.9" label.
Alternatively, you can backport using cherry_picker on the command line.
cherry_picker 1080c4386dd3beb9ee808acdf3c3f01835f73860 3.9

miss-islington · 2023-05-26T08:57:27Z

Sorry, @hugovk, I could not cleanly backport this to 3.8 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 1080c4386dd3beb9ee808acdf3c3f01835f73860 3.8

miss-islington · 2023-05-26T08:57:29Z

Sorry @hugovk, I had trouble checking out the 3.10 backport branch.
Please retry by removing and re-adding the "needs backport to 3.10" label.
Alternatively, you can backport using cherry_picker on the command line.
cherry_picker 1080c4386dd3beb9ee808acdf3c3f01835f73860 3.10

hugovk · 2023-05-26T10:00:06Z

Actually, maybe we should skip the security branches?

They've already diverged quite a bit, and don't get built (or backported) as often, so there's less benefit.

erlend-aasland · 2023-05-26T13:38:54Z

Actually, maybe we should skip the security branches?

They've already diverged quite a bit, and don't get built (or backported) as often, so there's less benefit.

Yeah, makes sense.

… (#104967)

… (#104968) Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>

itamaro · 2023-05-27T01:32:16Z

I might give it another try separately after this merges.

I think it worked out nicely in gh-105008

CI: Cache config.cache across runs to speed up build

6b4bd9e

hugovk added skip issue skip news labels May 23, 2023

hugovk marked this pull request as ready for review May 23, 2023 15:44

hugovk requested a review from ezio-melotti as a code owner May 23, 2023 15:44

bedevere-bot added the awaiting core review label May 23, 2023

hugovk added 3 commits May 23, 2023 20:17

Combine hashFiles

7ef178c

Add missing runner.os

539a1a0

Include workflow file in key, to invalidate when env vars and OS vers…

5962941

…ions changed

erlend-aasland approved these changes May 23, 2023

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting core review labels May 23, 2023

hugovk merged commit 1080c43 into python:main May 25, 2023

hugovk deleted the ci-config.cache branch May 25, 2023 11:09

bedevere-bot removed the awaiting merge label May 25, 2023

hugovk added needs backport to 3.11 only security fixes needs backport to 3.12 bug and security fixes labels May 26, 2023

hugovk added a commit to hugovk/cpython that referenced this pull request May 26, 2023

[3.12] CI: Cache config.cache across runs to speed up build (pythonGH…

b52bf2b

…-104800) (cherry picked from commit 1080c43) Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>

bedevere-bot removed the needs backport to 3.12 bug and security fixes label May 26, 2023

bedevere-bot removed the needs backport to 3.11 only security fixes label May 26, 2023

hugovk added needs backport to 3.7 needs backport to 3.9 only security fixes needs backport to 3.10 only security fixes labels May 26, 2023

erlend-aasland removed needs backport to 3.7 needs backport to 3.9 only security fixes needs backport to 3.10 only security fixes labels May 26, 2023

hugovk added a commit that referenced this pull request May 26, 2023

[3.12] CI: Cache config.cache across runs to speed up build (GH-104800)…

97509ca

… (#104967)

hugovk added a commit that referenced this pull request May 26, 2023

[3.11] CI: Cache config.cache across runs to speed up build (GH-104800)…

4af56ec

… (#104968) Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>

itamaro mentioned this pull request May 27, 2023

CI: Precompute hash for config cache key in check_source job #105008

Merged

erlend-aasland mentioned this pull request Jun 6, 2023

3.12 backport gh 105236 #105358

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CI: Cache config.cache across runs to speed up build #104800

CI: Cache config.cache across runs to speed up build #104800

hugovk commented May 23, 2023 •

edited

Loading

itamaro commented May 23, 2023

hugovk commented May 23, 2023

arhadthedev commented May 23, 2023 •

edited

Loading

itamaro commented May 23, 2023

hugovk commented May 23, 2023

itamaro commented May 23, 2023 •

edited

Loading

erlend-aasland left a comment

erlend-aasland commented May 23, 2023

hugovk commented May 24, 2023

itamaro commented May 24, 2023

hugovk commented May 24, 2023

hugovk commented May 24, 2023

itamaro commented May 24, 2023

erlend-aasland commented May 25, 2023

miss-islington commented May 26, 2023

bedevere-bot commented May 26, 2023

bedevere-bot commented May 26, 2023

miss-islington commented May 26, 2023

miss-islington commented May 26, 2023

miss-islington commented May 26, 2023

miss-islington commented May 26, 2023

miss-islington commented May 26, 2023

miss-islington commented May 26, 2023

miss-islington commented May 26, 2023

miss-islington commented May 26, 2023

hugovk commented May 26, 2023

erlend-aasland commented May 26, 2023

itamaro commented May 27, 2023

CI: Cache config.cache across runs to speed up build #104800

CI: Cache config.cache across runs to speed up build #104800

Conversation

hugovk commented May 23, 2023 • edited Loading

itamaro commented May 23, 2023

hugovk commented May 23, 2023

arhadthedev commented May 23, 2023 • edited Loading

itamaro commented May 23, 2023

hugovk commented May 23, 2023

itamaro commented May 23, 2023 • edited Loading

erlend-aasland left a comment

Choose a reason for hiding this comment

erlend-aasland commented May 23, 2023

hugovk commented May 24, 2023

itamaro commented May 24, 2023

hugovk commented May 24, 2023

hugovk commented May 24, 2023

itamaro commented May 24, 2023

erlend-aasland commented May 25, 2023

miss-islington commented May 26, 2023

bedevere-bot commented May 26, 2023

bedevere-bot commented May 26, 2023

miss-islington commented May 26, 2023

miss-islington commented May 26, 2023

miss-islington commented May 26, 2023

miss-islington commented May 26, 2023

miss-islington commented May 26, 2023

miss-islington commented May 26, 2023

miss-islington commented May 26, 2023

miss-islington commented May 26, 2023

hugovk commented May 26, 2023

erlend-aasland commented May 26, 2023

itamaro commented May 27, 2023

hugovk commented May 23, 2023 •

edited

Loading

arhadthedev commented May 23, 2023 •

edited

Loading

itamaro commented May 23, 2023 •

edited

Loading