Docserver: Make the hand-written Cron methods return a Future and run first

rather than last, so that they can be parallelised and have the most effect.
Implement a few of the more trivial Cron methods, including moving most
FeaturesBundle methods to return Futures.

BUG=305280
R=jyasskin@chromium.org
NOTRY=true

Review URL: https://codereview.chromium.org/63203002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@233427 0039d316-1c4b-4281-b951-d872f2087c98

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

@@ -66,7 +66,7 @@ class APIListDataSource(object):
     def _GenerateAPIDict(self):
       documented_apis = self._cache.GetFromFileListing(
           PUBLIC_TEMPLATE_PATH).Get()
-      api_features = self._features_bundle.GetAPIFeatures()
+      api_features = self._features_bundle.GetAPIFeatures().Get()
 
       def FilterAPIs(platform):
         return (api for api in api_features.itervalues()

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

@@ -36,7 +36,7 @@ class APIModels(object):
     # features file. APIs are those which either implicitly or explicitly have
     # no parent feature (e.g. app, app.window, and devtools.inspectedWindow are
     # APIs; runtime.onConnectNative is not).
-    api_features = self._features_bundle.GetAPIFeatures()
+    api_features = self._features_bundle.GetAPIFeatures().Get()
     return [name for name, feature in api_features.iteritems()
             if ('.' not in name or
                 name.rsplit('.', 1)[0] not in api_features or

chrome/common/extensions/docs/server2/app.yaml

 application: chrome-apps-doc
-version: 2-38-1
+version: 2-38-2
 runtime: python27
 api_version: 1
 threadsafe: false

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

@@ -85,5 +85,7 @@ class ContentProvider(object):
 
   def Cron(self):
     # Running Refresh() on the file system is enough to pull GitHub content,
-    # which is all we need for now.
-    self.file_system.Refresh().Get()
+    # which is all we need for now while the full render-every-page cron step
+    # is in effect.
+    # TODO(kalman): Walk over the whole filesystem and compile the content.
+    return self.file_system.Refresh()

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

@@ -8,6 +8,7 @@ import posixpath
 
 from chroot_file_system import ChrootFileSystem
 from content_provider import ContentProvider
+from future import Gettable, Future
 from svn_constants import JSON_PATH
 from third_party.json_schema_compiler.memoize import memoize
 
@@ -102,5 +103,6 @@ class ContentProviders(object):
                            supports_zip=supports_zip)
 
   def Cron(self):
-    for name, config in self._GetConfig().iteritems():
-      self._CreateContentProvider(name, config).Cron()
+    futures = [self._CreateContentProvider(name, config).Cron()
+               for name, config in self._GetConfig().iteritems()]
+    return Future(delegate=Gettable(lambda: [f.Get() for f in futures]))

chrome/common/extensions/docs/server2/cron.yaml

@@ -2,4 +2,4 @@ cron:
 - description: Repopulates all cached data.
   url: /_cron
   schedule: every 5 minutes
-  target: 2-38-1
+  target: 2-38-2

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

@@ -15,6 +15,7 @@ from data_source_registry import CreateDataSources
 from empty_dir_file_system import EmptyDirFileSystem
 from environment import IsDevServer
 from file_system_util import CreateURLsFromPaths
+from future import Gettable, Future
 from github_file_system_provider import GithubFileSystemProvider
 from host_file_system_provider import HostFileSystemProvider
 from object_store_creator import ObjectStoreCreator
@@ -151,6 +152,41 @@ class CronServlet(Servlet):
     results = []
 
     try:
+      # Start running the hand-written Cron methods first; they can be run in
+      # parallel. They are resolved at the end.
+      def run_cron_for_future(target):
+        title = target.__class__.__name__
+        start_time = time.time()
+        future = target.Cron()
+        init_time = time.time() - start_time
+        assert isinstance(future, Future), (
+            '%s.Cron() did not return a Future' % title)
+        def resolve():
+          start_time = time.time()
+          try:
+            future.Get()
+          except Exception as e:
+            _cronlog.error('%s: error %s' % (title, traceback.format_exc()))
+            results.append(False)
+            if IsDeadlineExceededError(e): raise
+          finally:
+            resolve_time = time.time() - start_time
+            _cronlog.info(
+                '%s: used %s seconds, %s to initialize and %s to resolve' %
+                (title, init_time + resolve_time, init_time, resolve_time))
+        return Future(delegate=Gettable(resolve))
+
+      targets = (CreateDataSources(server_instance).values() +
+                 [server_instance.content_providers])
+      title = 'initializing %s parallel Cron targets' % len(targets)
+      start_time = time.time()
+      _cronlog.info(title)
+      try:
+        cron_futures = [run_cron_for_future(target) for target in targets]
+      finally:
+        _cronlog.info('%s took %s seconds' % (title, time.time() - start_time))
+
+
       # Rendering the public templates will also pull in all of the private
       # templates.
       results.append(request_files_in_dir(svn_constants.PUBLIC_TEMPLATE_PATH))
@@ -179,24 +215,15 @@ class CronServlet(Servlet):
             example_zips,
             lambda path: render('extensions/examples/' + path)))
 
