From 59fd8656ac3c4f34de9dbb93e9fae90f7f1ec9f2 Mon Sep 17 00:00:00 2001 From: Milos Gajdos Date: Sun, 3 Sep 2023 22:41:51 +0100 Subject: [PATCH 1/4] Enable prealloc linter This will give us nice little performance gains in some code paths. Signed-off-by: Milos Gajdos --- .golangci.yml | 1 + errors.go | 2 +- registry/handlers/app.go | 1 + registry/proxy/proxytagservice_test.go | 2 +- registry/storage/driver/azure/azure.go | 2 +- registry/storage/driver/inmemory/mfs.go | 10 ++++++++-- registry/storage/driver/s3-aws/s3.go | 2 +- registry/storage/driver/s3-aws/s3_test.go | 1 + registry/storage/tagstore.go | 9 ++++----- testutil/manifests.go | 2 +- 10 files changed, 20 insertions(+), 12 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 35a903a8..5931556d 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -10,6 +10,7 @@ linters: - unused - misspell - bodyclose + - prealloc disable: - errcheck diff --git a/errors.go b/errors.go index 8e0b788d..4fbfc852 100644 --- a/errors.go +++ b/errors.go @@ -90,7 +90,7 @@ func (ErrManifestUnverified) Error() string { type ErrManifestVerification []error func (errs ErrManifestVerification) Error() string { - var parts []string + parts := make([]string, 0, len(errs)) for _, err := range errs { parts = append(parts, err.Error()) } diff --git a/registry/handlers/app.go b/registry/handlers/app.go index 7ea9f435..bd3a60d7 100644 --- a/registry/handlers/app.go +++ b/registry/handlers/app.go @@ -441,6 +441,7 @@ func (app *App) register(routeName string, dispatch dispatchFunc) { // configureEvents prepares the event sink for action. func (app *App) configureEvents(configuration *configuration.Configuration) { // Configure all of the endpoint sinks. + // nolint:prealloc var sinks []events.Sink for _, endpoint := range configuration.Notifications.Endpoints { if endpoint.Disabled { diff --git a/registry/proxy/proxytagservice_test.go b/registry/proxy/proxytagservice_test.go index 61e15852..e31a44eb 100644 --- a/registry/proxy/proxytagservice_test.go +++ b/registry/proxy/proxytagservice_test.go @@ -49,7 +49,7 @@ func (m *mockTagStore) All(ctx context.Context) ([]string, error) { m.Lock() defer m.Unlock() - var tags []string + tags := make([]string, 0, len(m.mapping)) for tag := range m.mapping { tags = append(tags, tag) } diff --git a/registry/storage/driver/azure/azure.go b/registry/storage/driver/azure/azure.go index c2cf9277..3e3c05ce 100644 --- a/registry/storage/driver/azure/azure.go +++ b/registry/storage/driver/azure/azure.go @@ -416,7 +416,7 @@ func directDescendants(blobs []string, prefix string) []string { } } - var keys []string + keys := make([]string, 0, len(out)) for k := range out { keys = append(keys, k) } diff --git a/registry/storage/driver/inmemory/mfs.go b/registry/storage/driver/inmemory/mfs.go index da0c2abb..7eaa3c57 100644 --- a/registry/storage/driver/inmemory/mfs.go +++ b/registry/storage/driver/inmemory/mfs.go @@ -1,6 +1,7 @@ package inmemory import ( + "errors" "fmt" "io" "path" @@ -97,8 +98,13 @@ func (d *dir) list(p string) ([]string, error) { return nil, errIsNotDir } - var children []string - for _, child := range n.(*dir).children { + dir, ok := n.(*dir) + if !ok { + return nil, errors.New("not a directory") + } + + children := make([]string, 0, len(dir.children)) + for _, child := range dir.children { children = append(children, child.path()) } diff --git a/registry/storage/driver/s3-aws/s3.go b/registry/storage/driver/s3-aws/s3.go index 46ee8a18..ff0f46b3 100644 --- a/registry/storage/driver/s3-aws/s3.go +++ b/registry/storage/driver/s3-aws/s3.go @@ -1456,7 +1456,7 @@ func (w *writer) Commit() error { } w.committed = true - var completedUploadedParts completedParts + completedUploadedParts := make(completedParts, 0, len(w.parts)) for _, part := range w.parts { completedUploadedParts = append(completedUploadedParts, &s3.CompletedPart{ ETag: part.ETag, diff --git a/registry/storage/driver/s3-aws/s3_test.go b/registry/storage/driver/s3-aws/s3_test.go index 9f39975b..8d201804 100644 --- a/registry/storage/driver/s3-aws/s3_test.go +++ b/registry/storage/driver/s3-aws/s3_test.go @@ -500,6 +500,7 @@ func TestWalk(t *testing.T) { } // create file structure matching fileset above + // nolint:prealloc var created []string for _, p := range fileset { err := drvr.PutContent(context.Background(), p, []byte("content "+p)) diff --git a/registry/storage/tagstore.go b/registry/storage/tagstore.go index bf6541f9..268c21cc 100644 --- a/registry/storage/tagstore.go +++ b/registry/storage/tagstore.go @@ -24,25 +24,24 @@ type tagStore struct { // All returns all tags func (ts *tagStore) All(ctx context.Context) ([]string, error) { - var tags []string - pathSpec, err := pathFor(manifestTagPathSpec{ name: ts.repository.Named().Name(), }) if err != nil { - return tags, err + return nil, err } entries, err := ts.blobStore.driver.List(ctx, pathSpec) if err != nil { switch err := err.(type) { case storagedriver.PathNotFoundError: - return tags, distribution.ErrRepositoryUnknown{Name: ts.repository.Named().Name()} + return nil, distribution.ErrRepositoryUnknown{Name: ts.repository.Named().Name()} default: - return tags, err + return nil, err } } + tags := make([]string, 0, len(entries)) for _, entry := range entries { _, filename := path.Split(entry) tags = append(tags, filename) diff --git a/testutil/manifests.go b/testutil/manifests.go index 9501aace..96af5703 100644 --- a/testutil/manifests.go +++ b/testutil/manifests.go @@ -14,7 +14,7 @@ import ( func MakeManifestList(blobstatter distribution.BlobStatter, manifestDigests []digest.Digest) (*manifestlist.DeserializedManifestList, error) { ctx := context.Background() - var manifestDescriptors []manifestlist.ManifestDescriptor + manifestDescriptors := make([]manifestlist.ManifestDescriptor, 0, len(manifestDigests)) for _, manifestDigest := range manifestDigests { descriptor, err := blobstatter.Stat(ctx, manifestDigest) if err != nil { From a9d31ec7b911a4247cba6b9acc645b11171bbb07 Mon Sep 17 00:00:00 2001 From: Milos Gajdos Date: Sun, 3 Sep 2023 23:23:25 +0100 Subject: [PATCH 2/4] Avoid unnecessary type assertion in mfs driver We already make sure the node in *dir Signed-off-by: Milos Gajdos --- registry/storage/driver/inmemory/mfs.go | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/registry/storage/driver/inmemory/mfs.go b/registry/storage/driver/inmemory/mfs.go index 7eaa3c57..f3837c5f 100644 --- a/registry/storage/driver/inmemory/mfs.go +++ b/registry/storage/driver/inmemory/mfs.go @@ -1,7 +1,6 @@ package inmemory import ( - "errors" "fmt" "io" "path" @@ -98,13 +97,12 @@ func (d *dir) list(p string) ([]string, error) { return nil, errIsNotDir } - dir, ok := n.(*dir) - if !ok { - return nil, errors.New("not a directory") - } + // NOTE(milosgajdos): this is safe to do because + // n can only be *dir due to the compile time check + dirChildren := n.(*dir).children - children := make([]string, 0, len(dir.children)) - for _, child := range dir.children { + children := make([]string, 0, len(dirChildren)) + for _, child := range dirChildren { children = append(children, child.path()) } From 108980064305fc67affd9cb45886e828aac9563a Mon Sep 17 00:00:00 2001 From: Milos Gajdos Date: Sun, 3 Sep 2023 23:26:32 +0100 Subject: [PATCH 3/4] Preallocate created slice in S3 tests In case drvr.PutContent fails and returns error we'd have some extra memory allocated, though in this case (test with known size of the slice being iterated), that's fine. Signed-off-by: Milos Gajdos --- registry/storage/driver/s3-aws/s3_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/registry/storage/driver/s3-aws/s3_test.go b/registry/storage/driver/s3-aws/s3_test.go index 8d201804..4987c52d 100644 --- a/registry/storage/driver/s3-aws/s3_test.go +++ b/registry/storage/driver/s3-aws/s3_test.go @@ -500,8 +500,7 @@ func TestWalk(t *testing.T) { } // create file structure matching fileset above - // nolint:prealloc - var created []string + created := make([]string, 0, len(fileset)) for _, p := range fileset { err := drvr.PutContent(context.Background(), p, []byte("content "+p)) if err != nil { From 823cd4a370e2aa7fe9d4bccc0cae9940bb88fa84 Mon Sep 17 00:00:00 2001 From: Milos Gajdos Date: Sun, 3 Sep 2023 23:33:20 +0100 Subject: [PATCH 4/4] Add a comment why prealloc linter is disabled when configuring endpoints Signed-off-by: Milos Gajdos --- registry/handlers/app.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/registry/handlers/app.go b/registry/handlers/app.go index bd3a60d7..7e0b418c 100644 --- a/registry/handlers/app.go +++ b/registry/handlers/app.go @@ -441,6 +441,10 @@ func (app *App) register(routeName string, dispatch dispatchFunc) { // configureEvents prepares the event sink for action. func (app *App) configureEvents(configuration *configuration.Configuration) { // Configure all of the endpoint sinks. + // NOTE(milosgajdos): we are disabling the linter here as + // if an endpoint is disabled we continue with the evaluation + // of the next one so we do not know the exact size the slice + // should have at the time the iteration starts // nolint:prealloc var sinks []events.Sink for _, endpoint := range configuration.Notifications.Endpoints {