Docserver: Override Walk in CachingFileSystem

chrome/common/extensions/docs/server2/compiled_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 sys
 
 import schema_util
 from docs_server_utils import ToUnicode
 from file_system import FileNotFoundError
 from future import Future
 from path_util import AssertIsDirectory, AssertIsFile, ToDirectory
 from third_party.json_schema_compiler import json_parse
 from third_party.json_schema_compiler.memoize import memoize
 from third_party.motemplate import Motemplate
 
 
+_CACHEABLE_FUNCTIONS = set()
 _SINGLE_FILE_FUNCTIONS = set()
 
 
+def _GetUnboundFunction(fn):
+  '''Functions bound to an object are separate from the unbound

[ahernandez, 2014/08/29 01:49:42]

I don't know how to explain this concisely, but the SingleFile annotation wasn't
actually doing it's job in a lot of cases because when a method is annotated,
the unbound version of it is passed to the annotation function, e.g. SingleFile.
But then the bound version is passed to compiled fs, e.g.
c_fs_factory.Create(self._comp_func, ...), so when checking if the compilation
function is in the set of single file functions, the test fails because the
bound method != the unbound method. So I wrote this to ensure that the unbound
version is always grabbed.

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

Neat, nice catch. This nested decorator stuff in Python always messed me up.

+  defintion. This causes issues when checking for membership,
+  so always get the unbound function, if possible.
+  '''
+  return getattr(fn, 'im_func', fn)
+
+
+def Cache(fn):
+  '''A decorator which can be applied to the compilation function
+  passed to CompiledFileSystem.Create, indicating that file/list data
+  should be cached.
+
+  This decorator should be listed first in any list of decorators, along
+  with the SingleFile decorator below.
+  '''
+  _CACHEABLE_FUNCTIONS.add(_GetUnboundFunction(fn))
+  return fn
+
+
 def SingleFile(fn):
   '''A decorator which can be optionally applied to the compilation function
   passed to CompiledFileSystem.Create, indicating that the function only
   needs access to the file which is given in the function's callback. When
   this is the case some optimisations can be done.
 
   Note that this decorator must be listed first in any list of decorators to
   have any effect.
   '''
-  _SINGLE_FILE_FUNCTIONS.add(fn)
+  _SINGLE_FILE_FUNCTIONS.add(_GetUnboundFunction(fn))
   return fn
 
 
 def Unicode(fn):
   '''A decorator which can be optionally applied to the compilation function
   passed to CompiledFileSystem.Create, indicating that the function processes
   the file's data as Unicode text.
   '''
 
   # The arguments passed to fn can be (self, path, data) or (path, data). In