-      def run_cron(data_source):
-        title = data_source.__class__.__name__
-        _cronlog.info('%s: starting' % title)
-        start_time = time.time()
-        try:
-          data_source.Cron()
-        except Exception as e:
-          _cronlog.error('%s: error %s' % (title, traceback.format_exc()))
-          results.append(False)
-          if IsDeadlineExceededError(e): raise
-        finally:
-          _cronlog.info(
-              '%s: took %s seconds' % (title, time.time() - start_time))
-
-      for data_source in CreateDataSources(server_instance).values():
-        run_cron(data_source)
-
-      run_cron(server_instance.content_providers)
+      # Resolve the hand-written Cron method futures.
+      title = 'resolving %s parallel Cron targets' % len(targets)
+      _cronlog.info(title)
+      start_time = time.time()
+      try:
+        for future in cron_futures:
+          future.Get()
+      finally:
+        _cronlog.info('%s took %s seconds' % (title, time.time() - start_time))
 
     except:
       results.append(False)

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

@@ -85,7 +85,9 @@ class CronServletTest(unittest.TestCase):
   def testSafeRevision(self):
     test_data = {
       'api': {
-        '_manifest_features.json': '{}'
+        '_api_features.json': '{}',
+        '_manifest_features.json': '{}',
+        '_permission_features.json': '{}',
       },
       'docs': {
         'examples': {
@@ -110,6 +112,7 @@ class CronServletTest(unittest.TestCase):
             'content_providers.json': ReadFile('%s/content_providers.json' %
                                                JSON_PATH),
             'manifest.json': '{}',
+            'permissions.json': '{}',
             'strings.json': '{}',
             'apps_sidenav.json': '{}',
             'extensions_sidenav.json': '{}',

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

@@ -3,15 +3,19 @@
 # found in the LICENSE file.
 
 import features_utility
+from future import Gettable, Future
 import svn_constants
 from third_party.json_schema_compiler.json_parse import Parse
 
 
-def _AddPlatformsFromDependencies(feature, features_bundle):
+def _AddPlatformsFromDependencies(feature,
+                                  api_features,
+                                  manifest_features,
+                                  permission_features):
   features_map = {
-    'api': features_bundle.GetAPIFeatures(),
-    'manifest': features_bundle.GetManifestFeatures(),
-    'permission': features_bundle.GetPermissionFeatures()
+    'api': api_features,
+    'manifest': manifest_features,
+    'permission': permission_features,
   }
   dependencies = feature.get('dependencies')
   if dependencies is None:
@@ -38,17 +42,19 @@ class _FeaturesCache(object):
     self._extra_paths = json_paths[1:]
 
   def _CreateCache(self, _, features_json):
+    extra_path_futures = [self._file_system.ReadSingle(path)
+                          for path in self._extra_paths]
     features = features_utility.Parse(Parse(features_json))
-    for path in self._extra_paths:
-      extra_json = self._file_system.ReadSingle(path).Get()
+    for path_future in extra_path_futures:
+      extra_json = path_future.Get()
       features = features_utility.MergedWith(
           features_utility.Parse(Parse(extra_json)), features)
     return features
 
   def GetFeatures(self):
     if self._json_path is None:
-      return {}
-    return self._cache.GetFromFile(self._json_path).Get()
+      return Future(value={})
+    return self._cache.GetFromFile(self._json_path)
 
 
 class FeaturesBundle(object):
@@ -79,14 +85,23 @@ class FeaturesBundle(object):
 
   def GetAPIFeatures(self):
     api_features = self._object_store.Get('api_features').Get()
-    if api_features is None:
-      api_features = self._api_cache.GetFeatures()
+    if api_features is not None:
+      return Future(value=api_features)
+
+    api_features_future = self._api_cache.GetFeatures()
+    manifest_features_future = self._manifest_cache.GetFeatures()
+    permission_features_future = self._permission_cache.GetFeatures()
+    def resolve():
+      api_features = api_features_future.Get()
+      manifest_features = manifest_features_future.Get()
+      permission_features = permission_features_future.Get()
       # TODO(rockot): Handle inter-API dependencies more gracefully.
       # Not yet a problem because there is only one such case (windows -> tabs).
       # If we don't store this value before annotating platforms, inter-API
       # dependencies will lead to infinite recursion.
-      self._object_store.Set('api_features', api_features)
       for feature in api_features.itervalues():
-        _AddPlatformsFromDependencies(feature, self)
+        _AddPlatformsFromDependencies(
+            feature, api_features, manifest_features, permission_features)
       self._object_store.Set('api_features', api_features)
-    return api_features
+      return api_features
+    return Future(delegate=Gettable(resolve))

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

@@ -173,7 +173,7 @@ class FeaturesBundleTest(unittest.TestCase):
     }
     self.assertEqual(
         expected_features,
-        self._server.features_bundle.GetManifestFeatures())
+        self._server.features_bundle.GetManifestFeatures().Get())
 
   def testPermissionFeatures(self):
     expected_features = {
@@ -206,7 +206,7 @@ class FeaturesBundleTest(unittest.TestCase):
     }
     self.assertEqual(
         expected_features,
-        self._server.features_bundle.GetPermissionFeatures())
+        self._server.features_bundle.GetPermissionFeatures().Get())
 
   def testAPIFeatures(self):
     expected_features = {
@@ -254,7 +254,7 @@ class FeaturesBundleTest(unittest.TestCase):
     }
     self.assertEqual(
         expected_features,
-        self._server.features_bundle.GetAPIFeatures())
+        self._server.features_bundle.GetAPIFeatures().Get())
 
 if __name__ == '__main__':
   unittest.main()

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

