rebaseline_server: add "prefetch" directive that just warms the cache without awaiting results

rebaseline_server: add "prefetch" directive that just warms the cache without awaiting results

This will allow the buildbots to warn the production rebaseline_server: "I just
generated some new results; here's a comparison that a human might ask for
soon. Download whatever images and generate whatever diffs you would need to
provide those results."

BUG=skia:1942
NOTREECHECKS=true
NOTRY=true

Committed: https://skia.googlesource.com/skia/+/3facc7c87d9f81c352c9d37b1b46340b9e745578

[epoger, 2014-08-06 02:39:46]

Ravi (and other interested parties)- PTAL at patchset 1.

With this CL, I can query the server with a URL like this:
http://172.24.179.183:8888/live-results/returnQuickly=true&setADir=gs%3a%2f%2...

... and it will return in <1 second; afterwards, worker threads will scurry
around downloading images and generating diffs.


(set NOTRY=true since this CL contains nothing that our commit-queue trybots
would catch anyway)

[epoger, 2014-08-06 14:02:35]

More thoughts since I wrote this CL:

Caching the results of setA-vs-setB is a start, but we'll need to do more.  When
generating diffs for such a comparison, we don't bother downloading any images
unless there are diffs to generate that need them.  (For any image whose actual
checksum matches its expected checksum, or which HAS NO expectation, we skip
downloading the actual image.)

Some ways I have thought of to do this:

A. Add a separate directive: "Here's a set of actual result summaries; download
all the images referred to within that set. You don't need to generate any diffs
from them."

B. Add a --download-all-images command-line flag to server.py: "Download all
images you learn about, whether you need them to generate diffs or not."  Pro:
simple.  Con: if the cache isn't mostly preloaded already, user requests will
take too long.

C. Add an additional parameter to the existing directive, telling the server
whether it should download all images (warms the cache, but takes a lot longer)
or only download the images it needs (responds faster).


I'm leaning towards C.  Let's discuss...

[epoger, 2014-08-06 14:28:34]

On 2014/08/06 14:02:35, epoger wrote:
> More thoughts since I wrote this CL:

Notes from live discussion with Ravi:

I am going to adjust this CL:

1. Create "prefetch" directive that gets called instead of
"live-results/returnQuickly=true".  The prefetch directive means: "Somebody will
probably send you a 'live-results' directive soon, with the following setA and
setB.  Be ready to serve that request."

2. The "prefetch" directive will take an optional
"downloadOnlyDifferingImages=true" parameter.  If set, the server will only
download images that it actually needs to generate diffs.  By default, the
server will download ALL images referred to in setA and setB (unless they have
already been downloaded to local disk).  I do not expect we will ever specify
the "downloadOnlyDifferingImages" parameter, but we'll have it just in case.

[epoger, 2014-08-06 14:29:13]

On 2014/08/06 14:28:34, epoger wrote:
> I am going to adjust this CL:

Oh, and here's a sample of what a full prefetch URL would look like:

http://172.24.179.183:8888/prefetch/downloadOnlyDifferingImages=true&setADir=...

[epoger, 2014-08-06 17:35:54]

Ravi- please see patchsets 2+3.

[rmistry, 2014-08-06 17:49:01]

LGTM

https://codereview.chromium.org/443013002/diff/40001/gm/rebaseline_server/ima...
File gm/rebaseline_server/imagediffdb.py (right):

https://codereview.chromium.org/443013002/diff/40001/gm/rebaseline_server/ima...
gm/rebaseline_server/imagediffdb.py:447: return locator
Is it ok for this to be an empty string or None? should this throw an exception
instead?

[epoger, 2014-08-06 17:53:34]

Thanks.  Committing now.

https://codereview.chromium.org/443013002/diff/40001/gm/rebaseline_server/ima...
File gm/rebaseline_server/imagediffdb.py (right):

https://codereview.chromium.org/443013002/diff/40001/gm/rebaseline_server/ima...
gm/rebaseline_server/imagediffdb.py:447: return locator
On 2014/08/06 17:49:01, rmistry wrote:
> Is it ok for this to be an empty string or None? should this throw an
exception
> instead?

We actually need to allow for None.  Added a comment explaining why.

[epoger, 2014-08-06 17:53:38]

The CQ bit was checked by epoger@google.com

[commit-bot: I haz the power, 2014-08-06 17:54:42]

CQ is trying da patch. Follow status at
 https://skia-tree-status.appspot.com/cq/epoger@google.com/443013002/60001

[commit-bot: I haz the power, 2014-08-06 17:54:46]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-08-06 17:54:47]

Failed to apply patch for gm/rebaseline_server/compare_rendered_pictures.py:
While running git apply --index -p1;
  error: patch failed: gm/rebaseline_server/compare_rendered_pictures.py:64
  error: gm/rebaseline_server/compare_rendered_pictures.py: patch does not apply

Patch:       gm/rebaseline_server/compare_rendered_pictures.py
Index: gm/rebaseline_server/compare_rendered_pictures.py
diff --git a/gm/rebaseline_server/compare_rendered_pictures.py
b/gm/rebaseline_server/compare_rendered_pictures.py
index
907ea631366e94604321c406a0def7db521320d7..2455d4f6796d6239fb65a88fe3e36e3987dfba1d
100755
--- a/gm/rebaseline_server/compare_rendered_pictures.py
+++ b/gm/rebaseline_server/compare_rendered_pictures.py
@@ -64,8 +64,15 @@ class RenderedPicturesComparisons(results.BaseComparisons):
                image_base_gs_url=DEFAULT_IMAGE_BASE_GS_URL,
                diff_base_url=None, setA_label='setA',
                setB_label='setB', gs=None,
