From ef5fdbcf861cc5d7d022884770cb0f293e3db85a Mon Sep 17 00:00:00 2001 From: Michal Fox Date: Tue, 29 Sep 2026 11:29:15 +0200 Subject: [PATCH] walk: fix --fast-list being very slow when filters exclude many files - fixes #9989 For every excluded object walkRDirTree called DirTree.Find to see whether the parent directory was already in the tree. Find scans the parent's entries linearly, so this went quadratic with many excluded files next to many kept files, or with many directories each holding an excluded file on a bucket based remote. With --exclude-if-present it also ran a FileExists for every excluded object rather than once per directory. This keeps a set of the directories already added to the tree or already checked, so each directory is checked once and Find isn't needed. With 100k kept and 100k excluded files this takes the tree build from 14.0s to 0.2s in one directory and from 39.5s to 0.54s with each pair in its own directory. --- fs/walk/walk.go | 20 ++++--- fs/walk/walk_test.go | 131 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 143 insertions(+), 8 deletions(-) diff --git a/fs/walk/walk.go b/fs/walk/walk.go index 4cbef90..8d32d1c 100644 --- a/fs/walk/walk.go +++ b/fs/walk/walk.go @@ -470,6 +470,10 @@ func walkRDirTree(ctx context.Context, f fs.Fs, startPath string, includeAll boo // Entries can come in arbitrary order. We use toPrune to keep // all directories to exclude later. toPrune := make(map[string]bool) + // knownDirs holds directories which already have an entry in + // their parent or which were checked and not included, so the + // parents of excluded objects don't need dirs.Find which is O(N) + knownDirs := make(map[string]struct{}) includeDirectory := fi.IncludeDirectory(ctx, f) var mu sync.Mutex err := listR(ctx, startPath, func(entries fs.DirEntries) error { @@ -498,16 +502,15 @@ func walkRDirTree(ctx context.Context, f fs.Fs, startPath string, includeAll boo dirPath = parentDir(dirPath) } } - inc, err := includeDirectory(dirPath) - if err != nil { - return err - } - if inc || includeAll { - // If the directory doesn't exist already, create it - _, obj := dirs.Find(dirPath) - if obj == nil { + if _, found := knownDirs[dirPath]; !found { + inc, err := includeDirectory(dirPath) + if err != nil { + return err + } + if inc || includeAll { dirs.AddDir(fs.NewDir(dirPath, time.Now())) } + knownDirs[dirPath] = struct{}{} } } // Check if we need to prune a directory later. @@ -533,6 +536,7 @@ func walkRDirTree(ctx context.Context, f fs.Fs, startPath string, includeAll boo } else { dirs.AddDir(x) } + knownDirs[x.Remote()] = struct{}{} } } default: diff --git a/fs/walk/walk_test.go b/fs/walk/walk_test.go index 1906d16..b2fe7f2 100644 --- a/fs/walk/walk_test.go +++ b/fs/walk/walk_test.go @@ -745,6 +745,137 @@ b/c/d/ fi.Opt.ExcludeFile = nil } +// countingFs counts the NewObject calls made by the exclude file check +type countingFs struct { + fs.Fs + newObjectCalls int +} + +func (f *countingFs) NewObject(ctx context.Context, remote string) (fs.Object, error) { + f.newObjectCalls++ + return f.Fs.NewObject(ctx, remote) +} + +func excludeTmpContext(t testing.TB, excludeFile ...string) context.Context { + fi, err := filter.NewFilter(nil) + require.NoError(t, err) + require.NoError(t, fi.Add(false, "*.tmp")) + fi.Opt.ExcludeFile = excludeFile + return filter.ReplaceConfig(context.Background(), fi) +} + +func TestWalkRDirTreeExcludedObjectParents(t *testing.T) { + ctx := excludeTmpContext(t) + entries := fs.DirEntries{ + mockdir.New("a"), + mockobject.Object("a/1.tmp"), + mockobject.Object("a/2.tmp"), + mockobject.Object("b/1.tmp"), + mockobject.Object("b/file"), + mockobject.Object("b/2.tmp"), + mockobject.Object("c/d/1.tmp"), + mockobject.Object("c/d/2.tmp"), + mockobject.Object("e/f/g/1.tmp"), + } + r, err := walkRDirTree(ctx, nil, "", false, -1, makeListRCallback(entries, nil)) + require.NoError(t, err) + assert.Equal(t, `/ + a/ + b/ + c/ + e/ +a/ +b/ + file +c/ + d/ +c/d/ +e/ + f/ +e/f/ + g/ +e/f/g/ +`, r.String()) + + r, err = walkRDirTree(ctx, nil, "", false, 2, makeListRCallback(entries, nil)) + require.NoError(t, err) + assert.Equal(t, `/ + a/ + b/ + c/ + e/ +a/ +b/ + file +c/ + d/ +c/d/ +e/ + f/ +e/f/ +`, r.String()) +} + +func TestWalkRDirTreeExcludeFileCheckedOncePerDir(t *testing.T) { + ctx := excludeTmpContext(t, ".ignore") + f, err := mockfs.NewFs(ctx, "mock", "", nil) + require.NoError(t, err) + cf := &countingFs{Fs: f} + var entries fs.DirEntries + for _, dir := range []string{"a", "b", "c"} { + for i := range 10 { + entries = append(entries, mockobject.Object(fmt.Sprintf("%s/%d.tmp", dir, i))) + } + } + r, err := walkRDirTree(ctx, cf, "", false, -1, makeListRCallback(entries, nil)) + require.NoError(t, err) + assert.Equal(t, `/ + a/ + b/ + c/ +a/ +b/ +c/ +`, r.String()) + assert.Equal(t, 3, cf.newObjectCalls) +} + +// makeExcludedEntries makes n kept and n excluded objects, all in the +// root or each pair in its own directory +func makeExcludedEntries(n int, ownDir bool) fs.DirEntries { + entries := make(fs.DirEntries, 0, 2*n) + for i := range n { + dir := "" + if ownDir { + dir = fmt.Sprintf("dir%d/", i) + } + entries = append(entries, + mockobject.Object(fmt.Sprintf("%sfile%d", dir, i)), + mockobject.Object(fmt.Sprintf("%sfile%d.tmp", dir, i)), + ) + } + return entries +} + +func BenchmarkWalkRDirTreeExcluded(b *testing.B) { + ctx := excludeTmpContext(b) + for _, bench := range []struct { + name string + ownDir bool + }{ + {"SameDir", false}, + {"OwnDir", true}, + } { + entries := makeExcludedEntries(10000, bench.ownDir) + b.Run(bench.name, func(b *testing.B) { + for b.Loop() { + _, err := walkRDirTree(ctx, nil, "", false, -1, makeListRCallback(entries, nil)) + require.NoError(b, err) + } + }) + } +} + func TestListType(t *testing.T) { assert.Equal(t, true, ListObjects.Objects()) assert.Equal(t, false, ListObjects.Dirs())