From 806fddd7f7bbe560c7aeb57a9b0a7f451eccdd33 Mon Sep 17 00:00:00 2001 From: Erik Sipsma Date: Thu, 16 Jul 2020 16:53:05 -0700 Subject: [PATCH] remotecache: Only visit each item once when walking results. Before this change, walkAllResults did not skip an item if it had already been visited, so walking the graph of results actually followed every single possible path in the graph instead of just visiting each item once. On small graphs this wasn't noticeable, but on sufficiently large remote cache imports this rapidly scaled out of control. For example, I first encountered this when importing a max-cache from a build of a full linux rootfs from source (which takes about 30 minutes to build w/out cache on an 18-core machine) and after 30 minutes the cache import was still running with all CPUs pegged at 100% To fix this, walkAllResults now keeps a map of already visited items (keyed by their pointer) and skips visiting an item if it's already been visited, making it usable on large remote cache imports. Signed-off-by: Erik Sipsma --- cache/remotecache/v1/cachestorage.go | 3 ++- cache/remotecache/v1/chains.go | 8 ++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/cache/remotecache/v1/cachestorage.go b/cache/remotecache/v1/cachestorage.go index 605b6d63..728b85a2 100644 --- a/cache/remotecache/v1/cachestorage.go +++ b/cache/remotecache/v1/cachestorage.go @@ -220,6 +220,7 @@ func (cs *cacheResultStorage) LoadWithParents(ctx context.Context, res solver.Ca m := map[string]solver.Result{} + visited := make(map[*item]struct{}) if err := v.walkAllResults(func(i *item) error { if i.result == nil { return nil @@ -236,7 +237,7 @@ func (cs *cacheResultStorage) LoadWithParents(ctx context.Context, res solver.Ca m[id] = worker.NewWorkerRefResult(ref, cs.w) } return nil - }); err != nil { + }, visited); err != nil { for _, v := range m { v.Release(context.TODO()) } diff --git a/cache/remotecache/v1/chains.go b/cache/remotecache/v1/chains.go index 6dd15707..a9b11c3e 100644 --- a/cache/remotecache/v1/chains.go +++ b/cache/remotecache/v1/chains.go @@ -128,13 +128,17 @@ func (c *item) LinkFrom(rec solver.CacheExporterRecord, index int, selector stri c.links[index][link{src: src, selector: selector}] = struct{}{} } -func (c *item) walkAllResults(fn func(i *item) error) error { +func (c *item) walkAllResults(fn func(i *item) error, visited map[*item]struct{}) error { + if _, ok := visited[c]; ok { + return nil + } + visited[c] = struct{}{} if err := fn(c); err != nil { return err } for _, links := range c.links { for l := range links { - if err := l.src.walkAllResults(fn); err != nil { + if err := l.src.walkAllResults(fn, visited); err != nil { return err } }