Changeset 133328 in webkit

Timestamp:

Nov 2, 2012, 12:02:30 PM (13 years ago)

Author:

dpranke@chromium.org

Message:

webkit-patch analyze-baselines output is weak
​https://bugs.webkit.org/show_bug.cgi?id=100998

Reviewed by Ojan Vafai.

Currently analyze-baselines prints a list of baselines that have
the same checksum per line, but the format is hard to read;
this patch cleans up the output to print by directory instead
and use the same format I recently added for debugging optimize-baselines,
then refactors the code so that we share and adds tests for
analyze-baselines (which was untested).

Also, I got rid of a couple of unnecessarily-hardcoded port names,
and modified the baseline optimizer to log the current world when
optimize fails.

• Scripts/webkitpy/common/checkout/baselineoptimizer.py:

(BaselineOptimizer.read_results_by_directory):
(BaselineOptimizer.write_by_directory):
(BaselineOptimizer.optimize):

• Scripts/webkitpy/tool/commands/rebaseline.py:

(OptimizeBaselines.execute):
(AnalyzeBaselines.init):
(AnalyzeBaselines._write):
(AnalyzeBaselines._analyze_baseline):
(AnalyzeBaselines.execute):

• Scripts/webkitpy/tool/commands/rebaseline_unittest.py:

(_FakeOptimizer):
(_FakeOptimizer.read_results_by_directory):
(TestAnalyzeBaselines):
(TestAnalyzeBaselines.setUp):
(TestAnalyzeBaselines.test_default):
(TestAnalyzeBaselines.test_missing_baselines):

Location:

trunk/Tools

Files:

• ChangeLog (modified) (1 diff)
• Scripts/webkitpy/common/checkout/baselineoptimizer.py (modified) (5 diffs)
• Scripts/webkitpy/common/checkout/baselineoptimizer_unittest.py (modified) (1 diff)
• Scripts/webkitpy/tool/commands/rebaseline.py (modified) (2 diffs)
• Scripts/webkitpy/tool/commands/rebaseline_unittest.py (modified) (2 diffs)

trunk/Tools/ChangeLog

@@ +1 @@
+2012-11-02  Dirk Pranke  <dpranke@chromium.org>
+
+        webkit-patch analyze-baselines output is weak
+        https://bugs.webkit.org/show_bug.cgi?id=100998
+
+        Reviewed by Ojan Vafai.
+
+        Currently analyze-baselines prints a list of baselines that have
+        the same checksum per line, but the format is hard to read;
+        this patch cleans up the output to print by directory instead
+        and use the same format I recently added for debugging optimize-baselines,
+        then refactors the code so that we share and adds tests for
+        analyze-baselines (which was untested).
+
+        Also, I got rid of a couple of unnecessarily-hardcoded port names,
+        and modified the baseline optimizer to log the current world when
+        optimize fails.
+
+        * Scripts/webkitpy/common/checkout/baselineoptimizer.py:
+        (BaselineOptimizer.read_results_by_directory):
+        (BaselineOptimizer.write_by_directory):
+        (BaselineOptimizer.optimize):
+        * Scripts/webkitpy/tool/commands/rebaseline.py:
+        (OptimizeBaselines.execute):
+        (AnalyzeBaselines.__init__):
+        (AnalyzeBaselines._write):
+        (AnalyzeBaselines._analyze_baseline):
+        (AnalyzeBaselines.execute):
+        * Scripts/webkitpy/tool/commands/rebaseline_unittest.py:
+        (_FakeOptimizer):
+        (_FakeOptimizer.read_results_by_directory):
+        (TestAnalyzeBaselines):
+        (TestAnalyzeBaselines.setUp):
+        (TestAnalyzeBaselines.test_default):
+        (TestAnalyzeBaselines.test_missing_baselines):
+
 2012-11-02  Adam Barth  <abarth@webkit.org>
 

trunk/Tools/Scripts/webkitpy/common/checkout/baselineoptimizer.py

