Skip to content
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

We should cache ExpandAPK, not just the apk #772

Closed
Tracked by #780 ...
jonjohnsonjr opened this issue Jun 30, 2023 · 1 comment · Fixed by chainguard-dev/go-apk#77
Closed
Tracked by #780 ...

We should cache ExpandAPK, not just the apk #772

jonjohnsonjr opened this issue Jun 30, 2023 · 1 comment · Fixed by chainguard-dev/go-apk#77

Comments

@jonjohnsonjr
Copy link
Contributor

When we fetch an APK, we fetch, expand, and install.

See gcc:

image

When we have a cache hit, we skip the fetch, but we still do the expand:

image

That 1.2s does not change across image builds and is fairly expensive, so I propose we cache the outputs of ExpandAPK (each section as one .tar.gz file) instead of caching the entire APK as a single file.

This would cut out approximately half the work apko has to do when we have cache hits.

@jonjohnsonjr
Copy link
Contributor Author

jonjohnsonjr commented Jul 7, 2023

Working towards this, we see a another large reduction in CPU usage:

apko publish --keyring-append  --repository-append  --arch amd64    10.81s user 3.05s system 181% cpu 7.639 total

Previous run was:

apko publish --keyring-append  --repository-append  --arch amd64    13.46s user 4.56s system 227% cpu 7.917 total

(Note that these are all with warm caches, just re-running the same thing.)

Overall latency is roughly the same because we already mitigated the ExpandAPK latency with chainguard-dev/go-apk#75, but we are doing 20% less work than before, which should translate to more throughput.

What's nice is that this work caching will apply to all builds, so when we rebuild the world, we will only pay the ExpandAPK price once per APK instead of every time a build depends on an APK.

I intend to remove writing the whole APK to the cache (in the cache transport), but I'll keep it on the read path so that I don't bust everyone's cache when we merge this. Dropping it from the write path is important because otherwise we are keeping two copies of the data needlessly.

Cache hits (cachedPackage succeeding) are now ~instantaneous instead of also doing ExpandAPK:

image

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 a pull request may close this issue.

1 participant