@@ -6,6 +6,7 @@ import json
 
 from data_source import DataSource
 import features_utility
+from future import Gettable, Future
 from manifest_features import ConvertDottedKeysToNested
 from third_party.json_schema_compiler.json_parse import Parse
 
@@ -105,27 +106,29 @@ class ManifestDataSource(DataSource):
         ManifestDataSource)
 
   def _CreateManifestData(self):
-    def for_templates(manifest_features, platform):
-      return _AddLevelAnnotations(
-          _ListifyAndSortDocs(
-              ConvertDottedKeysToNested(
-                  features_utility.Filtered(manifest_features, platform)),
-              app_name=platform.capitalize()))
-    manifest_features = self._features_bundle.GetManifestFeatures()
-    return {
-      'apps': for_templates(manifest_features, 'apps'),
-      'extensions': for_templates(manifest_features, 'extensions')
-    }
-
-  def _GetCachedManifestData(self, force_update=False):
+    future_manifest_features = self._features_bundle.GetManifestFeatures()
+    def resolve():
+      manifest_features = future_manifest_features.Get()
+      def for_templates(manifest_features, platform):
+        return _AddLevelAnnotations(_ListifyAndSortDocs(
+            ConvertDottedKeysToNested(
+                features_utility.Filtered(manifest_features, platform)),
+            app_name=platform.capitalize()))
+      return {
+        'apps': for_templates(manifest_features, 'apps'),
+        'extensions': for_templates(manifest_features, 'extensions')
+      }
+    return Future(delegate=Gettable(resolve))
+
+  def _GetCachedManifestData(self):
     data = self._object_store.Get('manifest_data').Get()