@@ -82 +82 @@
         self._directories = reduce(set.union, map(set, self._hypergraph.values()))
 
-    def _read_results_by_directory(self, baseline_name):
+    def read_results_by_directory(self, baseline_name):
         results_by_directory = {}
         for directory in self._directories:
@@ -123 +123 @@
 
     def _find_optimal_result_placement(self, baseline_name):
-        results_by_directory = self._read_results_by_directory(baseline_name)
+        results_by_directory = self.read_results_by_directory(baseline_name)
         results_by_port_name = self._results_by_port_name(results_by_directory)
         port_names_by_result = _invert_dictionary(results_by_port_name)
@@ -182 +182 @@
         except KeyError as e:
             # FIXME: KeyErrors get raised if we're missing baselines. We should handle this better.
-            return results_by_directory
+            return {}
 
     def _find_in_fallbackpath(self, fallback_path, current_result, results_by_directory):
@@ -241 +241 @@
 
     def directories_by_result(self, baseline_name):
-        results_by_directory = self._read_results_by_directory(baseline_name)
+        results_by_directory = self.read_results_by_directory(baseline_name)
         return _invert_dictionary(results_by_directory)
+
+    def write_by_directory(self, results_by_directory, writer, indent):
+        for path in sorted(results_by_directory):
+            writer("%s%s: %s" % (indent, self._platform(path), results_by_directory[path][0:6]))
 
     def optimize(self, baseline_name):
@@ -251 +255 @@
             if new_results_by_directory:
                 _log.debug("  %s: (already optimal)" % basename)
-                for path in sorted(results_by_directory):
-                    result = results_by_directory[path]
-                    _log.debug("      %s: %s" % (self._platform(path), result[0:6]))
+                self.write_by_directory(results_by_directory, _log.debug, "    ")
             else:
                 _log.debug("  %s: (no baselines found)" % basename)
             return True
         if self._filtered_results_by_port_name(results_by_directory) != self._filtered_results_by_port_name(new_results_by_directory):
-            _log.warning("Optimization failed")
+            _log.warning("  %s: optimization failed" % basename)
+            self.write_by_directory(results_by_directory, _log.warning, "      ")
             return False
 
         _log.debug("  %s:" % basename)
         _log.debug("    Before: ")
-        for path in sorted(results_by_directory):
-            result = results_by_directory[path]
-            _log.debug("      %s: %s" % (self._platform(path), result[0:6]))
+        self.write_by_directory(results_by_directory, _log.debug, "      ")
         _log.debug("    After: ")
-        for path in sorted(new_results_by_directory):
-            result = new_results_by_directory[path]
-            _log.debug("      %s: %s" % (self._platform(path), result[0:6]))
+        self.write_by_directory(new_results_by_directory, _log.debug, "      ")
 
         self._move_baselines(baseline_name, results_by_directory, new_results_by_directory)

trunk/Tools/Scripts/webkitpy/common/checkout/baselineoptimizer_unittest.py

@@ -43 +43 @@
     # We override this method for testing so we don't have to construct an
     # elaborate mock file system.
-    def _read_results_by_directory(self, baseline_name):
+    def read_results_by_directory(self, baseline_name):
         return self._mock_results_by_directory
 

trunk/Tools/Scripts/webkitpy/tool/commands/rebaseline.py

@@ -209 +209 @@
         self._baseline_suffix_list = options.suffixes.split(',')
         self._baseline_optimizer = BaselineOptimizer(tool)
-        self._port = tool.port_factory.get("chromium-win-win7")  # FIXME: This should be selectable.
-
+        self._port = tool.port_factory.get()
         for test_name in self._port.tests(args):
             _log.info("Optimizing %s" % test_name)
@@ -222 +221 @@
 
     def __init__(self):
