Optimize baselines in parallel.

This is by far the slowest part of doing rebaselines.
We can do everything but the SCM commands in parallel.
Like we do for the other commands, save off the SCM commands
and do them all at the end.

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

git-svn-id: svn://svn.chromium.org/blink/trunk@175191 bbb929c8-8fbe-4397-9dbb-9b2b20218538

third_party/WebKit/Tools/Scripts/webkitpy/common/checkout/baselineoptimizer.py

@@ -48,9 +48,12 @@ def _invert_dictionary(dictionary):
 class BaselineOptimizer(object):
     ROOT_LAYOUT_TESTS_DIRECTORY = 'LayoutTests'
 
-    def __init__(self, host, port_names):
+    def __init__(self, host, port_names, skip_scm_commands):
         self._filesystem = host.filesystem
         self._port_factory = host.port_factory
+        self._skip_scm_commands = skip_scm_commands
+        self._files_to_delete = []
+        self._files_to_add = []
         self._scm = host.scm()
         self._port_names = port_names
         # Only used by unittests.
@@ -224,7 +227,10 @@ class BaselineOptimizer(object):
                 _log.debug("    Deleting (SCM):")
                 for platform_dir in sorted(self._platform(filename) for filename in scm_files):
                     _log.debug("      " + platform_dir)
-                self._scm.delete_list(scm_files)
+                if self._skip_scm_commands:
+                    self._files_to_delete.extend(scm_files)
+                else:
+                    self._scm.delete_list(scm_files)
             if fs_files:
                 _log.debug("    Deleting (file system):")
                 for platform_dir in sorted(self._platform(filename) for filename in fs_files):
@@ -246,7 +252,12 @@ class BaselineOptimizer(object):
             _log.debug("    Adding:")
             for platform_dir in sorted(self._platform(filename) for filename in file_names):
                 _log.debug("      " + platform_dir)
-            self._scm.add_list(file_names)
+            if self._skip_scm_commands:
+                # Have adds win over deletes.
+                self._files_to_delete = list(set(self._files_to_delete) - set(file_names))
+                self._files_to_add.extend(file_names)
+            else:
+                self._scm.add_list(file_names)
         else:
             _log.debug("    (Nothing to add)")
 
@@ -303,7 +314,10 @@ class BaselineOptimizer(object):
                 break
 
         _log.debug("Deleting redundant virtual root expected result.")
-        self._scm.delete(virtual_root_expected_baseline_path)
+        if self._skip_scm_commands:
+            self._files_to_delete.append(virtual_root_expected_baseline_path)
+        else:
+            self._scm.delete(virtual_root_expected_baseline_path)
 
     def optimize(self, baseline_name):
         # The virtual fallback path is the same as the non-virtual one tacked on to the bottom of the non-virtual path.
@@ -316,10 +330,10 @@ class BaselineOptimizer(object):
         result = self._optimize_subtree(baseline_name)
         non_virtual_baseline_name = self._port_factory.get().lookup_virtual_test_base(baseline_name)
         if not non_virtual_baseline_name:
-            return result
+            return result, self._files_to_delete, self._files_to_add
 
         self._optimize_virtual_root(baseline_name, non_virtual_baseline_name)
 
         _log.debug("Optimizing non-virtual fallback path.")
         result |= self._optimize_subtree(non_virtual_baseline_name)
-        return result
+        return result, self._files_to_delete, self._files_to_add

third_party/WebKit/Tools/Scripts/webkitpy/common/checkout/baselineoptimizer_unittest.py

@@ -65,7 +65,7 @@ class BaselineOptimizerTest(unittest.TestCase):
         host.filesystem.write_binary_file('/mock-checkout/third_party/WebKit/LayoutTests/platform/win/another/test-expected.txt', 'result A')
         host.filesystem.write_binary_file('/mock-checkout/third_party/WebKit/LayoutTests/platform/mac/another/test-expected.txt', 'result A')
         host.filesystem.write_binary_file('/mock-checkout/third_party/WebKit/LayoutTests/another/test-expected.txt', 'result B')
-        baseline_optimizer = BaselineOptimizer(host, host.port_factory.all_port_names())
+        baseline_optimizer = BaselineOptimizer(host, host.port_factory.all_port_names(), skip_scm_commands=False)
         baseline_optimizer._move_baselines('another/test-expected.txt', {
             '/mock-checkout/third_party/WebKit/LayoutTests/platform/win': 'aaa',
             '/mock-checkout/third_party/WebKit/LayoutTests/platform/mac': 'aaa',
@@ -75,7 +75,32 @@ class BaselineOptimizerTest(unittest.TestCase):
         })
         self.assertEqual(host.filesystem.read_binary_file('/mock-checkout/third_party/WebKit/LayoutTests/another/test-expected.txt'), 'result A')
 
