Docserver: Override Walk in CachingFileSystem

chrome/common/extensions/docs/server2/caching_file_system.py

 # Copyright (c) 2012 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 import posixpath
 import sys
 
 from file_system import FileSystem, StatInfo, FileNotFoundError
 from future import Future
-from path_util import IsDirectory, ToDirectory
+from path_util import AssertIsDirectory, IsDirectory, ToDirectory
 from third_party.json_schema_compiler.memoize import memoize
 
 
 class CachingFileSystem(FileSystem):
   '''FileSystem which implements a caching layer on top of |file_system|. It's
   smart, using Stat() to decided whether to skip Read()ing from |file_system|,
   and only Stat()ing directories never files.
   '''
   def __init__(self, file_system, object_store_creator):
     self._file_system = file_system
     def create_object_store(category, **optargs):
       return object_store_creator.Create(
           CachingFileSystem,
           category='%s/%s' % (file_system.GetIdentity(), category),
           **optargs)
     self._stat_object_store = create_object_store('stat')
     # The read caches can start populated (start_empty=False) because file
     # updates are picked up by the stat, so it doesn't need the force-refresh
     # which starting empty is designed for. Without this optimisation, cron
     # runs are extra slow.
     self._read_object_store = create_object_store('read', start_empty=False)
+    self._walk_cache = create_object_store('walk')

[not at google - send to devlin, 2014/08/29 05:00:14]

Should this be start_empty=False for the same reason as the 'read' cache is?

[ahernandez, 2014/08/29 17:20:00]

Probably, I wasn't sure.

 
   def Refresh(self):
     return self._file_system.Refresh()
 
   def StatAsync(self, path):
     '''Stats the directory given, or if a file is given, stats the file's parent
     directory to get info about the file.
     '''
     # Always stat the parent directory, since it will have the stat of the child
     # anyway, and this gives us an entire directory's stat info at once.
@@ skipping 91 matching lines @@
       # constantly trying to read a file we now know doesn't exist.
       self._read_object_store.SetMulti(
           dict((path, (None, None)) for path in paths
                if stat_futures[path].Get() is None))
       new_results.update(up_to_date_data)
       return new_results
     # Read in the values that were uncached or old.
     return self._file_system.Read(set(paths) - set(up_to_date_data.iterkeys()),
                                   skip_not_found=skip_not_found).Then(next)
 
+  def Walk(self, root, depth=-1):

[not at google - send to devlin, 2014/08/29 05:00:13]

I'm looking at this quite late at night, so, apologies if this is silly - but
could you write this closer to a delegate style?

Like could this be basically a wrapper around self._file_system.Walk() which
cancels the walk (and runs any cached data) if it runs into anything that isn't
new?

Can't think of quite what that would look like, but it's a shame needing to
implement the walk logic again.

Or maybe you could rephrase FileSystem.Walk so that CachingFileSystem.Walk can
hook into it somehow?

I think it's too late at night for me to review this part of the CL :)

+    '''Overrides FileSystem.Walk() to provide caching functionality.
+    '''
+    AssertIsDirectory(root)
+    basepath = root
+
+    def walk(root, depth):
+      if depth == 0:
+        return
+      AssertIsDirectory(root)
+      res = self._walk_cache.Get(root).Get()
+      cache_stat = self._stat_object_store.Get(root).Get()

[ahernandez, 2014/08/29 02:15:11]

These don't need to be Then-ified, right?

+      root_stat = self.Stat(root)
+
+      if res and cache_stat.version == root_stat.version:
+        dirs, files = res
+      else:
+        dirs, files = [], []
+        for f in self.ReadSingle(root).Get():
+          if IsDirectory(f):
+            dirs.append(f)
+          else:
+            files.append(f)
+        self._walk_cache.Set(root, (dirs, files))
+
+      yield root[len(basepath):].rstrip('/'), dirs, files
+
+      for d in dirs:
+        for walkinfo in walk(root + d, depth - 1):
+          yield walkinfo
+
+    for walkinfo in walk(root, depth):
+      yield walkinfo
+
   def GetIdentity(self):
     return self._file_system.GetIdentity()
 
   def __repr__(self):
     return '%s of <%s>' % (type(self).__name__, repr(self._file_system))