From 2f900210445d82bc6cc21611bdbfadb6a7d13277 Mon Sep 17 00:00:00 2001
From: ljluestc <63439129+ljluestc@users.noreply.github.com>
Date: Mon, 7 Sep 2026 22:17:26 -0700
Subject: [PATCH] cache: load parent chain only once in remote cache
 LoadWithParents

On a remote cache match, LoadWithParents loads the matched record and
every parent record so that cache metadata is saved for all parent
layers. Each of these records was loaded with its own worker.FromRemote
call, and because every parent's remote contains the full descriptor
chain up to that record, each call verified and loaded all the layers
below it again, resulting in O(n^2) layer verifications for a chain of
n layers. This is especially costly when layer verification hits the
local disk (e.g. via the moby download manager).

Since every parent record's remote is a strict prefix of the matched
result's remote, collect the records during the walk, call FromRemote
once for the full chain, and take each record's ref from the returned
ref's LayerChain instead. Each layer is now verified and loaded only
once. If a worker returns a ref whose layer chain does not map 1:1 to
the remote descriptors, the affected records fall back to the previous
per-record load.

This also fixes a ref leak where multiple results on the same item
overwrote earlier entries in the result map without releasing them.

Fixes #2184
---
 cache/remotecache/v1/cachestorage.go | 94 +++++++++++++++++++++-------
 1 file changed, 70 insertions(+), 24 deletions(-)

diff --git a/cache/remotecache/v1/cachestorage.go b/cache/remotecache/v1/cachestorage.go
index cd36dbf13412..48a6c7620c45 100644
--- a/cache/remotecache/v1/cachestorage.go
+++ b/cache/remotecache/v1/cachestorage.go
@@ -249,6 +249,11 @@ func (cs *cacheResultStorage) Save(res solver.Result, createdAt time.Time) (solv
 
 func (cs *cacheResultStorage) LoadWithParents(ctx context.Context, res solver.CacheResult) (map[string]solver.Result, error) {
 	m := map[string]solver.Result{}
+	releaseAll := func() {
+		for _, v := range m {
+			v.Release(context.TODO())
+		}
+	}
 
 	visited := make(map[*item]struct{})
 
@@ -259,36 +264,77 @@ func (cs *cacheResultStorage) LoadWithParents(ctx context.Context, res solver.Ca
 
 	for id := range ids {
 		v, ok := cs.byID[id]
-		if ok {
-			if _, ok := visited[v.item]; ok {
+		if !ok {
+			continue
+		}
+		if _, ok := visited[v.item]; ok {
+			continue
+		}
+		for _, result := range v.results {
+			resultID := remoteID(result.Result)
+			if resultID != res.ID {
 				continue
 			}
-			for _, result := range v.results {
-				resultID := remoteID(result.Result)
-				if resultID == res.ID {
-					if err := v.walkAllResults(func(i *item) error {
-						for _, subRes := range i.results {
-							id, ok := cs.byItem[i]
-							if !ok {
-								return nil
-							}
-							if isSubRemote(*subRes.Result, *result.Result) {
-								ref, err := cs.w.FromRemote(ctx, subRes.Result)
-								if err != nil {
-									return err
-								}
-								m[id] = worker.NewWorkerRefResult(ref, cs.w)
-							}
-						}
-						return nil
-					}, visited); err != nil {
-						for _, v := range m {
-							v.Release(context.TODO())
-						}
+			// Every record in the parent chain has a remote that is a prefix
+			// of the matched result's remote, so instead of loading each of
+			// them separately (which would verify the same parent layers over
+			// and over again), collect the records with the depth of their
+			// remote and load the full chain only once.
+			type entry struct {
+				id     string
+				remote *solver.Remote
+			}
+			var entries []entry
+			if err := v.walkAllResults(func(i *item) error {
+				id, ok := cs.byItem[i]
+				if !ok {
+					return nil
+				}
+				for _, subRes := range i.results {
+					if isSubRemote(*subRes.Result, *result.Result) {
+						entries = append(entries, entry{id: id, remote: subRes.Result})
+					}
+				}
+				return nil
+			}, visited); err != nil {
+				releaseAll()
+				return nil, err
+			}
+			if len(entries) == 0 {
+				continue
+			}
+
+			ref, err := cs.w.FromRemote(ctx, result.Result)
+			if err != nil {
+				releaseAll()
+				return nil, err
+			}
+			chain := ref.LayerChain()
+			ref.Release(context.TODO())
+
+			for _, e := range entries {
+				depth := len(e.remote.Descriptors)
+				var res solver.Result
+				if depth > 0 && depth <= len(chain) && len(chain) == len(result.Result.Descriptors) {
+					res = worker.NewWorkerRefResult(chain[depth-1].Clone(), cs.w)
+				} else {
+					// the worker returned a ref with a layer chain that does
+					// not map 1:1 to the remote descriptors; load this record
+					// directly
+					ref, err := cs.w.FromRemote(ctx, e.remote)
+					if err != nil {
+						chain.Release(context.TODO())
+						releaseAll()
 						return nil, err
 					}
+					res = worker.NewWorkerRefResult(ref, cs.w)
+				}
+				if prev, ok := m[e.id]; ok {
+					prev.Release(context.TODO())
 				}
+				m[e.id] = res
 			}
+			chain.Release(context.TODO())
 		}
 	}