-    def _assertOptimization(self, results_by_directory, expected_new_results_by_directory, baseline_dirname=''):
+    def test_move_baselines_skip_scm_commands(self):
+        host = MockHost(scm=ExcludingMockSCM(['/mock-checkout/third_party/WebKit/LayoutTests/platform/mac/another/test-expected.txt']))
+        host.filesystem.write_binary_file('/mock-checkout/third_party/WebKit/LayoutTests/platform/win/another/test-expected.txt', 'result A')
+        host.filesystem.write_binary_file('/mock-checkout/third_party/WebKit/LayoutTests/platform/mac/another/test-expected.txt', 'result A')
+        host.filesystem.write_binary_file('/mock-checkout/third_party/WebKit/LayoutTests/another/test-expected.txt', 'result B')
+        baseline_optimizer = BaselineOptimizer(host, host.port_factory.all_port_names(), skip_scm_commands=True)
+        baseline_optimizer._move_baselines('another/test-expected.txt', {
+            '/mock-checkout/third_party/WebKit/LayoutTests/platform/win': 'aaa',
+            '/mock-checkout/third_party/WebKit/LayoutTests/platform/mac': 'aaa',
+            '/mock-checkout/third_party/WebKit/LayoutTests': 'bbb',
+        }, {
+            '/mock-checkout/third_party/WebKit/LayoutTests/platform/linux': 'bbb',
+            '/mock-checkout/third_party/WebKit/LayoutTests': 'aaa',
+        })
+        self.assertEqual(host.filesystem.read_binary_file('/mock-checkout/third_party/WebKit/LayoutTests/another/test-expected.txt'), 'result A')
+
+        self.assertEqual(baseline_optimizer._files_to_delete, [
+            '/mock-checkout/third_party/WebKit/LayoutTests/platform/win/another/test-expected.txt',
+        ])
+
+        self.assertEqual(baseline_optimizer._files_to_add, [
+            '/mock-checkout/third_party/WebKit/LayoutTests/another/test-expected.txt',
+            '/mock-checkout/third_party/WebKit/LayoutTests/platform/linux/another/test-expected.txt',
+        ])
+
+    def _assertOptimization(self, results_by_directory, expected_new_results_by_directory, baseline_dirname='', expected_files_to_delete=None):
         host = MockHost()
         fs = host.filesystem
         webkit_base = WebKitFinder(fs).webkit_base()
@@ -85,13 +110,13 @@ class BaselineOptimizerTest(unittest.TestCase):
             path = fs.join(webkit_base, 'LayoutTests', dirname, baseline_name)
             fs.write_binary_file(path, contents)
 
-        baseline_optimizer = BaselineOptimizer(host, host.port_factory.all_port_names())
+        baseline_optimizer = BaselineOptimizer(host, host.port_factory.all_port_names(), skip_scm_commands=expected_files_to_delete is not None)
         self.assertTrue(baseline_optimizer.optimize(fs.join(baseline_dirname, baseline_name)))
 
         for dirname, contents in expected_new_results_by_directory.items():
             path = fs.join(webkit_base, 'LayoutTests', dirname, baseline_name)
             if contents is None:
-                self.assertFalse(fs.exists(path))
+                self.assertTrue(not fs.exists(path) or path in baseline_optimizer._files_to_delete)
             else:
                 self.assertEqual(fs.read_binary_file(path), contents)
 
@@ -99,7 +124,10 @@ class BaselineOptimizerTest(unittest.TestCase):
         for dirname in results_by_directory:
             path = fs.join(webkit_base, 'LayoutTests', dirname, baseline_name)
             if not dirname in expected_new_results_by_directory:
-                self.assertFalse(fs.exists(path))
+                self.assertTrue(not fs.exists(path) or path in baseline_optimizer._files_to_delete)
+
+        if expected_files_to_delete:
+            self.assertEqual(baseline_optimizer._files_to_delete, expected_files_to_delete)
 
     def test_linux_redundant_with_win(self):
         self._assertOptimization({
@@ -209,6 +237,22 @@ class BaselineOptimizerTest(unittest.TestCase):
             'compositing': '2',
         }, baseline_dirname='virtual/softwarecompositing')
 
