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

fix: Avoid a copy of sync.Mutex in Repository #603

Merged
merged 2 commits into from
Sep 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 20 additions & 12 deletions registry/remote/repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ type Repository struct {
// - https://www.rfc-editor.org/rfc/rfc7234#section-5.5
HandleWarning func(warning Warning)

// NOTE: Must keep fields in sync with newRepositoryWithOptions function.
// NOTE: Must keep fields in sync with clone().

// referrersState represents that if the repository supports Referrers API.
// default: referrersStateUnknown
Expand Down Expand Up @@ -186,16 +186,24 @@ func newRepositoryWithOptions(ref registry.Reference, opts *RepositoryOptions) (
if err := ref.ValidateRepository(); err != nil {
return nil, err
}
repo := (*Repository)(opts).clone()
repo.Reference = ref
return repo, nil
}

// clone makes a copy of the Repository being careful not to copy non-copyable fields (sync.Mutex and syncutil.Pool types)
func (r *Repository) clone() *Repository {
ktarplee marked this conversation as resolved.
Show resolved Hide resolved
return &Repository{
Client: opts.Client,
Reference: ref,
PlainHTTP: opts.PlainHTTP,
SkipReferrersGC: opts.SkipReferrersGC,
ManifestMediaTypes: slices.Clone(opts.ManifestMediaTypes),
TagListPageSize: opts.TagListPageSize,
ReferrerListPageSize: opts.ReferrerListPageSize,
MaxMetadataBytes: opts.MaxMetadataBytes,
}, nil
Client: r.Client,
Reference: r.Reference,
PlainHTTP: r.PlainHTTP,
ManifestMediaTypes: slices.Clone(r.ManifestMediaTypes),
TagListPageSize: r.TagListPageSize,
ReferrerListPageSize: r.ReferrerListPageSize,
MaxMetadataBytes: r.MaxMetadataBytes,
SkipReferrersGC: r.SkipReferrersGC,
HandleWarning: r.HandleWarning,
}
ktarplee marked this conversation as resolved.
Show resolved Hide resolved
}

// SetReferrersCapability indicates the Referrers API capability of the remote
Expand Down Expand Up @@ -803,10 +811,10 @@ func (s *blobStore) Mount(ctx context.Context, desc ocispec.Descriptor, fromRepo
// sibling returns a blob store for another repository in the same
// registry.
func (s *blobStore) sibling(otherRepoName string) *blobStore {
otherRepo := *s.repo
otherRepo := s.repo.clone()
otherRepo.Reference.Repository = otherRepoName
return &blobStore{
repo: &otherRepo,
repo: otherRepo,
}
}

Expand Down
21 changes: 21 additions & 0 deletions registry/remote/repository_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7447,3 +7447,24 @@ func TestRepository_do(t *testing.T) {
t.Errorf("Repository.do() = %v, want %v", gotWarnings, wantWarnings)
}
}

func TestRepository_clone(t *testing.T) {
repo, err := NewRepository("localhost:1234/repo/image")
if err != nil {
t.Fatalf("invalid repository: %v", err)
}

crepo := repo.clone()

if repo.Reference != crepo.Reference {
t.Fatal("references should be the same")
}

if !reflect.DeepEqual(&repo.referrersPingLock, &crepo.referrersPingLock) {
t.Fatal("referrersPingLock should be different")
}

if !reflect.DeepEqual(&repo.referrersMergePool, &crepo.referrersMergePool) {
t.Fatal("referrersMergePool should be different")
}
ktarplee marked this conversation as resolved.
Show resolved Hide resolved
}
Loading