[ci] Use GH cache for bitstream PRs - #31260
Conversation
| id: gh-cache-key | ||
| if: steps.strategy.outputs.bitstreamStrategy != 'cached' | ||
| run: | | ||
| hash_gen_command="//hw/bitstream/vivado:fpga_${{ inputs.design_suffix }}_hash" |
There was a problem hiding this comment.
Just a small note that this caching strategy has one (unavoidable) flaw: changes to the fusesoc command line or fusesoc rule will not change the hash and hence the bitstream might not get rebuild in those cases.
I don't know how likely it is to happen, we could potentially modify the rule to hash more stuff (like part of the command line if necessary) but there will always be a "gap" which can only be solved by using the bazel action cache (maybe a future work, I have some ideas for this).
Maybe github having a label to force a bitstream rebuild could solve this in cases where we know we want to rebuild
There was a problem hiding this comment.
Thanks for pointing this out. I incorporated this note in a follow up issue that I just created #31264.
I guess this may cause surprises in commits that touch the build system in this way. Do you think the other caching mechanisms we have are subject to the same (or similar) flaw? I guess the diff mechanism used for the GCS bitstream cache may miss updates to the tool itself, which is probably tolerable.
I would be happy if the GH cache mechanism was - at least - not worse than the one we already have (in terms of missing changes to the bitstream). I guess adding some BUILD files could be enough. Or - as you suggest - add parts of the command line. Not very sure how to do this, though.
There was a problem hiding this comment.
We discussed in the office about this. @engdoreis thinks that in practice this shouldn't be an issue. Well, he said it never has been so far. I'll look into adding a label to force rebuilding the bitstream and update the documentation on bitstream caching.
There was a problem hiding this comment.
The bitstream strategy computed by the script rebuilds on any rule file change. This means that it will rebuild too often than necessary but it also means that it doesn't have this flaw. I can add the command line hash in another PR. I don't think it's a big problem though as long as the merge run does not use the GH cache.
50b4933 to
bc1af4e
Compare
Use the GitHub cache to reuse FPGA bitstreams built during the review lifetime of a PR. This allows to skip rebuilding the bitstream when pushing a PR update that doesn't touch any of the bitstream sources. Note that this caching mechanism only helps PRs that require rebuilding the bitstream and undergo several review cycles which don't need the bitstream to be rebuilt. The GitHub cache is disabled on PR merges, meaning that the bitstream is always recomputed at merge time. NOTE: The GitHub cache is indexed using the SHA built by the recently introduced `//hw/bitstream/vivado:fpga_cw340_hash` Bazel target. This minimizes bitstream rebuilds if compared to other caching mechanisms we rely on (e.g. the GCS caching). However, it currently has a flaw: the bitstream is not rebuilt when the only thing to change is the FuseSoC command-line. Indeed, the cache indexing is based exclusively on the source files that are used by FuseSoC to build the bitstream. Signed-off-by: Matteo Franchin <matteo.franchin@lowrisc.org>
This name is not well suited now that we have multiple bitstream caches. Rename this script to check-bitstream-gcs-cache.sh to indicate that what it does is to just check whether we can reuse a bistream from the Google Cloud Storage (GCS) cache. Signed-off-by: Matteo Franchin <matteo.franchin@lowrisc.org>
In particular, add details on how the GitHub cache is used and what it aims to achieve. Signed-off-by: Matteo Franchin <matteo.franchin@lowrisc.org>
mfnch
changed the title
|
Something is not clear to me but that might be due to my lack of knowledge about github caching: where is the bitstream upload to the cache? Is the caching mechanism looking for previously built artifacts of the same PR? |
It all happens here: - name: Extract cached bitstream from GitHub cache
uses: actions/cache@v6
with:
path: build-bin.tar
key: ABCThis action checks whether an entry exists in the GitHub cache with key In the example run I posted this action can be seen running as one of the final steps named "Post Extract cached bitstream from GitHub cache". So a single action implements the whole caching mechanism... relying on the job will actually producing itself the |
3 participants
Use the GitHub action cache to save bitstreams during a PR's review lifetime. This is useful for PRs that change the bitstream. The GitHub cache saves the bitstream in early PR uploads so that it can be reused in later revisions where possible, thus alleviating CI wait times during reviews.
NOTES: The GitHub cache is indexed using the SHA built by the recently introduced
//hw/bitstream/vivado:fpga_cw340_hashBazel target (#31041). This minimizes bitstream rebuilds if compared to other cachingmechanisms we rely on (e.g. the GCS caching). However, it currently has a flaw: the bitstream is not rebuilt when the only thing to change is the FuseSoC command-line. Indeed, the cache indexing is based exclusively on the source files that are used by FuseSoC to build the bitstream.
Note also that the GitHub action cache is scoped to the repository branch and therefore is not reused across different PRs.
Cached items are shown here
Testing:
This was done by adding temporary testing commits to this PR (removed in the final revision):
Closes: #31059