diff --git a/benchmark/scripts/Benchmark_Driver b/benchmark/scripts/Benchmark_Driver index 6e9af9703b4fc..9de641f990715 100755 --- a/benchmark/scripts/Benchmark_Driver +++ b/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: '
'} + 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): diff --git a/benchmark/scripts/run_smoke_bench b/benchmark/scripts/run_smoke_bench index 7904aed76f147..25f63a2e85840 100755 --- a/benchmark/scripts/run_smoke_bench +++ b/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()) @@ -338,7 +339,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( @@ -347,12 +348,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() diff --git a/benchmark/scripts/test_Benchmark_Driver.py b/benchmark/scripts/test_Benchmark_Driver.py index 4ff7c32a0e28c..fd81c0f0e90d4 100644 --- a/benchmark/scripts/test_Benchmark_Driver.py +++ b/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
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__':