Extensions Docs Server: Large performance increase

chrome/common/extensions/docs/server2/file_system_cache.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 os
 
+import appengine_memcache as memcache
+
+FS_CACHE_CRON = 'FileSystemCache.Cron'
+FS_CACHE_RENDER = 'FileSystemCache.Render'
+FS_CACHE_PERMS = 'FileSystemCache.Perms'
+FS_CACHE_JSON = 'FileSystemCache.JSON'
+FS_CACHE_IDL = 'FileSystemCache.IDL'
+FS_CACHE_LIST = 'FileSystemCache.List'
+FS_CACHE_ZIP = 'FileSystemCache.Zip'
+FS_CACHE_INTRO = 'FileSystemCache.Intro'
+FS_CACHE_EXTENSIONS = 'FileSystemCache.Extensions'
+FS_CACHE_APPS = 'FileSystemCache.Apps'
+FS_CACHE_STATIC = 'FileSystemCache.Static'
+FS_CACHE_HANDLEBAR = 'FileSystemCache.Handlebar'

[not at google - send to devlin, 2012/08/20 05:27:10]

nit: alphabet

extra points (extra nitty): align the = vertically, too.

Although... it seems to me like the FS_CACHE and the FileSystemCache. prefixes
are unnecessary; FS_CACHE is implied (because these are in that module), and
FileSystemCache. should be prepended to the namespace on construction of the
FileSystemCache.

Or this should go in AppengineMemcache, or, that should be updated to be
similar. Something consistent.

[cduvall, 2012/08/20 21:28:09]

Done.

+
+class _CacheEntry(object):
+  def __init__(self, cache_data, version):
+    self._cache_data = cache_data
+    self.version = version
+
 class FileSystemCache(object):

[not at google - send to devlin, 2012/08/20 05:27:10]

I reckon we should rename this (as in my email) to CompiledFileSystem or
CompilingFileSystem or TypedFileSystem. The purpose of it has changed.

... but after this patch goes in. No need to add extra noise.

   """This class caches FileSystem data that has been processed.
   """
   class Builder(object):

[not at google - send to devlin, 2012/08/20 05:27:10]

ditto (after this patch): rename to Factory

     """A class to build a FileSystemCache.
     """
-    def __init__(self, file_system):
+    def __init__(self, file_system, memcache):
       self._file_system = file_system
+      self._memcache = memcache
 
-    def build(self, populate_function):
-      return FileSystemCache(self._file_system, populate_function)
+    def build(self, populate_function, namespace):

[not at google - send to devlin, 2012/08/20 05:27:10]

ditto (after this patch): rename to Create

+      return FileSystemCache(self._file_system,
+                             populate_function,
+                             self._memcache,
+                             namespace)
 
-  class _CacheEntry(object):
-    def __init__(self, cache_data, version):
-      self._cache_data = cache_data
-      self.version = version
-
-  def __init__(self, file_system, populate_function):
+  def __init__(self, file_system, populate_function, memcache, namespace):
     self._file_system = file_system
     self._populate_function = populate_function
     self._cache = {}

[not at google - send to devlin, 2012/08/20 05:27:10]

not used anymore?

[cduvall, 2012/08/20 21:28:09]

Done.

+    self._memcache = memcache
+    self._namespace = namespace
+
+  def _MakeKey(self, key):
+    return self._namespace + '.' + key
 
   def _RecursiveList(self, files):
     all_files = files[:]
     dirs = {}
     for filename in files:
       if filename.endswith('/'):
         all_files.remove(filename)
         dirs.update(self._file_system.Read([filename]).Get())
     for dir_, files in dirs.iteritems():
       all_files.extend(self._RecursiveList([dir_ + f for f in files]))
     return all_files
 
   def GetFromFile(self, path):
     """Calls |populate_function| on the contents of the file at |path|.
     """
     version = self._file_system.Stat(path).version
-    if path in self._cache:
-      if version > self._cache[path].version:
-        self._cache.pop(path)
+    cache_entry = self._memcache.Get(self._MakeKey(path),
+                                     memcache.MEMCACHE_FILE_SYSTEM_CACHE,
+                                     time=0)
+
+    if  cache_entry is not None:

[not at google - send to devlin, 2012/08/20 05:27:10]

nit: extra space

[cduvall, 2012/08/20 21:28:09]

Done.

+      if version != cache_entry.version:
+        self._memcache.Delete(self._MakeKey(path),
+                              memcache.MEMCACHE_FILE_SYSTEM_CACHE)

[not at google - send to devlin, 2012/08/20 05:27:10]

I don't think we need to Delete this, we Set it right afterwards?

[cduvall, 2012/08/20 21:28:09]

Done.

       else:
-        return self._cache[path]._cache_data
-    cache_data = self._file_system.ReadSingle(path)
-    self._cache[path] = self._CacheEntry(self._populate_function(cache_data),
-                                         version)
-    return self._cache[path]._cache_data
+        return cache_entry._cache_data
+    cache_data = self._populate_function(self._file_system.ReadSingle(path))
+    self._memcache.Set(self._MakeKey(path),
+                       _CacheEntry(cache_data, version),
+                       memcache.MEMCACHE_FILE_SYSTEM_CACHE,
+                       time=0)
+    return cache_data
 
   def GetFromFileListing(self, path):
     """Calls |populate_function| on the listing of the files at |path|.
     Assumes that the path given is to a directory.
     """
     if not path.endswith('/'):
       path += '/'
     version = self._file_system.Stat(path).version
-    if path in self._cache:
-      if version > self._cache[path].version:
-        self._cache.pop(path)
+    cache_entry = self._memcache.Get(
+        self._MakeKey(path),
+        memcache.MEMCACHE_FILE_SYSTEM_CACHE_LISTING,
+        time=0)
+    if cache_entry is not None:
+      if version != cache_entry.version:
+        self._memcache.Delete(self._MakeKey(path),
+                              memcache.MEMCACHE_FILE_SYSTEM_CACHE_LISTING)
       else:
-        return self._cache[path]._cache_data
-    cache_data = self._RecursiveList(
-        [path + f for f in self._file_system.ReadSingle(path)])
-    self._cache[path] = self._CacheEntry(self._populate_function(cache_data),
-                                         version)
-    return self._cache[path]._cache_data
+        return cache_entry._cache_data
+    cache_data = self._populate_function(self._RecursiveList(
+        [path + f for f in self._file_system.ReadSingle(path)]))
+    self._memcache.Set(self._MakeKey(path),
+                       _CacheEntry(cache_data, version),
+                       memcache.MEMCACHE_FILE_SYSTEM_CACHE_LISTING,
+                       time=0)
+    return cache_data