-
Notifications
You must be signed in to change notification settings - Fork 20
Accelerate via @actions/cache #8
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
Conversation
@dennisameling I would also love to have your 👀 over this PR... |
I am currently testing this by running a full matrix here: https://github.com/dscho/setup-git-for-windows-sdk/runs/1925713769?check_suite_focus=true. Once that run is done (and so far, it looks pretty good, having cached 4 out of the 6 artifacts), I will push a new commit to the |
Wow, caching is not exactly cheap, either: the full 32-bit SDK takes ~2m45s to cache 914MB... in contrast, it only takes ~15s to cache the ~75MB of |
Yes!!! The subsequent run manages to restore the minimal SDK in 10s instead of the 1m4s in the uncached case. For the In contrast, the full SDKs seem not to benefit all that much. A run from yesterday (without caching the just-downloaded artifacts) clocked the 32-bit and 64-bit full SDK at ~6m and ~8m20s, respectively. When restoring them from cache, the numbers are ~5m and ~8m15s, respectively. Given these findings, I think we will want to disable the cache for the full SDKs by default because it does not really seem to give us a big bang for the buck. |
This PR is now based on #9, which should be merged first. |
And here is a run that exercises the full supported flavor/architecture matrix with the default caching. I think this is good to go now. |
This looks great, but I'm not sure about one thing...
For
Are you sure that's correct? |
Yes, that's correct. The |
This opt-out feature uses the cache, so that the files are cached the first time a given artifact from a given buildId is downloaded. Subsequent attempts to download the same artifact with the same buildId should become much quicker as a consequence. Signed-off-by: Johannes Schindelin <[email protected]>
Some testing reveals that the 64-bit and the 32-bit `full` SDK weigh in with ~1,300MB and ~915MB at time of writing, respectively. Packing and saving these to the cache takes a long time: ~5m20s and ~2m45s. Given that the non-cached download takes ~8m20s and ~6m, respectively, and restoring the artifacts from cache takes ~8m15s and ~5m (which is not a lot faster, not by a long stretch), it is neither worth the time nor the disk space to cache the `full` SDKs. Let's introduce a new default value for the input parameter `cache`, `auto`, that turns on caching except for the `full` flavor. Signed-off-by: Johannes Schindelin <[email protected]>
Signed-off-by: Johannes Schindelin <[email protected]>
Rebased (and regenerated |
With the question why 500MB are equivalent to 1,300MB out of the way, I interpret this as your 👍 to merging this ;-) |
This change makes this Action truly useful: the first time we try to get a given build artifact, it will be downloaded from Azure Pipelines. But then it will be cached. And subsequent attempts will use the cached version and be a lot faster!