-        return super(AnalyzeBaselines, self).__init__(options=[self.suffixes_option])
-
-    def _print(self, baseline_name, directories_by_result):
-        for result, directories in directories_by_result.items():
-            if len(directories) <= 1:
-                continue
-            results_names = [self._tool.filesystem.join(directory, baseline_name) for directory in directories]
-            print ' '.join(results_names)
-
-    def _analyze_baseline(self, test_name):
+        super(AnalyzeBaselines, self).__init__(options=[
+            self.suffixes_option,
+            optparse.make_option('--missing', action='store_true', default=False, help='show missing baselines as well'),
+            ])
+        self._optimizer_class = BaselineOptimizer  # overridable for testing
+
+    def _write(self, msg):
+        print msg
+
+    def _analyze_baseline(self, options, test_name):
         for suffix in self._baseline_suffix_list:
             baseline_name = _baseline_name(self._tool.filesystem, test_name, suffix)
-            directories_by_result = self._baseline_optimizer.directories_by_result(baseline_name)
-            self._print(baseline_name, directories_by_result)
+            results_by_directory = self._baseline_optimizer.read_results_by_directory(baseline_name)
+            if results_by_directory:
+                self._write("%s:" % baseline_name)
+                self._baseline_optimizer.write_by_directory(results_by_directory, self._write, "  ")
+            elif options.missing:
+                self._write("%s: (no baselines found)" % baseline_name)
 
     def execute(self, options, args, tool):
         self._baseline_suffix_list = options.suffixes.split(',')
-        self._baseline_optimizer = BaselineOptimizer(tool)
-        self._port = tool.port_factory.get("chromium-win-win7")  # FIXME: This should be selectable.
-
+        self._baseline_optimizer = self._optimizer_class(tool)
+        self._port = tool.port_factory.get()
         for test_name in self._port.tests(args):
-            self._analyze_baseline(test_name)
+            self._analyze_baseline(options, test_name)
 
 

trunk/Tools/Scripts/webkitpy/tool/commands/rebaseline_unittest.py

@@ -30 +30 @@
 
 from webkitpy.common.system.outputcapture import OutputCapture
+from webkitpy.common.checkout.baselineoptimizer import BaselineOptimizer
+from webkitpy.common.net.buildbot.buildbot_mock import MockBuilder
+from webkitpy.common.system.executive_mock import MockExecutive2
 from webkitpy.thirdparty.mock import Mock
 from webkitpy.tool.commands.rebaseline import *
 from webkitpy.tool.mocktool import MockTool, MockOptions
-from webkitpy.common.net.buildbot.buildbot_mock import MockBuilder
-from webkitpy.common.system.executive_mock import MockExecutive2
 
 
@@ -361 +362 @@
 
 
+class _FakeOptimizer(BaselineOptimizer):
+    def read_results_by_directory(self, baseline_name):
+        if baseline_name.endswith('txt'):
+            return {'LayoutTests/passes/text.html': '123456',
+                    'LayoutTests/platform/test-mac-leopard/passes/text.html': 'abcdef'}
+        return {}
+
+
+class TestAnalyzeBaselines(_BaseTestCase):
+    command_constructor = AnalyzeBaselines
+
+    def setUp(self):
+        super(TestAnalyzeBaselines, self).setUp()
+        self.port = self.tool.port_factory.get('test')
+        self.tool.port_factory.get = lambda port_name=None, options=None: self.port
+        self.lines = []
+        self.command._optimizer_class = _FakeOptimizer
+        self.command._write = lambda msg: self.lines.append(msg)
+
+    def test_default(self):
+        self.command.execute(MockOptions(suffixes='txt', missing=False), ['passes/text.html'], self.tool)
+        self.assertEquals(self.lines,
+            ['passes/text-expected.txt:',
+             '  (generic): 123456',
+             '  test-mac-leopard: abcdef'])
+
+    def test_missing_baselines(self):
+        self.command.execute(MockOptions(suffixes='png,txt', missing=True), ['passes/text.html'], self.tool)
+        self.assertEquals(self.lines,
+            ['passes/text-expected.png: (no baselines found)',
+             'passes/text-expected.txt:',
+             '  (generic): 123456',
+             '  test-mac-leopard: abcdef'])