vfs: reduce directory cache cleared by poll-interval

Reduce the number of nodes purged from the dir-cache when ForgetPath is
called. This is done by only forgetting the cache of the received path
and invalidating the parent folder cache by resetting *Dir.read.

The parent will read the listing on the next access and reuse the
dir-cache of entries in *Dir.items.

vfs/dir.go

@@ -101,42 +101,70 @@ func (d *Dir) ForgetAll() {
 
 // ForgetPath clears the cache for itself and all subdirectories if
 // they match the given path. The path is specified relative from the
-// directory it is called from.
+// directory it is called from. The cache of the parent directory is
+// marked as stale, but not cleared otherwise.
 // It is not possible to traverse the directory tree upwards, i.e.
 // you cannot clear the cache for the Dir's ancestors or siblings.
 func (d *Dir) ForgetPath(relativePath string, entryType fs.EntryType) {
-	// if we are requested to forget a file, we use its parent
-	absPath := path.Join(d.path, relativePath)
-	if entryType != fs.EntryDirectory {
-		absPath = path.Dir(absPath)
-	}
-	if absPath == "." || absPath == "/" {
-		absPath = ""
+	if absPath := path.Join(d.path, relativePath); absPath != "" {
+		parent := path.Dir(absPath)
+		if parent == "." || parent == "/" {
+			parent = ""
+		}
+		parentNode := d.vfs.root.cachedNode(parent)
+		if dir, ok := parentNode.(*Dir); ok {
+			dir.mu.Lock()
+			if !dir.read.IsZero() {
+				fs.Debugf(dir.path, "invalidating directory cache")
+				dir.read = time.Time{}
+			}
+			dir.mu.Unlock()
+		}
 	}
 
-	d.walk(absPath, func(dir *Dir) {
-		fs.Debugf(dir.path, "forgetting directory cache")
-		dir.read = time.Time{}
-		dir.items = make(map[string]Node)
-	})
+	if entryType == fs.EntryDirectory {
+		if dir := d.cachedDir(relativePath); dir != nil {
+			dir.walk(func(dir *Dir) {
+				fs.Debugf(dir.path, "forgetting directory cache")
+				dir.read = time.Time{}
+				dir.items = make(map[string]Node)
+			})
+		}
+	}
 }
 
-// walk runs a function on all cached directories whose path matches
-// the given absolute one. It will be called on a directory's children
-// first. It will not apply the function to parent nodes, regardless
-// of the given path.
-func (d *Dir) walk(absPath string, fun func(*Dir)) {
+// walk runs a function on all cached directories. It will be called
+// on a directory's children first.
+func (d *Dir) walk(fun func(*Dir)) {
 	d.mu.Lock()
 	defer d.mu.Unlock()
 	for _, node := range d.items {
 		if dir, ok := node.(*Dir); ok {
-			dir.walk(absPath, fun)
+			dir.walk(fun)
 		}
 	}
 
-	if d.path == absPath || absPath == "" || strings.HasPrefix(d.path, absPath+"/") {
-		fun(d)
+	fun(d)
+}
+
+// stale returns true if the directory contents will be read the next
+// time it is accessed. stale must be called with d.mu held.
+func (d *Dir) stale(when time.Time) bool {
+	_, stale := d.age(when)
+	return stale
+}
+
+// age returns the duration since the last time the directory contents
+// was read and the content is cosidered stale. age will be 0 and
+// stale true if the last read time is empty.
+// age must be called with d.mu held.
+func (d *Dir) age(when time.Time) (age time.Duration, stale bool) {
+	if d.read.IsZero() {
+		return age, true
 	}
+	age = when.Sub(d.read)
+	stale = age > d.vfs.Opt.DirCacheTime
+	return
 }
 
 // rename should be called after the directory is renamed
@@ -171,14 +199,12 @@ func (d *Dir) delObject(leaf string) {
 // read the directory and sets d.items - must be called with the lock held
 func (d *Dir) _readDir() error {
 	when := time.Now()
-	if d.read.IsZero() {
-		// fs.Debugf(d.path, "Reading directory")
-	} else {
-		age := when.Sub(d.read)
-		if age < d.vfs.Opt.DirCacheTime {
-			return nil
+	if age, stale := d.age(when); stale {
+		if age != 0 {
+			fs.Debugf(d.path, "Re-reading directory (%v old)", age)
 		}
-		fs.Debugf(d.path, "Re-reading directory (%v old)", age)
+	} else {
+		return nil
 	}
 	entries, err := list.DirSorted(d.f, false, d.path)
 	if err == fs.ErrorDirNotFound {
@@ -338,6 +364,33 @@ func (d *Dir) SetModTime(modTime time.Time) error {
 	return nil
 }
 
+func (d *Dir) cachedDir(relativePath string) (dir *Dir) {
+	dir, _ = d.cachedNode(relativePath).(*Dir)
+	return
+}
+
+func (d *Dir) cachedNode(relativePath string) Node {
+	segments := strings.Split(strings.Trim(relativePath, "/"), "/")
+	var node Node = d
+	for _, s := range segments {
+		if s == "" {
+			continue
+		}
+		if dir, ok := node.(*Dir); ok {
+			dir.mu.Lock()
+			node = dir.items[s]
+			dir.mu.Unlock()
+
+			if node != nil {
+				continue
+			}
+		}
+		return nil
+	}
+
+	return node
+}
+
 // Stat looks up a specific entry in the receiver.
 //
 // Stat should return a Node corresponding to the entry.  If the

vfs/dir_test.go

@@ -89,14 +89,19 @@ func TestDirForgetAll(t *testing.T) {
 
 	assert.Equal(t, 1, len(root.items))
 	assert.Equal(t, 1, len(dir.items))
+	assert.False(t, root.read.IsZero())
+	assert.False(t, dir.read.IsZero())
 
 	dir.ForgetAll()
 	assert.Equal(t, 1, len(root.items))
 	assert.Equal(t, 0, len(dir.items))
+	assert.True(t, root.read.IsZero())
+	assert.True(t, dir.read.IsZero())
 
 	root.ForgetAll()
 	assert.Equal(t, 0, len(root.items))
 	assert.Equal(t, 0, len(dir.items))
+	assert.True(t, root.read.IsZero())
 }
 
 func TestDirForgetPath(t *testing.T) {
@@ -113,10 +118,19 @@ func TestDirForgetPath(t *testing.T) {
 
 	assert.Equal(t, 1, len(root.items))
 	assert.Equal(t, 1, len(dir.items))
+	assert.False(t, root.read.IsZero())
+	assert.False(t, dir.read.IsZero())
+
+	root.ForgetPath("dir/notfound", fs.EntryObject)
+	assert.Equal(t, 1, len(root.items))
+	assert.Equal(t, 1, len(dir.items))
+	assert.False(t, root.read.IsZero())
+	assert.True(t, dir.read.IsZero())
 
 	root.ForgetPath("dir", fs.EntryDirectory)
 	assert.Equal(t, 1, len(root.items))
 	assert.Equal(t, 0, len(dir.items))
+	assert.True(t, root.read.IsZero())
 
 	root.ForgetPath("not/in/cache", fs.EntryDirectory)
 	assert.Equal(t, 1, len(root.items))
@@ -151,41 +165,46 @@ func TestDirWalk(t *testing.T) {
 	}
 
 	result = nil
-	root.walk("", fn)
+	root.walk(fn)
 	sort.Strings(result) // sort as there is a map traversal involved
 	assert.Equal(t, []string{"", "dir", "fil", "fil/a", "fil/a/b"}, result)
 
-	result = nil
-	root.walk("dir", fn)
-	assert.Equal(t, []string{"dir"}, result)
-
-	result = nil
-	root.walk("not found", fn)
-	assert.Equal(t, []string(nil), result)
-
-	result = nil
-	root.walk("fil", fn)
-	assert.Equal(t, []string{"fil/a/b", "fil/a", "fil"}, result)
-
-	result = nil
-	fil.(*Dir).walk("fil", fn)
-	assert.Equal(t, []string{"fil/a/b", "fil/a", "fil"}, result)
-
-	result = nil
-	root.walk("fil/a", fn)
-	assert.Equal(t, []string{"fil/a/b", "fil/a"}, result)
-
-	result = nil
-	fil.(*Dir).walk("fil/a", fn)
-	assert.Equal(t, []string{"fil/a/b", "fil/a"}, result)
-
-	result = nil
-	root.walk("fil/a", fn)
-	assert.Equal(t, []string{"fil/a/b", "fil/a"}, result)
-
-	result = nil
-	root.walk("fil/a/b", fn)
-	assert.Equal(t, []string{"fil/a/b"}, result)
+	assert.Nil(t, root.cachedDir("not found"))
+	if dir := root.cachedDir("dir"); assert.NotNil(t, dir) {
+		result = nil
+		dir.walk(fn)
+		assert.Equal(t, []string{"dir"}, result)
+	}
+	if dir := root.cachedDir("fil"); assert.NotNil(t, dir) {
+		result = nil
+		dir.walk(fn)
+		assert.Equal(t, []string{"fil/a/b", "fil/a", "fil"}, result)
+	}
+	if dir := fil.(*Dir); assert.NotNil(t, dir) {
+		result = nil
+		dir.walk(fn)
+		assert.Equal(t, []string{"fil/a/b", "fil/a", "fil"}, result)
+	}
+	if dir := root.cachedDir("fil/a"); assert.NotNil(t, dir) {
+		result = nil
+		dir.walk(fn)
+		assert.Equal(t, []string{"fil/a/b", "fil/a"}, result)
+	}
+	if dir := fil.(*Dir).cachedDir("a"); assert.NotNil(t, dir) {
+		result = nil
+		dir.walk(fn)
+		assert.Equal(t, []string{"fil/a/b", "fil/a"}, result)
+	}
+	if dir := root.cachedDir("fil/a"); assert.NotNil(t, dir) {
+		result = nil
+		dir.walk(fn)
+		assert.Equal(t, []string{"fil/a/b", "fil/a"}, result)
+	}
+	if dir := root.cachedDir("fil/a/b"); assert.NotNil(t, dir) {
+		result = nil
+		dir.walk(fn)
+		assert.Equal(t, []string{"fil/a/b"}, result)
+	}
 }
 
 func TestDirSetModTime(t *testing.T) {

vfs/vfs.go

@@ -227,7 +227,7 @@ func New(f fs.Fs, opt *Options) *VFS {
 	// Start polling function
 	if do := vfs.f.Features().ChangeNotify; do != nil {
 		vfs.pollChan = make(chan time.Duration)
-		do(vfs.notifyFunc, vfs.pollChan)
+		do(vfs.root.ForgetPath, vfs.pollChan)
 		vfs.pollChan <- vfs.Opt.PollInterval
 	} else {
 		fs.Infof(f, "poll-interval is not supported by this remote")
@@ -291,7 +291,7 @@ func (vfs *VFS) WaitForWriters(timeout time.Duration) {
 	tick.Stop()
 	for {
 		writers := 0
-		vfs.root.walk("", func(d *Dir) {
+		vfs.root.walk(func(d *Dir) {
 			fs.Debugf(d.path, "Looking for writers")
 			// NB d.mu is held by walk() here
 			for leaf, item := range d.items {
@@ -498,13 +498,3 @@ func (vfs *VFS) Statfs() (total, used, free int64) {
 	}
 	return
 }
-
-// notifyFunc removes the last path segement for directories and calls ForgetPath with the result.
-//
-// This ensures that new or renamed directories appear in their parent.
-func (vfs *VFS) notifyFunc(relativePath string, entryType fs.EntryType) {
-	if entryType == fs.EntryDirectory {
-		relativePath = path.Dir(relativePath)
-	}
-	vfs.root.ForgetPath(relativePath, entryType)
-}