-
Notifications
You must be signed in to change notification settings - Fork 693
Cache loaded images for performance improvements. #1934
Cache loaded images for performance improvements. #1934
Conversation
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here with What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
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.
Thanks for the performance fix! Can you sign the CLA?
CI compile failure Also it wants a lint fix |
e29901e
to
8907c36
Compare
@googlebot I signed it! |
8907c36
to
c3e7e90
Compare
Locally on ubuntu 18.04, the join_layers step takes greater than 30 seconds for the container_bundle_with_install_pkgs target without this change, and ~5 seconds with this change. With the previous implementation, join_layers was passing the same set of images to reader.parts for each call to ReadImage. The reader, created fresh for each call to ReadImage, would then load these same images again. This results in a fixed cost for every image you add to the bundle. For large bundles, this can be very costly (on the order of minutes). Updated other callers of compat.ReadImages to use this new API of creating a reader first, then calling reader.ReadImage.
c3e7e90
to
b7753ad
Compare
Seems like too much of a coincidence for this to timeout on my branch (all recent builds seem to be successful). However, I can't seem to reproduce this locally. I figure I probably broke something with pushing or pulling docker images?
|
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. Other performance improvements are welcome. I'd love it if we had someone look at converting build_tar.py to golang.
Incoming upstream change: bazelbuild/rules_docker#1934
Locally on ubuntu 18.04, the join_layers step takes
With the previous implementation, join_layers was passing the same
set of images to reader.parts for each call to ReadImage. The reader,
created fresh for each call to ReadImage, would then load these same
images again.
This results in a fixed cost for every image you add to the bundle. For
large bundles, this can be very costly (on the order of minutes).
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information