-    if data is None or force_update:
-      data = self._CreateManifestData()
+    if data is None:
+      data = self._CreateManifestData().Get()
       self._object_store.Set('manifest_data', data)
     return data
 
   def Cron(self):
-    self._GetCachedManifestData(force_update=True)
+    return self._CreateManifestData()
 
   def get(self, key):
     return self._GetCachedManifestData().get(key)

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

@@ -9,6 +9,7 @@ import unittest
 
 from compiled_file_system import CompiledFileSystem
 from features_bundle import FeaturesBundle
+from future import Future
 import manifest_data_source
 from object_store_creator import ObjectStoreCreator
 
@@ -246,7 +247,7 @@ class ManifestDataSourceTest(unittest.TestCase):
 
     class FakeFeaturesBundle(object):
       def GetManifestFeatures(self):
-        return manifest_features
+        return Future(value=manifest_features)
 
     class FakeServerInstance(object):
       def __init__(self):

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

@@ -7,6 +7,7 @@ from operator import itemgetter
 
 from data_source import DataSource
 import features_utility as features
+from future import Gettable, Future
 from svn_constants import PRIVATE_TEMPLATE_PATH
 from third_party.json_schema_compiler.json_parse import Parse
 
@@ -50,38 +51,39 @@ class PermissionsDataSource(DataSource):
         server_instance.host_file_system_provider.GetTrunk())
 
   def _CreatePermissionsData(self):
-    api_features = self._features_bundle.GetAPIFeatures()
-    permission_features = self._features_bundle.GetPermissionFeatures()
+    api_features_future = self._features_bundle.GetAPIFeatures()
+    permission_features_future = self._features_bundle.GetPermissionFeatures()
+    def resolve():
+      permission_features = permission_features_future.Get()
+      _AddDependencyDescriptions(permission_features, api_features_future.Get())
 
-    def filter_for_platform(permissions, platform):
-      return _ListifyPermissions(features.Filtered(permissions, platform))
+      # Turn partial templates into descriptions, ensure anchors are set.
+      for permission in permission_features.values():
+        if not 'anchor' in permission:
+          permission['anchor'] = permission['name']
+        if 'partial' in permission:
+          permission['description'] = self._template_cache.GetFromFile('%s/%s' %
+              (PRIVATE_TEMPLATE_PATH, permission['partial'])).Get()
+          del permission['partial']
 
-    _AddDependencyDescriptions(permission_features, api_features)
-    # Turn partial templates into descriptions, ensure anchors are set.
-    for permission in permission_features.values():
-      if not 'anchor' in permission:
-        permission['anchor'] = permission['name']
-      if 'partial' in permission:
-        permission['description'] = self._template_cache.GetFromFile('%s/%s' %
-            (PRIVATE_TEMPLATE_PATH, permission['partial'])).Get()
-        del permission['partial']
-
-    return {
-      'declare_apps': filter_for_platform(permission_features, 'apps'),
-      'declare_extensions': filter_for_platform(
-          permission_features, 'extensions')
-    }
+      def filter_for_platform(permissions, platform):
+        return _ListifyPermissions(features.Filtered(permissions, platform))
+      return {
+        'declare_apps': filter_for_platform(permission_features, 'apps'),
+        'declare_extensions': filter_for_platform(
+            permission_features, 'extensions')
+      }
+    return Future(delegate=Gettable(resolve))
 
   def _GetCachedPermissionsData(self):
     data = self._object_store.Get('permissions_data').Get()
     if data is None:
