Skip to content

Add Submodule Caching to Toolchain Generation - #1607

Closed
TShapinsky wants to merge 5 commits into
riscv-collab:masterfrom
TShapinsky:cache-submodules
Closed

Add Submodule Caching to Toolchain Generation#1607
TShapinsky wants to merge 5 commits into
riscv-collab:masterfrom
TShapinsky:cache-submodules

Conversation

@TShapinsky

@TShapinsky TShapinsky commented Nov 2, 2024

Copy link
Copy Markdown

Adds a job at the beginning of the build workflow which updates caches containing the different submodules required to build the different variants.

This job takes about 3 minutes to run the first time, and after that it takes about 4 seconds to check that the cache is still up to date. This could also be moved to a different trigger which runs less often.

This greatly reduces the amount of resources used to clone submodules (just for the build variants for now). The clones that do happen are shallow (except for uclibc-ng) which also shrinks the git overhead by at least 1GB.

This action only caches the .git/modules directory as the rest of the working directory can be rebuilt from those without needing to download anything else. This allows us to choose which submodules we want to manifest into our build environment. The directory sizes for GCC and LLVM are 1.1GB and 1.9GB respectively, so being able to pick which we want in our environment saves a lot of disk space.

An additional note on github caches. They are scoped to branches, but any branch can restore a cache that was saved by master. Repos are limited to 10GB of total cache storage, so restricting cache generation to master may also be a good move to prevent using all of the cache storage capacity.

@TShapinsky

Copy link
Copy Markdown
Author

Workflow can be seen here

@TShapinsky
TShapinsky force-pushed the cache-submodules branch 4 times, most recently from 9351157 to 9f469aa Compare November 2, 2024 20:56

@TommyMurphyTM1234 TommyMurphyTM1234 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Minor comments

Comment thread .github/workflows/build.yaml Outdated
id: submodule-hash
run: echo "HASH=$(git submodule | sha1sum | head -c 40)" >> $GITHUB_OUTPUT

- name: Check is Cache Exists for Exact Submodule Configuration

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Typo? "is" should be "if"?

Very minor cosmetic (pedantic?!) issue - why the capitalisation of words in comments (here and elsewhere)?

@TShapinsky TShapinsky Nov 2, 2024

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thanks for catching the typo. I was using title case because they appear in the CI as the titles of each step. But if the standard here is all lower case or sentence case I'll happily switch to that.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for catching the typo. I was using title case because they appear in the CI as the titles of each step. But if the standard here is all lower case or sentence case I'll happily switch to that.

Ah - they're names of the steps/tasks. I mistakenly thought that they were comments. If that's the convention then it makes sense to adhere to it. But should every word start with a capital letter? Not all do - e.g. "Check if Cache Exists for Exact Submodule Configuration", "If no Cache Hit, Update Cache", "Clone submodules", etc. Maybe prepositions are exempt from the rule? :-)

@TShapinsky TShapinsky Nov 2, 2024

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yeah, maybe just the first word should be capital. Title case is what I was going for, link, which does not capitalize prepositions or conjunctions.

@TShapinsky
TShapinsky force-pushed the cache-submodules branch 2 times, most recently from 3b609a8 to 06a5cf2 Compare November 2, 2024 21:00
@TommyMurphyTM1234

Copy link
Copy Markdown
Collaborator

Is this PR still needed or is it made redundant by this one which has been merged?

@cmuellner

Copy link
Copy Markdown
Collaborator

Thanks for the contribution!
Closing this as superseded by #1608 and the current reusable CI workflow.
The current build workflow already has submodule caching keyed by the
submodule hash, shallow submodule checkout on cache miss, and cache
restore in the build jobs.

@cmuellner cmuellner closed this May 14, 2026
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