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