@@ skipping 53 matching lines @@
                                 compilation_function,
                                 create_object_store('file'),
                                 create_object_store('list'))
 
     @memoize
     def ForJson(self, file_system):
       '''A CompiledFileSystem specifically for parsing JSON configuration data.
       These are memoized over file systems tied to different branches.
       '''
       return self.Create(file_system,
-                         SingleFile(lambda _, data:
-                             json_parse.Parse(ToUnicode(data))),
+                         Cache(SingleFile(lambda _, data:

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

JSON should be ... fairly cheap? Very commonly used, though, hm.

Could you do some benchmarking perhaps? Starting from nothing Cached and start
adding back in things?

cron_servlet_test.py is actually pretty good - not only does the time running
reflect well on the pure computational time it tries to do this, but if you run
"top" at the same time you can see how the memory usage is affected.

(I expect that Motemplate compilation will be quite expensive, but, prove me
wrong!).

+                             json_parse.Parse(ToUnicode(data)))),
                          CompiledFileSystem,
                          category='json')
 
     @memoize
     def ForTemplates(self, file_system):
       '''Creates a CompiledFileSystem for parsing templates.
       '''
       return self.Create(
           file_system,
           SingleFile(lambda path, text: Motemplate(ToUnicode(text), name=path)),
@@ skipping 12 matching lines @@
   def __init__(self,
                file_system,
                compilation_function,
                file_object_store,
                list_object_store):
     self._file_system = file_system
     self._compilation_function = compilation_function
     self._file_object_store = file_object_store
     self._list_object_store = list_object_store
 
+  def _GetFromStore(self, key, store):
+    if _GetUnboundFunction(self._compilation_function) in _CACHEABLE_FUNCTIONS:
+      return store.Get(key)
+    return Future(value=None)
+
+  def _SetToStore(self, key, value, store):
+    if _GetUnboundFunction(self._compilation_function) in _CACHEABLE_FUNCTIONS:
+      store.Set(key, value)
+
+  def _GetFromFileStore(self, key):
+    return self._GetFromStore(key, self._file_object_store)
+
+  def _GetFromListStore(self, key):
+    return self._GetFromStore(key, self._list_object_store)
+
+  def _SetToFileStore(self, key, value):
+    self._SetToStore(key, value, self._file_object_store)
+
+  def _SetToListStore(self, key, value):
+    self._SetToStore(key, value, self._list_object_store)
+
   def _RecursiveList(self, path):
     '''Returns a Future containing the recursive directory listing of |path| as
     a flat list of paths.
     '''
     def split_dirs_from_files(paths):
       '''Returns a tuple (dirs, files) where |dirs| contains the directory
       names in |paths| and |files| contains the files.
       '''
       result = [], []
       for path in paths:
@@ skipping 45 matching lines @@
     AssertIsFile(path)
 
     try:
       version = self._file_system.Stat(path).version
     except FileNotFoundError:
       if skip_not_found:
         version = None
       else:
         return Future(exc_info=sys.exc_info())
 
-    cache_entry = self._file_object_store.Get(path).Get()
+    cache_entry = self._GetFromFileStore(path).Get()

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

These wrapper functions are nice, but if you had static _Get/_Set methods it
wouldn't be too bad to write just:

cache_entry = _Get(self._file_object_store, path).Get()

Ditto _Set. Then we wouldn't need to add quite so many lines.

     if (cache_entry is not None) and (version == cache_entry.version):
       return Future(value=cache_entry._cache_data)
 
     def compile_(files):
       cache_data = self._compilation_function(path, files)
-      self._file_object_store.Set(path, _CacheEntry(cache_data, version))
+      self._SetToFileStore(path, _CacheEntry(cache_data, version))

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

(FWIW I think that "set in" reads slightly better than "set to").

       return cache_data
 
     return self._file_system.ReadSingle(
         path, skip_not_found=skip_not_found).Then(compile_)
 
   def GetFromFileListing(self, path):
     '''Calls |compilation_function| on the listing of the files at |path|.
     Assumes that the path given is to a directory.
     '''
     AssertIsDirectory(path)
 
     try:
       version = self._file_system.Stat(path).version
     except FileNotFoundError:
       return Future(exc_info=sys.exc_info())
 
-    cache_entry = self._list_object_store.Get(path).Get()
+    cache_entry = self._GetFromListStore(path).Get()
     if (cache_entry is not None) and (version == cache_entry.version):
       return Future(value=cache_entry._cache_data)
 
     def next(files):
       cache_data = self._compilation_function(path, files)
-      self._list_object_store.Set(path, _CacheEntry(cache_data, version))
+      self._SetToListStore(path, _CacheEntry(cache_data, version))
       return cache_data
     return self._RecursiveList(path).Then(next)
 
   def GetFileVersion(self, path):
-    cache_entry = self._file_object_store.Get(path).Get()
+    cache_entry = self._GetFromFileStore(path).Get()
     if cache_entry is not None:
       return cache_entry.version
     return self._file_system.Stat(path).version
 
   def GetFileListingVersion(self, path):
     path = ToDirectory(path)
-    cache_entry = self._list_object_store.Get(path).Get()
+    cache_entry = self._GetFromListStore(path).Get()
     if cache_entry is not None:
       return cache_entry.version
     return self._file_system.Stat(path).version
 
   def FileExists(self, path):
     return self._file_system.Exists(path)
 
   def GetIdentity(self):
     return self._file_system.GetIdentity()