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

feat: Use remote.Reuse to make everything faster #2929

Merged
merged 1 commit into from
Apr 25, 2023

Conversation

jonjohnsonjr
Copy link
Contributor

Summary

Continuing #2901

Previously, every call to a remote.Function was performing a Ping and Token exchange, which meant that we were doing 3 round trips instead of 1 (for most calls).

The remote.Reuse option effectively tells the remote package to call methods on a Pusher/Puller instance rather than re-doing the Ping and Token requests.

The alternative to this PR would be to plumb around a puller and pusher instance and refactor all of cosign to call methods on those, but this is much simpler.

For an empty destination repository, cosign copy improved from 24s to 19s. When re-running it against a repository we already pushed to, we see improvement from ~12s to ~3.5s.

Command Mean [s] Min [s] Max [s]
cosign copy gcr.io/projectsigstore/cosign:v1.13.0 (before) 11.332 ± 0.212 11.175 11.876
cosign copy gcr.io/projectsigstore/cosign:v1.13.0 (after) 3.405 ± 0.285 2.819 3.751

Release Note

cosign registry operations were optimized to be much faster.

Documentation

Previously, every call to a remote.Function was performing a Ping and
Token exchange, which meant that we were doing 3 round trips instead of
1 (for most calls).

The remote.Reuse option effectively tells the remote package to call
methods on a Pusher/Puller instance rather than re-doing the Ping and
Token requests.

The alternative to this PR would be to plumb around a puller and pusher
instance and refactor all of cosign to call methods on those, but this
is much simpler.

Signed-off-by: Jon Johnson <jon.johnson@chainguard.dev>
@codecov
Copy link

codecov bot commented Apr 25, 2023

Codecov Report

Merging #2929 (65977a2) into main (4012c47) will decrease coverage by 0.01%.
The diff coverage is 15.38%.

@@            Coverage Diff             @@
##             main    #2929      +/-   ##
==========================================
- Coverage   30.33%   30.32%   -0.01%     
==========================================
  Files         151      151              
  Lines        9439     9451      +12     
==========================================
+ Hits         2863     2866       +3     
- Misses       6134     6140       +6     
- Partials      442      445       +3     
Impacted Files Coverage Δ
cmd/cosign/cli/options/registry.go 0.00% <0.00%> (ø)
pkg/oci/remote/referrers.go 66.66% <40.00%> (-33.34%) ⬇️

... and 1 file with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

jonjohnsonjr added a commit to jonjohnsonjr/apko that referenced this pull request Apr 25, 2023
This is a first pass at speeding up the registry portion of apko
publish.

Currently draft until this lands in cosign:
sigstore/cosign#2929
jonjohnsonjr added a commit to jonjohnsonjr/apko that referenced this pull request Apr 25, 2023
This is a first pass at speeding up the registry portion of apko
publish.

Currently draft until this lands in cosign:
sigstore/cosign#2929

Signed-off-by: Jon Johnson <jon.johnson@chainguard.dev>
@znewman01 znewman01 merged commit 17cc138 into sigstore:main Apr 25, 2023
@github-actions github-actions bot added this to the v1.14.0 milestone Apr 25, 2023
@janisz janisz mentioned this pull request Jun 20, 2023
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 this pull request may close these issues.

4 participants