From 1c586a820b2b4124b7f5f343c34f17ab5c10b891 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Thu, 27 Jun 2024 22:01:02 +0200 Subject: [PATCH 1/3] Eliminate a numLayers variable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The compiler is smart enough that referencing a local variable is not any faster than len(srcInfos). If anything, keeping the extra variable might add to register pressure because the compiler might not understand that the srcInfos change due to LayerInfosForCopy does not change the number of layers. Should not change behavior. Signed-off-by: Miloslav Trmač --- copy/single.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/copy/single.go b/copy/single.go index 9d544f5618..ad0d7d1b07 100644 --- a/copy/single.go +++ b/copy/single.go @@ -409,7 +409,6 @@ func (ic *imageCopier) compareImageDestinationManifestEqual(ctx context.Context, // copyLayers copies layers from ic.src/ic.c.rawSource to dest, using and updating ic.manifestUpdates if necessary and ic.cannotModifyManifestReason == "". func (ic *imageCopier) copyLayers(ctx context.Context) ([]compressiontypes.Algorithm, error) { srcInfos := ic.src.LayerInfos() - numLayers := len(srcInfos) updatedSrcInfos, err := ic.src.LayerInfosForCopy(ctx) if err != nil { return nil, err @@ -440,7 +439,7 @@ func (ic *imageCopier) copyLayers(ctx context.Context) ([]compressiontypes.Algor // copyGroup is used to determine if all layers are copied copyGroup := sync.WaitGroup{} - data := make([]copyLayerData, numLayers) + data := make([]copyLayerData, len(srcInfos)) copyLayerHelper := func(index int, srcLayer types.BlobInfo, toEncrypt bool, pool *mpb.Progress, srcRef reference.Named) { defer ic.c.concurrentBlobCopiesSemaphore.Release(1) defer copyGroup.Done() @@ -509,8 +508,8 @@ func (ic *imageCopier) copyLayers(ctx context.Context) ([]compressiontypes.Algor } compressionAlgos := set.New[string]() - destInfos := make([]types.BlobInfo, numLayers) - diffIDs := make([]digest.Digest, numLayers) + destInfos := make([]types.BlobInfo, len(srcInfos)) + diffIDs := make([]digest.Digest, len(srcInfos)) for i, cld := range data { if cld.err != nil { return nil, cld.err From 871cdd3369659072074c8b37f76eb36d6f58ab6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Thu, 27 Jun 2024 22:03:18 +0200 Subject: [PATCH 2/3] Eliminate a single-use variable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Should not change behavior. Signed-off-by: Miloslav Trmač --- copy/single.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/copy/single.go b/copy/single.go index ad0d7d1b07..9b5340ae35 100644 --- a/copy/single.go +++ b/copy/single.go @@ -462,9 +462,7 @@ func (ic *imageCopier) copyLayers(ctx context.Context) ([]compressiontypes.Algor // Decide which layers to encrypt layersToEncrypt := set.New[int]() - var encryptAll bool if ic.c.options.OciEncryptLayers != nil { - encryptAll = len(*ic.c.options.OciEncryptLayers) == 0 totalLayers := len(srcInfos) for _, l := range *ic.c.options.OciEncryptLayers { switch { @@ -477,7 +475,7 @@ func (ic *imageCopier) copyLayers(ctx context.Context) ([]compressiontypes.Algor } } - if encryptAll { + if len(*ic.c.options.OciEncryptLayers) == 0 { // “encrypt all layers” for i := 0; i < len(srcInfos); i++ { layersToEncrypt.Add(i) } From 22b11d1b252e8f127e8a3c18772715734b80b2a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Thu, 27 Jun 2024 22:04:33 +0200 Subject: [PATCH 3/3] Narrow down the scope of a variable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Otherwise "err" refers to a variable outside of this closure, not even to the one used by the immediate caller. Should not change behavior. Signed-off-by: Miloslav Trmač --- copy/single.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/copy/single.go b/copy/single.go index 9b5340ae35..17cc8c833e 100644 --- a/copy/single.go +++ b/copy/single.go @@ -490,8 +490,7 @@ func (ic *imageCopier) copyLayers(ctx context.Context) ([]compressiontypes.Algor defer copyGroup.Wait() for i, srcLayer := range srcInfos { - err = ic.c.concurrentBlobCopiesSemaphore.Acquire(ctx, 1) - if err != nil { + if err := ic.c.concurrentBlobCopiesSemaphore.Acquire(ctx, 1); err != nil { // This can only fail with ctx.Err(), so no need to blame acquiring the semaphore. return fmt.Errorf("copying layer: %w", err) }