Merge pull request #20807 from palimondo/and-dreadfully-distinct

[benchmark] Added Benchmark Check Report

Author: palimondo

benchmark/scripts/Benchmark_Driver

@@ -261,6 +261,51 @@ class LoggingReportFormatter(logging.Formatter):
             '{0} {1}{2}'.format(record.levelname, category, msg))
 
 
+class MarkdownReportHandler(logging.StreamHandler):
+    r"""Write custom formatted messages from BenchmarkDoctor to the stream.
+
+    It works around StreamHandler's hardcoded '\n' and handles the custom
+    level and category formatting for BenchmarkDoctor's check report.
+    """
+
+    def __init__(self, stream):
+        """Initialize the handler and write a Markdown table header."""
+        super(MarkdownReportHandler, self).__init__(stream)
+        self.setLevel(logging.INFO)
+        self.stream.write('\n✅  | Benchmark Check Report\n---|---')
+        self.stream.flush()
+
+    levels = {logging.WARNING: '\n⚠️', logging.ERROR: '\n⛔️',
+              logging.INFO: ' <br><sub> '}
+    categories = {'naming': '🔤', 'runtime': '⏱', 'memory': 'Ⓜ️'}
+    quotes_re = re.compile("'")
+
+    def format(self, record):
+        msg = super(MarkdownReportHandler, self).format(record)
+        return (self.levels.get(record.levelno, '') +
+                ('' if record.levelno == logging.INFO else
+                 self.categories.get(record.name.split('.')[-1], '') + ' | ') +
+                self.quotes_re.sub('`', msg))
+
+    def emit(self, record):
+        msg = self.format(record)
+        stream = self.stream
+        try:
+            if (isinstance(msg, unicode) and
+                    getattr(stream, 'encoding', None)):
+                stream.write(msg.encode(stream.encoding))
+            else:
+                stream.write(msg)
+        except UnicodeError:
+            stream.write(msg.encode("UTF-8"))
+        self.flush()
+
+    def close(self):
+        self.stream.write('\n\n')
+        self.stream.flush()
+        super(MarkdownReportHandler, self).close()
+
+
 class BenchmarkDoctor(object):
     """Checks that the benchmark conforms to the standard set of requirements.
 
@@ -302,8 +347,9 @@ class BenchmarkDoctor(object):
         ]
 
     def __del__(self):
-        """Unregister handler on exit."""
-        self.log.removeHandler(self.console_handler)
+        """Close log handlers on exit."""
+        for handler in list(self.log.handlers):
+            handler.close()
 
     capital_words_re = re.compile('[A-Z][a-zA-Z0-9]+')
 
@@ -407,20 +453,25 @@ class BenchmarkDoctor(object):
         range_i1, range_i2 = max_i1 - min_i1, max_i2 - min_i2
         normal_range = 15  # pages
         name = measurements['name']
+        more_info = False
 
         if abs(min_i1 - min_i2) > max(range_i1, range_i2, normal_range):
+            more_info = True
             BenchmarkDoctor.log_memory.error(
                 "'%s' varies the memory footprint of the base "
                 "workload depending on the `num-iters`.", name)
 
         if max(range_i1, range_i2) > normal_range:
+            more_info = True
             BenchmarkDoctor.log_memory.warning(
                 "'%s' has very wide range of memory used between "
                 "independent, repeated measurements.", name)
 
-        BenchmarkDoctor.log_memory.debug(
-            "%s mem_pages [i1, i2]:  min=[%d, %d] 𝚫=%d R=[%d, %d]", name,
-            *[min_i1, min_i2, abs(min_i1 - min_i2), range_i1, range_i2])
+        if more_info:
+            BenchmarkDoctor.log_memory.info(
+                "'%s' mem_pages [i1, i2]: min=[%d, %d] 𝚫=%d R=[%d, %d]",
+                name,
+                *[min_i1, min_i2, abs(min_i1 - min_i2), range_i1, range_i2])
 
     @staticmethod
     def _adjusted_1s_samples(runtime):

benchmark/scripts/run_smoke_bench

@@ -61,6 +61,9 @@ def main():
     argparser.add_argument(
         '-skip-performance', action='store_true',
         help="Don't report performance differences")
+    argparser.add_argument(
+        '-skip-check-added', action='store_true',
+        help="Don't validate newly added benchmarks")
     argparser.add_argument(
         '-o', type=str,
         help='In addition to stdout, write the results into a markdown file')
@@ -80,15 +83,10 @@ def main():
     argparser.add_argument(
         'newbuilddir', nargs=1, type=str,
         help='new benchmark build directory')
-    argparser.add_argument(
-        '-check-added', action='store_const',
-        help="Run BenchmarkDoctor's check on newly added benchmarks",
-        const=lambda args: check_added(args), dest='func')
-    argparser.set_defaults(func=test_opt_levels)
     args = argparser.parse_args()
     VERBOSE = args.verbose
 
-    return args.func(args)
+    return test_opt_levels(args)
 
 
 def test_opt_levels(args):
@@ -119,6 +117,9 @@ def test_opt_levels(args):
                             args.platform, output_file):
             changes = True
 
+    if not args.skip_check_added:
+        check_added(args, output_file)
+
     if output_file:
         if changes:
             output_file.write(get_info_text())
@@ -339,7 +340,7 @@ class DriverArgs(object):
         self.optimization = 'O'
 
 
-def check_added(args):
+def check_added(args, output_file=None):
     from imp import load_source
     # import Benchmark_Driver  # doesn't work because it misses '.py' extension
     Benchmark_Driver = load_source(
@@ -348,12 +349,15 @@ def check_added(args):
     # from Benchmark_Driver import BenchmarkDriver, BenchmarkDoctor
     BenchmarkDriver = Benchmark_Driver.BenchmarkDriver
     BenchmarkDoctor = Benchmark_Driver.BenchmarkDoctor
+    MarkdownReportHandler = Benchmark_Driver.MarkdownReportHandler
 
     old = BenchmarkDriver(DriverArgs(args.oldbuilddir[0]))
     new = BenchmarkDriver(DriverArgs(args.newbuilddir[0]))
     added = set(new.tests).difference(set(old.tests))
     new.tests = list(added)
     doctor = BenchmarkDoctor(args, driver=new)
+    if added and output_file:
+        doctor.log.addHandler(MarkdownReportHandler(output_file))
     doctor.check()
 
 

benchmark/scripts/test_Benchmark_Driver.py

@@ -18,6 +18,7 @@
 import time
 import unittest
 
+from StringIO import StringIO
 from imp import load_source
 
 from compare_perf_tests import PerformanceTestResult
@@ -33,6 +34,7 @@
 BenchmarkDriver = Benchmark_Driver.BenchmarkDriver
 BenchmarkDoctor = Benchmark_Driver.BenchmarkDoctor
 LoggingReportFormatter = Benchmark_Driver.LoggingReportFormatter
+MarkdownReportHandler = Benchmark_Driver.MarkdownReportHandler
 
 
 class Test_parse_args(unittest.TestCase):
@@ -421,6 +423,58 @@ def test_no_prefix_for_base_logging(self):
         self.assertEquals(f.format(lr), 'INFO Hi!')
 
 
+class TestMarkdownReportHandler(unittest.TestCase):
+    def setUp(self):
+        super(TestMarkdownReportHandler, self).setUp()
+        self.stream = StringIO()
+        self.handler = MarkdownReportHandler(self.stream)
+
+    def assert_contains(self, texts):
+        assert not isinstance(texts, str)
+        for text in texts:
+            self.assertIn(text, self.stream.getvalue())
+
+    def record(self, level, category, msg):
+        return logging.makeLogRecord({
+            'name': 'BenchmarkDoctor.' + category,
+            'levelno': level, 'msg': msg})
+
+    def test_init_writes_table_header(self):
+        self.assertEquals(self.handler.level, logging.INFO)
+        self.assert_contains(['Benchmark Check Report\n', '---|---'])
+
+    def test_close_writes_final_newlines(self):
+        self.handler.close()
+        self.assert_contains(['---|---\n\n'])
+
+    def test_errors_and_warnings_start_new_rows_with_icons(self):
+        self.handler.emit(self.record(logging.ERROR, '', 'Blunder'))
+        self.handler.emit(self.record(logging.WARNING, '', 'Boo-boo'))
+        self.assert_contains(['\n⛔️ | Blunder',
+                              '\n⚠️ | Boo-boo'])
+
+    def test_category_icons(self):
+        self.handler.emit(self.record(logging.WARNING, 'naming', 'naming'))
+        self.handler.emit(self.record(logging.WARNING, 'runtime', 'runtime'))
+        self.handler.emit(self.record(logging.WARNING, 'memory', 'memory'))
+        self.assert_contains(['🔤 | naming',
+                              '⏱ | runtime',
+                              'Ⓜ️ | memory'])
+
+    def test_info_stays_in_table_cell_breaking_line_row_to_subscript(self):
+        """Assuming Infos only follow after Errors and Warnings.
+
+        Infos don't emit category icons.
+        """
+        self.handler.emit(self.record(logging.ERROR, 'naming', 'Blunder'))
+        self.handler.emit(self.record(logging.INFO, 'naming', 'Fixit'))
+        self.assert_contains(['Blunder <br><sub> Fixit'])
+
+    def test_names_in_code_format(self):
+        self.handler.emit(self.record(logging.WARNING, '', "'QuotedName'"))
+        self.assert_contains(['| `QuotedName`'])
+
+
 def _PTR(min=700, mem_pages=1000, setup=None):
     """Create PerformanceTestResult Stub."""
     return Stub(samples=Stub(min=min), mem_pages=mem_pages, setup=setup)
@@ -681,10 +735,18 @@ def test_benchmark_has_constant_memory_use(self):
             ["'VariableMemory' varies the memory footprint of the base "
              "workload depending on the `num-iters`."],
             self.logs['error'])
+        self.assert_contains(
+            ["'VariableMemory' "
+             "mem_pages [i1, i2]: min=[1460, 1750] 𝚫=290 R=[12, 2]"],
+            self.logs['info'])
         self.assert_contains(
             ["'HighVariance' has very wide range of memory used between "
              "independent, repeated measurements."],
             self.logs['warning'])
+        self.assert_contains(
+            ["'HighVariance' "
+             "mem_pages [i1, i2]: min=[4818, 4674] 𝚫=144 R=[1382, 1570]"],
+            self.logs['info'])
 
 
 if __name__ == '__main__':