rebaseline_server: cache results in long-running ImageDiffDB instance

Rather than rebuilding the ImageDiffDB from scratch every time we update expected/actual results, keep the same ImageDiffDB instance around and add image pairs to it.

This makes updating results within a long-running server.py *much* faster, and tests out an idea I'm ruminating in https://goto.google.com/LongRunningImageDiffDBServer : we could run an ImageDiffDB server that developers could connect to from their locally running rebaseline_server instances, for much faster launch times.

BUG=skia:2414
NOTRY=True
R=rmistry@google.com

Author: epoger@google.com

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

diff --git a/gm/rebaseline_server/compare_to_expectations.py b/gm/rebaseline_server/compare_to_expectations.py
index e1a5f5f..dad206d 100755 (executable)
--- a/gm/rebaseline_server/compare_to_expectations.py
+++ b/gm/rebaseline_server/compare_to_expectations.py
@@ -63,32 +63,30 @@ class ExpectationComparisons(results.BaseComparisons):
   are immutable.  If you want to update the results based on updated JSON
   file contents, you will need to create a new ExpectationComparisons object."""
 
-  def __init__(self, actuals_root=results.DEFAULT_ACTUALS_DIR,
+  def __init__(self, image_diff_db, actuals_root=results.DEFAULT_ACTUALS_DIR,
                expected_root=DEFAULT_EXPECTATIONS_DIR,
                ignore_failures_file=DEFAULT_IGNORE_FAILURES_FILE,
-               generated_images_root=results.DEFAULT_GENERATED_IMAGES_ROOT,
                diff_base_url=None, builder_regex_list=None):
     """
     Args:
+      image_diff_db: instance of ImageDiffDB we use to cache the image diffs
       actuals_root: root directory containing all actual-results.json files
       expected_root: root directory containing all expected-results.json files
       ignore_failures_file: if a file with this name is found within
           expected_root, ignore failures for any tests listed in the file
-      generated_images_root: directory within which to create all pixel diffs;
-          if this directory does not yet exist, it will be created
       diff_base_url: base URL within which the client should look for diff
           images; if not specified, defaults to a "file:///" URL representation
-          of generated_images_root
+          of image_diff_db's storage_root
       builder_regex_list: List of regular expressions specifying which builders
           we will process. If None, process all builders.
     """
     time_start = int(time.time())
     if builder_regex_list != None:
       self.set_match_builders_pattern_list(builder_regex_list)
-    self._image_diff_db = imagediffdb.ImageDiffDB(generated_images_root)
+    self._image_diff_db = image_diff_db
     self._diff_base_url = (
         diff_base_url or
-        url_utils.create_filepath_url(generated_images_root))
+        url_utils.create_filepath_url(image_diff_db.storage_root))
     self._actuals_root = actuals_root
     self._expected_root = expected_root
     self._ignore_failures_on_these_tests = []
@@ -402,11 +400,12 @@ def main():
       help='Directory within which to download images and generate diffs; '
       'defaults to \'%(default)s\' .')
   args = parser.parse_args()
+  image_diff_db = imagediffdb.ImageDiffDB(storage_root=args.workdir)
   results_obj = ExpectationComparisons(
+      image_diff_db=image_diff_db,
       actuals_root=args.actuals,
       expected_root=args.expectations,
-      ignore_failures_file=args.ignore_failures_file,
-      generated_images_root=args.workdir)
+      ignore_failures_file=args.ignore_failures_file)
   gm_json.WriteToFile(
       results_obj.get_packaged_results_of_type(results_type=args.results),
       args.outfile)

diff --git a/gm/rebaseline_server/compare_to_expectations_test.py b/gm/rebaseline_server/compare_to_expectations_test.py
index 76e4a7b..d2b2dd6 100755 (executable)
--- a/gm/rebaseline_server/compare_to_expectations_test.py
+++ b/gm/rebaseline_server/compare_to_expectations_test.py
@@ -19,11 +19,11 @@ within self._output_dir_expected, which wouldn't be good...
 """
 
 import os
