From 1d14972e41dce78c97fe4d6623c9b07d3a7a3ed0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabian=20M=C3=B6ller?= Date: Tue, 25 Sep 2018 18:27:37 +0200 Subject: [PATCH] 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 | 109 +++++++++++++++++++++++++++++++++++------------- vfs/dir_test.go | 83 ++++++++++++++++++++++-------------- vfs/vfs.go | 14 +------ 3 files changed, 134 insertions(+), 72 deletions(-) diff --git a/vfs/dir.go b/vfs/dir.go index 16084b833..82ff5693c 100644 --- a/vfs/dir.go +++ b/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 diff --git a/vfs/dir_test.go b/vfs/dir_test.go index 9ce20576a..958160f2a 100644 --- a/vfs/dir_test.go +++ b/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) { diff --git a/vfs/vfs.go b/vfs/vfs.go index 05bcf4392..e49235228 100644 --- a/vfs/vfs.go +++ b/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) -}