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

@@ -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:

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())