Skip to content
This repository has been archived by the owner on Oct 2, 2023. It is now read-only.

Cache loaded images for performance improvements. #1934

Merged

Conversation

aptenodytes-forsteri
Copy link
Contributor

@aptenodytes-forsteri aptenodytes-forsteri commented Oct 6, 2021

Locally on ubuntu 18.04, the join_layers step takes

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).

PR Checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@google-cla
Copy link

google-cla bot commented Oct 6, 2021

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 @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@google-cla google-cla bot added the cla: no label Oct 6, 2021
Copy link
Collaborator

@alexeagle alexeagle left a 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?

@alexeagle
Copy link
Collaborator

CI compile failure
docker/container/go/cmd/digester/digester.go:52:14: undefined: compat.ReadImage

Also it wants a lint fix

@aptenodytes-forsteri
Copy link
Contributor Author

@googlebot I signed it!

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.
@aptenodytes-forsteri
Copy link
Contributor Author

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?

(04:21:27) ERROR: An error occurred during the fetch of repository 'alpine_linux_armv6':
  | Traceback (most recent call last):
  | File "/workdir/container/pull.bzl", line 216, column 13, in _impl
  | fail("Pull command failed: %s (%s)" % (result.stderr, " ".join([str(a) for a in args])))
  | Error in fail: Pull command failed: Timed out (/var/lib/buildkite-agent/.cache/bazel/_bazel_buildkite-agent/ec321eb2cc2d0f8f91b676b6d4c66c29/external/go_puller_linux_amd64/file/downloaded -directory /var/lib/buildkite-agent/.cache/bazel/_bazel_buildkite-agent/ec321eb2cc2d0f8f91b676b6d4c66c29/external/alpine_linux_armv6/image -os linux -os-version  -os-features  -architecture arm -variant v6 -features  -name index.docker.io/library/alpine@sha256:dabea2944dcc2b86482b4f0b0fb62da80e0673e900c46c0e03b45919881a5d84)
 ```

Copy link
Collaborator

@gravypod gravypod left a 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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants