-
Notifications
You must be signed in to change notification settings - Fork 3
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
Manual image caching #374
Manual image caching #374
Conversation
dec1236
to
22088c3
Compare
… dependency images, which rarely change
22088c3
to
892e75d
Compare
an example of the working cache: |
Nice! Would be great to quantify the speed ups :D How long did it take before and now when there are cache hits for everything? I didn't see the arm part here, do you use separate docker files for that? |
How come the linter takes so long? Is there so much work done? Also, I noticed you often compile with 4 parallel threads. GitHub actions runners only have 2. It might improve performance to reduce to 2 in CI |
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.
LGTM
The ci did not run the unit test image. (from the link you posted) Was that aborted by you?
@JonasKellerer it did, or is that something else? https://github.com/GenSpectrum/LAPIS-SILO/actions/runs/8463828054/job/23187878018 the unit test image is called builder and used in the test run AFAICT: LAPIS-SILO/.github/workflows/ci.yml Lines 201 to 217 in 7867bc7
|
I cancelled it after I saw that it hit the cache in the necessary job. I then just squash and rebased the commit for the PR. Then it rebuilt both dependency images as expected, as duckdb->0.10.1 dependency change was already on the changed main |
@corneliusroemer the quantification is not as straight forward, as the cache was hit also before but only sometimes Mostly the first dependency jobs, which take 20 mins each (although in parallel) should now be saved, if no dependency were changed. In the attached screenshot the upper pipeline would be shortened by 21m2s, and the lower one hit the cache by itself by chance |
faster builds by copying @corneliusroemer image caching for our dependency images, which rarely change