+    def test_virtual_root_redundant_with_ancestors_skip_scm_commands(self):
+        self._assertOptimization({
+            'virtual/softwarecompositing': '2',
+            'platform/mac/compositing': '2',
+            'platform/win/compositing': '2',
+        }, {
+            'virtual/softwarecompositing': None,
+            'compositing': '2',
+        },
+        baseline_dirname='virtual/softwarecompositing',
+        expected_files_to_delete=[
+            '/mock-checkout/third_party/WebKit/LayoutTests/virtual/softwarecompositing/mock-baseline-expected.txt',
+            '/mock-checkout/third_party/WebKit/LayoutTests/platform/mac/compositing/mock-baseline-expected.txt',
+            '/mock-checkout/third_party/WebKit/LayoutTests/platform/win/compositing/mock-baseline-expected.txt',
+        ])
+
     def test_virtual_root_not_redundant_with_ancestors(self):
         self._assertOptimization({
             'virtual/softwarecompositing': '2',

third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline.py

@@ -72,6 +72,13 @@ class AbstractRebaseliningCommand(AbstractDeclarativeCommand):
     def __init__(self, options=None):
         super(AbstractRebaseliningCommand, self).__init__(options=options)
         self._baseline_suffix_list = BASELINE_SUFFIX_LIST
+        self._scm_changes = {'add': [], 'delete': [], 'remove-lines': []}
+
+    def _add_to_scm_later(self, path):
+        self._scm_changes['add'].append(path)
+
+    def _delete_from_scm_later(self, path):
+        self._scm_changes['delete'].append(path)
 
 
 class BaseInternalRebaselineCommand(AbstractRebaseliningCommand):
@@ -82,10 +89,6 @@ class BaseInternalRebaselineCommand(AbstractRebaseliningCommand):
             optparse.make_option("--builder", help="Builder to pull new baselines from"),
             optparse.make_option("--test", help="Test to rebaseline"),
             ])
-        self._scm_changes = {'add': [], 'remove-lines': []}
-
-    def _add_to_scm(self, path):
-        self._scm_changes['add'].append(path)
 
     def _baseline_directory(self, builder_name):
         port = self._tool.port_factory.get_from_builder_name(builder_name)
@@ -168,7 +171,7 @@ class CopyExistingBaselinesInternal(BaseInternalRebaselineCommand):
             self._tool.filesystem.maybe_make_directory(self._tool.filesystem.dirname(new_baseline))
             self._tool.filesystem.copyfile(old_baseline, new_baseline)
             if not self._tool.scm().exists(new_baseline):
-                self._add_to_scm(new_baseline)
+                self._add_to_scm_later(new_baseline)
 
     def execute(self, options, args, tool):
         for suffix in options.suffixes.split(','):
@@ -192,7 +195,7 @@ class RebaselineTest(BaseInternalRebaselineCommand):
         filesystem.maybe_make_directory(filesystem.dirname(target_baseline))
         filesystem.write_binary_file(target_baseline, data)
         if not self._tool.scm().exists(target_baseline):
-            self._add_to_scm(target_baseline)
+            self._add_to_scm_later(target_baseline)
 
     def _rebaseline_test(self, builder_name, test_name, suffix, results_url):
         baseline_directory = self._baseline_directory(builder_name)
@@ -231,13 +234,18 @@ class OptimizeBaselines(AbstractRebaseliningCommand):
     argument_names = "TEST_NAMES"
 
     def __init__(self):
-        super(OptimizeBaselines, self).__init__(options=[self.suffixes_option] + self.platform_options)
+        super(OptimizeBaselines, self).__init__(options=[
+            self.suffixes_option,
+            optparse.make_option('--no-modify-scm', action='store_true', default=False, help='Dump SCM commands as JSON instead of '),
+            ] + self.platform_options)
 
     def _optimize_baseline(self, optimizer, test_name):
         for suffix in self._baseline_suffix_list:
             baseline_name = _baseline_name(self._tool.filesystem, test_name, suffix)
-            if not optimizer.optimize(baseline_name):
+            succeeded, files_to_delete, files_to_add = optimizer.optimize(baseline_name)
+            if not succeeded:
                 print "Heuristics failed to optimize %s" % baseline_name
+            return files_to_delete, files_to_add
 
     def execute(self, options, args, tool):
         self._baseline_suffix_list = options.suffixes.split(',')
@@ -246,11 +254,17 @@ class OptimizeBaselines(AbstractRebaseliningCommand):
             print "No port names match '%s'" % options.platform
             return
 
-        optimizer = BaselineOptimizer(tool, port_names)
+        optimizer = BaselineOptimizer(tool, port_names, skip_scm_commands=options.no_modify_scm)
         port = tool.port_factory.get(port_names[0])
         for test_name in port.tests(args):
             _log.info("Optimizing %s" % test_name)