-      data = self._CreatePermissionsData()
+      data = self._CreatePermissionsData().Get()
       self._object_store.Set('permissions_data', data)
     return data
 
   def Cron(self):
-    # TODO(kalman): Implement this.
-    pass
+    return self._CreatePermissionsData()
 
   def get(self, key):
     return self._GetCachedPermissionsData().get(key)

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

@@ -6,6 +6,7 @@ import posixpath
 from urlparse import urlsplit
 
 from file_system import FileNotFoundError
+from future import Gettable, Future
 
 class Redirector(object):
   def __init__(self, compiled_fs_factory, file_system):
@@ -60,6 +61,9 @@ class Redirector(object):
   def Cron(self):
     ''' Load files during a cron run.
     '''
+    futures = []
     for root, dirs, files in self._file_system.Walk(''):
       if 'redirects.json' in files:
-        self._cache.GetFromFile(posixpath.join(root, 'redirects.json')).Get()
+        futures.append(
+            self._cache.GetFromFile(posixpath.join(root, 'redirects.json')))
+    return Future(delegate=Gettable(lambda: [f.Get() for f in futures]))

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

@@ -87,7 +87,7 @@ class RedirectorTest(unittest.TestCase):
         self._redirector.Redirect('https://code.google.com', ''))
 
   def testCron(self):
-    self._redirector.Cron()
+    self._redirector.Cron().Get()
 
     expected_paths = set([
       'redirects.json',

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

@@ -80,8 +80,7 @@ class SidenavDataSource(DataSource):
     futures = [
         self._cache.GetFromFile('%s/%s_sidenav.json' % (JSON_PATH, platform))
         for platform in ('apps', 'extensions')]
-    for future in futures:
-      future.Get()
+    return Future(delegate=Gettable(lambda: [f.Get() for f in futures]))
 
   def get(self, key):
     sidenav = copy.deepcopy(self._cache.GetFromFile(

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

@@ -151,7 +151,7 @@ class SamplesDataSourceTest(unittest.TestCase):
     # Ensure Cron doesn't rely on request.
     sidenav_data_source = SidenavDataSource(
         ServerInstance.ForTest(file_system), request=None)
-    sidenav_data_source.Cron()
+    sidenav_data_source.Cron().Get()
 
     # If Cron fails, apps_sidenav.json will not be cached, and the _cache_data
     # access will fail.

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

@@ -13,8 +13,11 @@ class StringsDataSource(DataSource):
         server_instance.host_file_system_provider.GetTrunk())
     self._strings_json_path = server_instance.strings_json_path
 
+  def _GetStringsData(self):
+    return self._cache.GetFromFile(self._strings_json_path)
+
   def Cron(self):
-    self._cache.GetFromFile(self._strings_json_path).Get()
+    return self._GetStringsData()
 
   def get(self, key):
-    return self._cache.GetFromFile(self._strings_json_path).Get()[key]
+    return self._GetStringsData().Get().get(key)

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

@@ -9,6 +9,7 @@ import traceback
 from data_source import DataSource
 from docs_server_utils import FormatKey
 from file_system import FileNotFoundError
+from future import Future
 from svn_constants import PRIVATE_TEMPLATE_PATH
 
 
@@ -32,4 +33,4 @@ class TemplateDataSource(DataSource):
   def Cron(self):
     # TODO(kalman): Implement this; probably by finding all files that can be
     # compiled to templates underneath |self._partial_dir| and compiling them.
-    pass
+    return Future(value=())