Skip to content

Commit a24d0ff

Browse files
committed
[benchmark] BenchmarkDoctor checks setup time
Add a check against unreasonably long setup times for benchmarks that do their initialization work in the `setUpFunction`. Given the typical benchmark measurements will last about 1 second, it’s reasonable to expect the setup to take at most 20% extra, on top of that: 200 ms. The `DictionaryKeysContains*` benchmarks are an instance of this mistake. The setup of `DictionaryKeysContainsNative` takes 3 seconds on my machine, to prepare a dictionary for the run function, whose typical runtime is 90 μs. The setup of Cocoa version takes 8 seconds!!! It is trivial to rewite these with much smaller dictionaries that demonstrate the point of these benchmarks perfectly well, without the need to wait for ages to setup these benchmarks.
1 parent 638f4f8 commit a24d0ff

File tree

2 files changed

+37
-2
lines changed

2 files changed

+37
-2
lines changed

benchmark/scripts/Benchmark_Driver

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,7 @@ class BenchmarkDoctor(object):
296296
self._name_matches_capital_words_convention,
297297
self._name_is_at_most_40_chars_long,
298298
self._no_setup_overhead,
299+
self._reasonable_setup_time,
299300
self._optimized_runtime_in_range,
300301
self._constant_memory_use
301302
]
@@ -383,6 +384,17 @@ class BenchmarkDoctor(object):
383384
'Move initialization of benchmark data to the `setUpFunction` '
384385
'registered in `BenchmarkInfo`.')
385386

387+
@staticmethod
388+
def _reasonable_setup_time(measurements):
389+
setup = min([result.setup
390+
for result in BenchmarkDoctor._select(measurements)])
391+
if 200000 < setup: # 200 ms
392+
BenchmarkDoctor.log_runtime.error(
393+
"'%s' setup took at least %d μs.",
394+
measurements['name'], setup)
395+
BenchmarkDoctor.log_runtime.info(
396+
'The `setUpFunction` should take no more than 200 ms.')
397+
386398
@staticmethod
387399
def _constant_memory_use(measurements):
388400
select = BenchmarkDoctor._select

benchmark/scripts/test_Benchmark_Driver.py

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -421,9 +421,9 @@ def test_no_prefix_for_base_logging(self):
421421
self.assertEquals(f.format(lr), 'INFO Hi!')
422422

423423

424-
def _PTR(min=700, mem_pages=1000):
424+
def _PTR(min=700, mem_pages=1000, setup=None):
425425
"""Create PerformanceTestResult Stub."""
426-
return Stub(min=min, mem_pages=mem_pages)
426+
return Stub(min=min, mem_pages=mem_pages, setup=setup)
427427

428428

429429
def _run(test, num_samples=None, num_iters=None, verbose=None,
@@ -621,6 +621,29 @@ def test_benchmark_has_no_significant_setup_overhead(self):
621621
["Move initialization of benchmark data to the `setUpFunction` "
622622
"registered in `BenchmarkInfo`."], self.logs['info'])
623623

624+
def test_benchmark_setup_takes_reasonable_time(self):
625+
"""Setup < 200 ms (20% extra on top of the typical 1 s measurement)"""
626+
with captured_output() as (out, _):
627+
doctor = BenchmarkDoctor(self.args, BenchmarkDriverMock([]))
628+
doctor.analyze({
629+
'name': 'NormalSetup',
630+
'NormalSetup O i1a': _PTR(setup=199999),
631+
'NormalSetup O i2a': _PTR(setup=200001)})
632+
doctor.analyze({
633+
'name': 'LongSetup',
634+
'LongSetup O i1a': _PTR(setup=200001),
635+
'LongSetup O i2a': _PTR(setup=200002)})
636+
output = out.getvalue()
637+
638+
self.assertIn('runtime: ', output)
639+
self.assertNotIn('NormalSetup', output)
640+
self.assert_contains(
641+
["'LongSetup' setup took at least 200001 μs."],
642+
self.logs['error'])
643+
self.assert_contains(
644+
["The `setUpFunction` should take no more than 200 ms."],
645+
self.logs['info'])
646+
624647
def test_benchmark_has_constant_memory_use(self):
625648
"""Benchmark's memory footprint must not vary with num-iters."""
626649
with captured_output() as (out, _):

0 commit comments

Comments
 (0)