-            self._optimize_baseline(optimizer, test_name)
+            files_to_delete, files_to_add = self._optimize_baseline(optimizer, test_name)
+            for path in files_to_delete:
+                self._delete_from_scm_later(path)
+            for path in files_to_add:
+                self._add_to_scm_later(path)
+
+        print json.dumps(self._scm_changes)
 
 
 class AnalyzeBaselines(AbstractRebaseliningCommand):
@@ -288,7 +302,7 @@ class AnalyzeBaselines(AbstractRebaseliningCommand):
             print "No port names match '%s'" % options.platform
             return
 
-        self._baseline_optimizer = self._optimizer_class(tool, port_names)
+        self._baseline_optimizer = self._optimizer_class(tool, port_names, skip_scm_commands=False)
         self._port = tool.port_factory.get(port_names[0])
         for test_name in self._port.tests(args):
             self._analyze_baseline(options, test_name)
@@ -373,8 +387,9 @@ class AbstractParallelRebaselineCommand(AbstractRebaseliningCommand):
                     rebaseline_commands.append(tuple([[path_to_webkit_patch, 'rebaseline-test-internal'] + cmd_line, cwd]))
         return copy_baseline_commands, rebaseline_commands
 
-    def _files_to_add(self, command_results):
+    def _serial_commands(self, command_results):
         files_to_add = set()
+        files_to_delete = set()
         lines_to_remove = {}
         for output in [result[1].split('\n') for result in command_results]:
             file_added = False
@@ -384,6 +399,8 @@ class AbstractParallelRebaselineCommand(AbstractRebaseliningCommand):
                         parsed_line = json.loads(line)
                         if 'add' in parsed_line:
                             files_to_add.update(parsed_line['add'])
+                        if 'delete' in parsed_line:
+                            files_to_delete.update(parsed_line['delete'])
                         if 'remove-lines' in parsed_line:
                             for line_to_remove in parsed_line['remove-lines']:
                                 test = line_to_remove['test']
@@ -398,16 +415,24 @@ class AbstractParallelRebaselineCommand(AbstractRebaseliningCommand):
             if not file_added:
                 _log.debug('Could not add file based off output "%s"' % output)
 
-        return list(files_to_add), lines_to_remove
+        return list(files_to_add), list(files_to_delete), lines_to_remove
 
     def _optimize_baselines(self, test_prefix_list, verbose=False):
-        # We don't run this in parallel because modifying the SCM in parallel is unreliable.
+        optimize_commands = []
         for test in test_prefix_list:
             all_suffixes = set()
             for builder in self._builders_to_fetch_from(test_prefix_list[test]):
                 all_suffixes.update(self._suffixes_for_actual_failures(test, builder, test_prefix_list[test][builder]))
+
             # FIXME: We should propagate the platform options as well.
-            self._run_webkit_patch(['optimize-baselines', '--suffixes', ','.join(all_suffixes), test], verbose)
+            cmd_line = ['--no-modify-scm', '--suffixes', ','.join(all_suffixes), test]
+            if verbose:
+                cmd_line.append('--verbose')
+
+            path_to_webkit_patch = self._tool.path()
+            cwd = self._tool.scm().checkout_root
+            optimize_commands.append(tuple([[path_to_webkit_patch, 'optimize-baselines'] + cmd_line, cwd]))
+        return optimize_commands
 
     def _update_expectations_files(self, lines_to_remove):
         # FIXME: This routine is way too expensive. We're creating N ports and N TestExpectations
@@ -451,9 +476,11 @@ class AbstractParallelRebaselineCommand(AbstractRebaseliningCommand):
             if line:
                 print >> sys.stderr, line  # FIXME: Figure out how to log properly.
 
-        files_to_add, lines_to_remove = self._files_to_add(command_results)
+        files_to_add, files_to_delete, lines_to_remove = self._serial_commands(command_results)
+        if files_to_delete:
+            self._tool.scm().delete_list(files_to_delete)
         if files_to_add:
-            self._tool.scm().add_list(list(files_to_add))
+            self._tool.scm().add_list(files_to_add)
         if lines_to_remove:
             self._update_expectations_files(lines_to_remove)
 
@@ -468,9 +495,8 @@ class AbstractParallelRebaselineCommand(AbstractRebaseliningCommand):
             self._run_in_parallel_and_update_scm(copy_baseline_commands)
         if rebaseline_commands:
             self._run_in_parallel_and_update_scm(rebaseline_commands)
-
         if options.optimize:
-            self._optimize_baselines(test_prefix_list, options.verbose)
+            self._run_in_parallel_and_update_scm(self._optimize_baselines(test_prefix_list, options.verbose))
 
     def _suffixes_for_actual_failures(self, test, builder_name, existing_suffixes):
         actual_results = self.builder_data()[builder_name].actual_results(test)