Docserver: Make the hand-written Cron methods run first rather than last, since

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

Index: chrome/common/extensions/docs/server2/cron_servlet.py
diff --git a/chrome/common/extensions/docs/server2/cron_servlet.py b/chrome/common/extensions/docs/server2/cron_servlet.py
index aa37ae3091c8d02ab97770ac91e0cd5e43081c0b..6209a5e90c89183f695e1f2fb114ee9c7e8162cd 100644
--- a/chrome/common/extensions/docs/server2/cron_servlet.py
+++ b/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,37 @@ class CronServlet(Servlet):
     results = []
 
     try:
+      # Run the hand-written Cron methods first; they can be run in parallel.
+      def run_cron_for_future(target):
+        title = target.__class__.__name__
+        start_time = time.time()
+        future = target.Cron()
+        assert isinstance(future, Future), (
+            '%s.Cron() did not return a Future' % title)
+        def resolve():
+          try:
+            future.Get()
+          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))

[Jeffrey Yasskin, 2013/11/06 22:37:09]

This may not be so useful anymore since even if the work is completely finished
by the .Get() call, you'll still get the time of the .Get call rather than the
time the action took. Maybe take a time for creating the future and a separate
time for running .Get() and add them?

[not at google - send to devlin, 2013/11/11 04:08:33]

Makes sense.

This logging is most for me to check to make sure nothing crazy is going on, but
separating out the times would be useful.

+        return Future(delegate=Gettable(resolve))
+
+      targets = (CreateDataSources(server_instance).values() +
+                 [server_instance.content_providers])
+      title = 'running %s parallel Cron targets' % len(targets)
+      start_time = time.time()
+      _cronlog.info(title)
+      try:
+        futures = [run_cron_for_future(target) for target in targets]
+        for future in futures:

[Jeffrey Yasskin, 2013/11/06 22:37:09]

You could delay this loop until the end of the function, to give it more time to
overlap with rendering.

[not at google - send to devlin, 2013/11/11 04:08:33]

Good idea.

+          future.Get()
+      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))
@@ -178,26 +210,6 @@ class CronServlet(Servlet):
             'example zips',
             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)
-
     except:
       results.append(False)
       # This should never actually happen (each cron step does its own