From 95f45541e47253610ed83b064dab2124a11027e8 Mon Sep 17 00:00:00 2001 From: Jin Dong Date: Fri, 3 Jan 2025 03:29:08 +0000 Subject: [PATCH 1/2] Avoid duplicated chain ID calculation in unpack This PR optimizes the chain ID calculation in unpack so we only (pre-)calculate the chain ID for layer_i exactly once, by calling `identity.ChainIDs(diffIDs)`. Currently in `unpack` for every layer_i, we calculate the chain id of layer_i-1 and layer_i *repeatedly*. Because each `identity.ChainID(diffIDs)` call involves: 1. Copy `diffIDs` to a new slice 2. Calculate chain ID for every index (by calling `ChainIDs(diffIDs)`) 3. Return the final one as result of `identity.ChainID(diffIDs)`. This means, given an image with N layers, for every layer_i: 1. we create 2 new slices and copy the diffIDs sofar; 2. we're recalculating all chain IDs from layer_0 to layer_i-1 twice; 3. in total, the chain ID for layer_i is calculated 2 * (N - layer_i) times; Signed-off-by: Jin Dong --- core/unpack/unpacker.go | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/core/unpack/unpacker.go b/core/unpack/unpacker.go index 35dc4dc47..d1a1e47c0 100644 --- a/core/unpack/unpacker.go +++ b/core/unpack/unpacker.go @@ -273,8 +273,6 @@ func (u *Unpacker) unpack( a = unpack.Applier cs = u.content - chain []digest.Digest - fetchOffset int fetchC []chan struct{} fetchErr chan error @@ -285,10 +283,17 @@ func (u *Unpacker) unpack( ctx, cancel := context.WithCancel(ctx) defer cancel() + // pre-calculate chain ids for each layer + chainIDs := make([]digest.Digest, len(diffIDs)) + copy(chainIDs, diffIDs) + chainIDs = identity.ChainIDs(chainIDs) + doUnpackFn := func(i int, desc ocispec.Descriptor) error { - parent := identity.ChainID(chain) - chain = append(chain, diffIDs[i]) - chainID := identity.ChainID(chain).String() + var parent string + if i > 0 { + parent = chainIDs[i-1].String() + } + chainID := chainIDs[i].String() unlock, err := u.lockSnChainID(ctx, chainID, unpack.SnapshotterKey) if err != nil { @@ -312,7 +317,7 @@ func (u *Unpacker) unpack( for try := 1; try <= 3; try++ { // Prepare snapshot with from parent, label as root key = fmt.Sprintf(snapshots.UnpackKeyFormat, uniquePart(), chainID) - mounts, err = sn.Prepare(ctx, key, parent.String(), opts...) + mounts, err = sn.Prepare(ctx, key, parent, opts...) if err != nil { if errdefs.IsAlreadyExists(err) { if _, err := sn.Stat(ctx, chainID); err != nil { @@ -424,7 +429,10 @@ func (u *Unpacker) unpack( }).Debug("layer unpacked") } - chainID := identity.ChainID(chain).String() + var chainID string + if len(chainIDs) > 0 { + chainID = chainIDs[len(chainIDs)-1].String() + } cinfo := content.Info{ Digest: config.Digest, Labels: map[string]string{ From d156d3df9620844491a4e6c94945693d5c7df043 Mon Sep 17 00:00:00 2001 From: Jin Dong Date: Fri, 3 Jan 2025 15:06:00 +0000 Subject: [PATCH 2/2] Benchamrk chainID calculation in unpack Signed-off-by: Jin Dong --- core/unpack/unpacker_test.go | 93 ++++++++++++++++++++++++++++++++++++ 1 file changed, 93 insertions(+) create mode 100644 core/unpack/unpacker_test.go diff --git a/core/unpack/unpacker_test.go b/core/unpack/unpacker_test.go new file mode 100644 index 000000000..32581dbd1 --- /dev/null +++ b/core/unpack/unpacker_test.go @@ -0,0 +1,93 @@ +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package unpack + +import ( + "crypto/rand" + "fmt" + "testing" + + "github.com/opencontainers/go-digest" + "github.com/opencontainers/image-spec/identity" +) + +func generateRandomDiffIDs(t testing.TB, num int) []digest.Digest { + const size = 10 + diffIDs := make([]digest.Digest, 0, num) + for i := 0; i < num; i++ { + b := make([]byte, size) + _, err := rand.Read(b) + if err != nil { + t.Fatalf("failed to generate random bytes: %v", err) + } + diffIDs = append(diffIDs, digest.FromBytes(b)) + } + return diffIDs +} + +func BenchmarkUnpackWithChainID(b *testing.B) { + // This simulates the old way of repeatedly calculating per-layer chainID + // as we unpack every layers, by calling `identity.ChainID`. + unpackWithChainID := func(diffIDs []digest.Digest) { + var chain []digest.Digest + for i := 0; i < len(diffIDs); i++ { + _ = identity.ChainID(chain) // parent layer chainID + chain = append(chain, diffIDs[i]) + _ = identity.ChainID(chain).String() // current layer chainID + } + _ = identity.ChainID(chain).String() + } + + numLayers := []int{5, 10, 25, 50} + for _, sz := range numLayers { + b.Run(fmt.Sprintf("num of layers: %d", sz), func(b *testing.B) { + diffIDs := generateRandomDiffIDs(b, sz) + for i := 0; i < b.N; i++ { + unpackWithChainID(diffIDs) + } + }) + } +} + +func BenchmarkUnpackWithChainIDs(b *testing.B) { + // This simulates the new way of pre-calculating all chainIDs for every layer + // by calling `identity.ChainIDs` once. + unpackWithChainIDs := func(diffIDs []digest.Digest) { + chainIDs := make([]digest.Digest, len(diffIDs)) + copy(chainIDs, diffIDs) + chainIDs = identity.ChainIDs(chainIDs) + for i := 0; i < len(diffIDs); i++ { + if i > 0 { + _ = chainIDs[i-1].String() // parent layer chainID + } + _ = chainIDs[i].String() // current layer chainID + } + if len(chainIDs) > 0 { + _ = chainIDs[len(chainIDs)-1].String() + } + } + + numLayers := []int{5, 10, 25, 50} + for _, sz := range numLayers { + b.Run(fmt.Sprintf("num of layers: %d", sz), func(b *testing.B) { + diffIDs := generateRandomDiffIDs(b, sz) + for i := 0; i < b.N; i++ { + unpackWithChainIDs(diffIDs) + } + }) + } +}