-import sys
 
 # Imports from within Skia
 import base_unittest
 import compare_to_expectations
+import imagediffdb
 import results
 import gm_json  # must import results first, so that gm_json will be in sys.path
 
@@ -32,10 +32,11 @@ class CompareToExpectationsTest(base_unittest.TestCase):
 
   def test_gm(self):
     """Process results of a GM run with the ExpectationComparisons object."""
+    image_diff_db = imagediffdb.ImageDiffDB(storage_root=self._temp_dir)
     results_obj = compare_to_expectations.ExpectationComparisons(
+        image_diff_db=image_diff_db,
         actuals_root=os.path.join(self._input_dir, 'gm-actuals'),
         expected_root=os.path.join(self._input_dir, 'gm-expectations'),
-        generated_images_root=self._temp_dir,
         diff_base_url='/static/generated-images')
     results_obj.get_timestamp = mock_get_timestamp
     gm_json.WriteToFile(

diff --git a/gm/rebaseline_server/imagediffdb.py b/gm/rebaseline_server/imagediffdb.py
index 985017a..f6071f9 100644 (file)
--- a/gm/rebaseline_server/imagediffdb.py
+++ b/gm/rebaseline_server/imagediffdb.py
@@ -175,6 +175,8 @@ class DiffRecord(object):
     finally:
       shutil.rmtree(skpdiff_output_dir)
 
+  # TODO(epoger): Use properties instead of getters throughout.
+  # See http://stackoverflow.com/a/6618176
   def get_num_pixels_differing(self):
     """Returns the absolute number of pixels that differ."""
     return self._num_pixels_differing
@@ -221,6 +223,10 @@ class ImageDiffDB(object):
     # actual_image_locator) tuples.
     self._diff_dict = {}
 
+  @property
+  def storage_root(self):
+    return self._storage_root
+
   def add_image_pair(self,
                      expected_image_url, expected_image_locator,
                      actual_image_url, actual_image_locator):

diff --git a/gm/rebaseline_server/server.py b/gm/rebaseline_server/server.py
index ee4d299..e82ecf2 100755 (executable)
--- a/gm/rebaseline_server/server.py
+++ b/gm/rebaseline_server/server.py
@@ -44,6 +44,7 @@ import gm_json
 import compare_configs
 import compare_to_expectations
 import download_actuals
+import imagediffdb
 import imagepairset
 import results as results_mod
 
@@ -233,8 +234,10 @@ class Server(object):
     # 1. self._results
     # 2. the expected or actual results on local disk
     self.results_rlock = threading.RLock()
-    # self._results will be filled in by calls to update_results()
+
+    # These will be filled in by calls to update_results()
     self._results = None
+    self._image_diff_db = None
 
   @property
   def results(self):
@@ -341,11 +344,15 @@ class Server(object):
             compare_to_expectations.DEFAULT_EXPECTATIONS_DIR)
         _run_command(['gclient', 'sync'], TRUNK_DIRECTORY)
 
+      if not self._image_diff_db:
+        self._image_diff_db = imagediffdb.ImageDiffDB(
+            storage_root=os.path.join(
+              PARENT_DIRECTORY, STATIC_CONTENTS_SUBDIR,
+              GENERATED_IMAGES_SUBDIR))
+
       self._results = compare_to_expectations.ExpectationComparisons(
+          image_diff_db=self._image_diff_db,
           actuals_root=self._actuals_dir,
-          generated_images_root=os.path.join(
-              PARENT_DIRECTORY, STATIC_CONTENTS_SUBDIR,
-              GENERATED_IMAGES_SUBDIR),
           diff_base_url=posixpath.join(
               os.pardir, STATIC_CONTENTS_SUBDIR, GENERATED_IMAGES_SUBDIR),
           builder_regex_list=self._builder_regex_list)