Random interleaving of benchmark repetitions - the sequel (fixes #1051) #1163

LebedevRI · 2021-06-01T19:38:55Z

Inspired by the original implementation by Hai Huang @haih-g
from #1105.

The original implementation had design deficiencies that
weren't really addressable without redesign, so it was reverted.

In essence, the original implementation consisted of two separateable parts:

reducing the amount time each repetition is run for, and symmetrically increasing repetition count
running the repetitions in random order

While it worked fine for the usual case, it broke down when user would specify repetitions
(it would completely ignore that request), or specified per-repetition min time (while it would
still adjust the repetition count, it would not adjust the per-repetition time,
leading to much greater run times)

Here, like i was originally suggesting in the original review, i'm separating the features,
and only dealing with a single one - running repetitions in random order.

Now that the runs/repetitions are no longer in-order, the tooling may wish to sort the output,
and indeed compare.py has been updated to do that: #1168.

docs/random_interleaving.md

haihuang-google · 2021-06-01T21:26:11Z

@LebedevRI Thanks for doing this. You may also want to include the follow test cases from test/benchmark_random_interleaving_gtest.cc

TEST_F(BenchmarkTest, Match1) {...}
TEST_F(BenchmarkTest, Match1WithRepetition) {...}
TEST_F(BenchmarkTest, Match1WithRandomInterleaving) {...}

src/benchmark.cc

src/benchmark_runner.cc

dmah42 · 2021-06-02T09:55:13Z

src/benchmark_runner.cc

-      i.results = manager->results;
-    }
+void BenchmarkRunner::do_one_repetition() {
+  assert(has_repeats_remaining() && "Already done all repetitions?");


src/benchmark_runner.cc

src/benchmark_runner.h

src/benchmark.cc

…le#1051) Inspired by the original implementation by Hai Huang @haih-g from google#1105. The original implementation had design deficiencies that weren't really addressable without redesign, so it was reverted. In essence, the original implementation consisted of two separateable parts: * reducing the amount time each repetition is run for, and symmetrically increasing repetition count * running the repetitions in random order While it worked fine for the usual case, it broke down when user would specify repetitions (it would completely ignore that request), or specified per-repetition min time (while it would still adjust the repetition count, it would not adjust the per-repetition time, leading to much greater run times) Here, like i was originally suggesting in the original review, i'm separating the features, and only dealing with a single one - running repetitions in random order. Now that the runs/repetitions are no longer in-order, the tooling may wish to sort the output, and indeed `compare.py` has been updated to do that: google#1168.

LebedevRI · 2021-06-03T18:16:48Z

Thank you for the review!

…le#1051) (google#1163) Inspired by the original implementation by Hai Huang @haih-g from google#1105. The original implementation had design deficiencies that weren't really addressable without redesign, so it was reverted. In essence, the original implementation consisted of two separateable parts: * reducing the amount time each repetition is run for, and symmetrically increasing repetition count * running the repetitions in random order While it worked fine for the usual case, it broke down when user would specify repetitions (it would completely ignore that request), or specified per-repetition min time (while it would still adjust the repetition count, it would not adjust the per-repetition time, leading to much greater run times) Here, like i was originally suggesting in the original review, i'm separating the features, and only dealing with a single one - running repetitions in random order. Now that the runs/repetitions are no longer in-order, the tooling may wish to sort the output, and indeed `compare.py` has been updated to do that: google#1168.

LebedevRI requested a review from dmah42 June 1, 2021 19:38

google-cla bot added the cla: yes label Jun 1, 2021

haihuang-google reviewed Jun 1, 2021

View reviewed changes

docs/random_interleaving.md Outdated Show resolved Hide resolved