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

Why is imgpkg copy is inefficient #65

Closed
gcheadle-vmware opened this issue Jan 28, 2021 · 2 comments
Closed

Why is imgpkg copy is inefficient #65

gcheadle-vmware opened this issue Jan 28, 2021 · 2 comments
Assignees

Comments

@gcheadle-vmware
Copy link
Contributor

gcheadle-vmware commented Jan 28, 2021

Explore why imgpkg copy is inefficient.

The copy command takes a very long time to copy a simple image from a repository to a different one in the same OCI registry.
A small image like hello-world takes around 20 seconds to be copied.

I think this is tied to the network which lead us to think that there might be a download and upload of a layer that is already in the registry.

We should investigate if there is a more efficient way to do the copy of the layers.

For reference projects like pack does manipulation of layers and maybe there is some difference between what we are doing in our code and the way pack is pushing/pulling the images that it uses.

Acceptance Criteria

  1. Determine whether the issue still exists
  2. If so, understand the problem.
  3. Fix the problem if it's easy. Otherwise, create a story to make the fix.
  4. Provide a means for anyone on the team to check imgpkg's performance and easily detect a regression (e.g. add a benchmark test).

Notes

  • this feedback was captured prior to a substantial refactor so this will be worth validating before diving deep
  • consider testing performance against other similar tools
@aaronshurley aaronshurley added this to the Week 7 Sprint milestone Feb 4, 2021
@DennisDenuto
Copy link
Contributor

DennisDenuto commented Feb 10, 2021

The table below shows the performance difference from using imgpkg copy with some 'spike' changes to hopefully speed things up.

Registry # images # distinct images image size Time (old) Time (new)
dockerhub 20 1 small 3.69s user 3.46s system 37% cpu 18.974 total 2.47s user 2.34s system 28% cpu 17.066 total
dockerhub 20 1 1 Gb 45.69s user 42.19s system 10% cpu 13:57.48 total 13.08s user 9.04s system 4% cpu 7:48.27 total
dockerhub 20 20 small 5.07s user 4.54s system 19% cpu 49.273 total 3.35s user 2.70s system 25% cpu 23.254 total
local-registry 20 1 small 4.64s user 4.31s system 115% cpu 7.734 total 3.07s user 2.80s system 92% cpu 6.347 total
local-registry 20 1 1 Gb 36.14s user 14.63s system 14% cpu 5:51.51 total 8.33s user 4.58s system 19% cpu 1:05.71 total
local-registry 20 20 small -- --

Tasks

  • Profile and find bottlenecks in our code
  • Remove the double image ref check from resolved.go
  • In the generic call, make a HEAD request instead of a full GET [or] create new method that uses HEAD requests and identify which generic calls can be switched to the new method.
  • Can we "mount" layers so avoid re-uploading to the same registry?
  • Replace our write calls with a call to MultiWrite
  • Performance/Benchmark test
  • Test that when copying an image that already exists, it does not upload blobs again

@gcheadle-vmware gcheadle-vmware self-assigned this Feb 11, 2021
@cari-lynn cari-lynn assigned cari-lynn and unassigned StevenLocke Feb 18, 2021
@aaronshurley aaronshurley removed this from the Week 7 Sprint milestone Feb 22, 2021
@aaronshurley
Copy link
Contributor

This issue was resolved as a part of #95 and was released in v0.6.0.

@github-actions github-actions bot added the carvel triage This issue has not yet been reviewed for validity label Apr 5, 2021
@pivotaljohn pivotaljohn removed the carvel triage This issue has not yet been reviewed for validity label Apr 26, 2021
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

No branches or pull requests

6 participants