-               truncate_results=False):
-    """
+               truncate_results=False, prefetch_only=False,
+               download_all_images=False):
+    """Constructor: downloads images and generates diffs.
+
+    Once the object has been created (which may take a while), you can call its
+    get_packaged_results_of_type() method to quickly retrieve the results...
+    unless you have set prefetch_only to True, in which case we will
+    asynchronously warm up the ImageDiffDB cache but not fill in self._results.
+
     Args:
       setA_dirs: list of root directories to copy all JSON summaries from,
           and to use as setA within the comparisons
@@ -85,6 +92,14 @@ class RenderedPicturesComparisons(results.BaseComparisons):
       gs: instance of GSUtils object we can use to download summary files
       truncate_results: FOR MANUAL TESTING: if True, truncate the set of images
           we process, to speed up testing.
+      prefetch_only: if True, return the new object as quickly as possible
+          with empty self._results (just queue up all the files to process,
+          don't wait around for them to be processed and recorded); otherwise,
+          block until the results have been assembled and recorded in
+          self._results.
+      download_all_images: if True, download all images, even if we don't
+          need them to generate diffs.  This will take much longer to complete,
+          but is useful for warming up the bitmap cache on local disk.
     """
     super(RenderedPicturesComparisons, self).__init__()
     self._image_diff_db = image_diff_db
@@ -96,6 +111,8 @@ class RenderedPicturesComparisons(results.BaseComparisons):
     self._setB_label = setB_label
     self._gs = gs
     self.truncate_results = truncate_results
+    self._prefetch_only = prefetch_only
+    self._download_all_images = download_all_images
 
     tempdir = tempfile.mkdtemp()
     try:
@@ -113,11 +130,12 @@ class
RenderedPicturesComparisons(results.BaseComparisons):
       self._results = self._load_result_pairs(
           setA_root=setA_root, setA_section=gm_json.JSONKEY_ACTUALRESULTS,
           setB_root=setB_root, setB_section=gm_json.JSONKEY_ACTUALRESULTS)
-      self._timestamp = int(time.time())
-      logging.info('Number of download file collisions: %s' %
-                   imagediffdb.global_file_collisions)
-      logging.info('Results complete; took %d seconds.' %
-                   (self._timestamp - time_start))
+      if self._results:
+        self._timestamp = int(time.time())
+        logging.info('Number of download file collisions: %s' %
+                     imagediffdb.global_file_collisions)
+        logging.info('Results complete; took %d seconds.' %
+                     (self._timestamp - time_start))
     finally:
       shutil.rmtree(tempdir)
 
@@ -133,7 +151,8 @@ class RenderedPicturesComparisons(results.BaseComparisons):
       setB_section: which section (gm_json.JSONKEY_ACTUALRESULTS or
           gm_json.JSONKEY_EXPECTEDRESULTS) to load from the summaries in setB
 
-    Returns the summary of all image diff results.
+    Returns the summary of all image diff results (or None, depending on
+    self._prefetch_only).
     """
     logging.info('Reading JSON image summaries from dirs %s and %s...' % (
         setA_root, setB_root))
@@ -174,8 +193,9 @@ class RenderedPicturesComparisons(results.BaseComparisons):
     dict_num = 0
     for dict_path in union_dict_paths:
       dict_num += 1
-      logging.info('Generating pixel diffs for dict #%d of %d, "%s"...' %
-                   (dict_num, num_union_dict_paths, dict_path))
+      logging.info(
+          'Asynchronously requesting pixel diffs for dict #%d of %d, "%s"...' %
+          (dict_num, num_union_dict_paths, dict_path))
 
       dictA = self.get_default(setA_dicts, None, dict_path)
       self._validate_dict_version(dictA)
@@ -227,12 +247,15 @@ class
RenderedPicturesComparisons(results.BaseComparisons):
             if result_type != results.KEY__RESULT_TYPE__SUCCEEDED:
               failing_image_pairs.add_image_pair(one_imagepair)
 
-    return {
-      results.KEY__HEADER__RESULTS_ALL: all_image_pairs.as_dict(
-          column_ids_in_order=ORDERED_COLUMN_IDS),
-      results.KEY__HEADER__RESULTS_FAILURES: failing_image_pairs.as_dict(
-          column_ids_in_order=ORDERED_COLUMN_IDS),
-    }
+    if self._prefetch_only:
+      return None
+    else:
+      return {
+          results.KEY__HEADER__RESULTS_ALL: all_image_pairs.as_dict(
+              column_ids_in_order=ORDERED_COLUMN_IDS),
+          results.KEY__HEADER__RESULTS_FAILURES: failing_image_pairs.as_dict(
+              column_ids_in_order=ORDERED_COLUMN_IDS),
+      }
 
   def _validate_dict_version(self, result_dict):
     """Raises Exception if the dict is not the type/version we know how to
read.
@@ -312,7 +335,8 @@ class RenderedPicturesComparisons(results.BaseComparisons):
           base_url=self._image_base_gs_url,
           imageA_relative_url=imageA_relative_url,
           imageB_relative_url=imageB_relative_url,
-          extra_columns=extra_columns_dict)
+          extra_columns=extra_columns_dict,
+          download_all_images=self._download_all_images)
     except (KeyError, TypeError):
       logging.exception(
           'got exception while creating ImagePair for'

[epoger, 2014-08-06 17:56:35]

The CQ bit was checked by epoger@google.com

[commit-bot: I haz the power, 2014-08-06 17:56:42]

CQ is trying da patch. Follow status at
 https://skia-tree-status.appspot.com/cq/epoger@google.com/443013002/80001

[commit-bot: I haz the power, 2014-08-06 17:56:55]

Change committed as 3facc7c87d9f81c352c9d37b1b46340b9e745578