Skip to content

Commit 3d85343

Browse files
LebedevRILebedevRI
authored
Rewrite complexity_test to use (hardcoded) manual time (google#1757)
* Rewrite complexity_test to use (hardcoded) manual time This test is fundamentally flaky, because it tried to read tea leafs, and is inherently misbehaving in CI environments, since there are unmitigated sources of noise. That being said, the computed Big-O also depends on the `--benchmark_min_time=` Fixes google#272 * Correctly compute Big-O for manual timings. Fixes google#1758. * complexity_test: do more stuff in empty loop * Make all empty loops be a bit longer empty Looks like on windows, some of these tests still fail, i guess clock precision is too small.
1 parent 7f7c96a commit 3d85343

15 files changed

+139
-85
lines changed

‎include/benchmark/benchmark.h

+5
Original file line numberDiff line numberDiff line change
@@ -1792,6 +1792,7 @@ class BENCHMARK_EXPORT BenchmarkReporter {
17921792
real_accumulated_time(0),
17931793
cpu_accumulated_time(0),
17941794
max_heapbytes_used(0),
1795+
use_real_time_for_initial_big_o(false),
17951796
complexity(oNone),
17961797
complexity_lambda(),
17971798
complexity_n(0),
@@ -1834,6 +1835,10 @@ class BENCHMARK_EXPORT BenchmarkReporter {
18341835
// This is set to 0.0 if memory tracing is not enabled.
18351836
double max_heapbytes_used;
18361837

1838+
// By default Big-O is computed for CPU time, but that is not what you want
1839+
// to happen when manual time was requested, which is stored as real time.
1840+
bool use_real_time_for_initial_big_o;
1841+
18371842
// Keep track of arguments to compute asymptotic complexity
18381843
BigO complexity;
18391844
BigOFunc* complexity_lambda;

‎src/benchmark_runner.cc

+1
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@ BenchmarkReporter::Run CreateRunReport(
9696
} else {
9797
report.real_accumulated_time = results.real_time_used;
9898
}
99+
report.use_real_time_for_initial_big_o = b.use_manual_time();
99100
report.cpu_accumulated_time = results.cpu_time_used;
100101
report.complexity_n = results.complexity_n;
101102
report.complexity = b.complexity();

‎src/complexity.cc

+13-2
Original file line numberDiff line numberDiff line change
@@ -186,8 +186,19 @@ std::vector<BenchmarkReporter::Run> ComputeBigO(
186186
result_cpu = MinimalLeastSq(n, cpu_time, reports[0].complexity_lambda);
187187
result_real = MinimalLeastSq(n, real_time, reports[0].complexity_lambda);
188188
} else {
189-
result_cpu = MinimalLeastSq(n, cpu_time, reports[0].complexity);
190-
result_real = MinimalLeastSq(n, real_time, result_cpu.complexity);
189+
const BigO* InitialBigO = &reports[0].complexity;
190+
const bool use_real_time_for_initial_big_o =
191+
reports[0].use_real_time_for_initial_big_o;
192+
if (use_real_time_for_initial_big_o) {
193+
result_real = MinimalLeastSq(n, real_time, *InitialBigO);
194+
InitialBigO = &result_real.complexity;
195+
// The Big-O complexity for CPU time must have the same Big-O function!
196+
}
197+
result_cpu = MinimalLeastSq(n, cpu_time, *InitialBigO);
198+
InitialBigO = &result_cpu.complexity;
199+
if (!use_real_time_for_initial_big_o) {
200+
result_real = MinimalLeastSq(n, real_time, *InitialBigO);
201+
}
191202
}
192203

193204
// Drop the 'args' when reporting complexity.

‎test/BUILD

+1
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ PER_SRC_TEST_ARGS = {
3535
"repetitions_test.cc": [" --benchmark_repetitions=3"],
3636
"spec_arg_test.cc": ["--benchmark_filter=BM_NotChosen"],
3737
"spec_arg_verbosity_test.cc": ["--v=42"],
38+
"complexity_test.cc": ["--benchmark_min_time=1000000x"],
3839
}
3940

4041
cc_library(

‎test/CMakeLists.txt

+1-7
Original file line numberDiff line numberDiff line change
@@ -218,14 +218,8 @@ if(NOT (MSVC OR CMAKE_CXX_SIMULATE_ID STREQUAL "MSVC"))
218218
benchmark_add_test(NAME cxx03 COMMAND cxx03_test --benchmark_min_time=0.01s)
219219
endif()
220220

221-
# Attempt to work around flaky test failures when running on Appveyor servers.
222-
if (DEFINED ENV{APPVEYOR})
223-
set(COMPLEXITY_MIN_TIME "0.5s")
224-
else()
225-
set(COMPLEXITY_MIN_TIME "0.01s")
226-
endif()
227221
compile_output_test(complexity_test)
228-
benchmark_add_test(NAME complexity_benchmark COMMAND complexity_test --benchmark_min_time=${COMPLEXITY_MIN_TIME})
222+
benchmark_add_test(NAME complexity_benchmark COMMAND complexity_test --benchmark_min_time=1000000x)
229223

230224
###############################################################################
231225
# GoogleTest Unit Tests

‎test/basic_test.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55

66
void BM_empty(benchmark::State& state) {
77
for (auto _ : state) {
8-
auto iterations = state.iterations();
8+
auto iterations = double(state.iterations()) * double(state.iterations());
99
benchmark::DoNotOptimize(iterations);
1010
}
1111
}

‎test/complexity_test.cc

+100-58
Original file line numberDiff line numberDiff line change
@@ -69,35 +69,44 @@ int AddComplexityTest(const std::string &test_name,
6969

7070
void BM_Complexity_O1(benchmark::State &state) {
7171
for (auto _ : state) {
72-
for (int i = 0; i < 1024; ++i) {
73-
benchmark::DoNotOptimize(i);
72+
// This test requires a non-zero CPU time to avoid divide-by-zero
73+
benchmark::DoNotOptimize(state.iterations());
74+
double tmp = state.iterations();
75+
benchmark::DoNotOptimize(tmp);
76+
for (benchmark::IterationCount i = 0; i < state.iterations(); ++i) {
77+
benchmark::DoNotOptimize(state.iterations());
78+
tmp *= state.iterations();
79+
benchmark::DoNotOptimize(tmp);
7480
}
81+
82+
// always 1ns per iteration
83+
state.SetIterationTime(42 * 1e-9);
7584
}
7685
state.SetComplexityN(state.range(0));
7786
}
78-
BENCHMARK(BM_Complexity_O1)->Range(1, 1 << 18)->Complexity(benchmark::o1);
79-
BENCHMARK(BM_Complexity_O1)->Range(1, 1 << 18)->Complexity();
8087
BENCHMARK(BM_Complexity_O1)
8188
->Range(1, 1 << 18)
89+
->UseManualTime()
90+
->Complexity(benchmark::o1);
91+
BENCHMARK(BM_Complexity_O1)->Range(1, 1 << 18)->UseManualTime()->Complexity();
92+
BENCHMARK(BM_Complexity_O1)
93+
->Range(1, 1 << 18)
94+
->UseManualTime()
8295
->Complexity([](benchmark::IterationCount) { return 1.0; });
8396

84-
const char *one_test_name = "BM_Complexity_O1";
85-
const char *big_o_1_test_name = "BM_Complexity_O1_BigO";
86-
const char *rms_o_1_test_name = "BM_Complexity_O1_RMS";
87-
const char *enum_big_o_1 = "\\([0-9]+\\)";
88-
// FIXME: Tolerate both '(1)' and 'lgN' as output when the complexity is auto
89-
// deduced.
90-
// See https://github.com/google/benchmark/issues/272
91-
const char *auto_big_o_1 = "(\\([0-9]+\\))|(lgN)|(N\\^2)";
97+
const char *one_test_name = "BM_Complexity_O1/manual_time";
98+
const char *big_o_1_test_name = "BM_Complexity_O1/manual_time_BigO";
99+
const char *rms_o_1_test_name = "BM_Complexity_O1/manual_time_RMS";
100+
const char *enum_auto_big_o_1 = "\\([0-9]+\\)";
92101
const char *lambda_big_o_1 = "f\\(N\\)";
93102

94103
// Add enum tests
95104
ADD_COMPLEXITY_CASES(one_test_name, big_o_1_test_name, rms_o_1_test_name,
96-
enum_big_o_1, /*family_index=*/0);
105+
enum_auto_big_o_1, /*family_index=*/0);
97106

98-
// Add auto enum tests
107+
// Add auto tests
99108
ADD_COMPLEXITY_CASES(one_test_name, big_o_1_test_name, rms_o_1_test_name,
100-
auto_big_o_1, /*family_index=*/1);
109+
enum_auto_big_o_1, /*family_index=*/1);
101110

102111
// Add lambda tests
103112
ADD_COMPLEXITY_CASES(one_test_name, big_o_1_test_name, rms_o_1_test_name,
@@ -107,84 +116,102 @@ ADD_COMPLEXITY_CASES(one_test_name, big_o_1_test_name, rms_o_1_test_name,
107116
// --------------------------- Testing BigO O(N) --------------------------- //
108117
// ========================================================================= //
109118

110-
std::vector<int> ConstructRandomVector(int64_t size) {
111-
std::vector<int> v;
112-
v.reserve(static_cast<size_t>(size));
113-
for (int i = 0; i < size; ++i) {
114-
v.push_back(static_cast<int>(std::rand() % size));
115-
}
116-
return v;
117-
}
118-
119119
void BM_Complexity_O_N(benchmark::State &state) {
120-
auto v = ConstructRandomVector(state.range(0));
121-
// Test worst case scenario (item not in vector)
122-
const int64_t item_not_in_vector = state.range(0) * 2;
123120
for (auto _ : state) {
124-
auto it = std::find(v.begin(), v.end(), item_not_in_vector);
125-
benchmark::DoNotOptimize(it);
121+
// This test requires a non-zero CPU time to avoid divide-by-zero
122+
benchmark::DoNotOptimize(state.iterations());
123+
double tmp = state.iterations();
124+
benchmark::DoNotOptimize(tmp);
125+
for (benchmark::IterationCount i = 0; i < state.iterations(); ++i) {
126+
benchmark::DoNotOptimize(state.iterations());
127+
tmp *= state.iterations();
128+
benchmark::DoNotOptimize(tmp);
129+
}
130+
131+
// 1ns per iteration per entry
132+
state.SetIterationTime(state.range(0) * 42 * 1e-9);
126133
}
127134
state.SetComplexityN(state.range(0));
128135
}
129136
BENCHMARK(BM_Complexity_O_N)
130137
->RangeMultiplier(2)
131-
->Range(1 << 10, 1 << 16)
138+
->Range(1 << 10, 1 << 20)
139+
->UseManualTime()
132140
->Complexity(benchmark::oN);
133141
BENCHMARK(BM_Complexity_O_N)
134142
->RangeMultiplier(2)
135-
->Range(1 << 10, 1 << 16)
143+
->Range(1 << 10, 1 << 20)
144+
->UseManualTime()
145+
->Complexity();
146+
BENCHMARK(BM_Complexity_O_N)
147+
->RangeMultiplier(2)
148+
->Range(1 << 10, 1 << 20)
149+
->UseManualTime()
136150
->Complexity([](benchmark::IterationCount n) -> double {
137151
return static_cast<double>(n);
138152
});
139-
BENCHMARK(BM_Complexity_O_N)
140-
->RangeMultiplier(2)
141-
->Range(1 << 10, 1 << 16)
142-
->Complexity();
143153

144-
const char *n_test_name = "BM_Complexity_O_N";
145-
const char *big_o_n_test_name = "BM_Complexity_O_N_BigO";
146-
const char *rms_o_n_test_name = "BM_Complexity_O_N_RMS";
154+
const char *n_test_name = "BM_Complexity_O_N/manual_time";
155+
const char *big_o_n_test_name = "BM_Complexity_O_N/manual_time_BigO";
156+
const char *rms_o_n_test_name = "BM_Complexity_O_N/manual_time_RMS";
147157
const char *enum_auto_big_o_n = "N";
148158
const char *lambda_big_o_n = "f\\(N\\)";
149159

150160
// Add enum tests
151161
ADD_COMPLEXITY_CASES(n_test_name, big_o_n_test_name, rms_o_n_test_name,
152162
enum_auto_big_o_n, /*family_index=*/3);
153163

164+
// Add auto tests
165+
ADD_COMPLEXITY_CASES(n_test_name, big_o_n_test_name, rms_o_n_test_name,
166+
enum_auto_big_o_n, /*family_index=*/4);
167+
154168
// Add lambda tests
155169
ADD_COMPLEXITY_CASES(n_test_name, big_o_n_test_name, rms_o_n_test_name,
156-
lambda_big_o_n, /*family_index=*/4);
170+
lambda_big_o_n, /*family_index=*/5);
157171

158172
// ========================================================================= //
159-
// ------------------------- Testing BigO O(N*lgN) ------------------------- //
173+
// ------------------------- Testing BigO O(NlgN) ------------------------- //
160174
// ========================================================================= //
161175

176+
static const double kLog2E = 1.44269504088896340736;
162177
static void BM_Complexity_O_N_log_N(benchmark::State &state) {
163-
auto v = ConstructRandomVector(state.range(0));
164178
for (auto _ : state) {
165-
std::sort(v.begin(), v.end());
179+
// This test requires a non-zero CPU time to avoid divide-by-zero
180+
benchmark::DoNotOptimize(state.iterations());
181+
double tmp = state.iterations();
182+
benchmark::DoNotOptimize(tmp);
183+
for (benchmark::IterationCount i = 0; i < state.iterations(); ++i) {
184+
benchmark::DoNotOptimize(state.iterations());
185+
tmp *= state.iterations();
186+
benchmark::DoNotOptimize(tmp);
187+
}
188+
189+
state.SetIterationTime(state.range(0) * kLog2E * std::log(state.range(0)) *
190+
42 * 1e-9);
166191
}
167192
state.SetComplexityN(state.range(0));
168193
}
169-
static const double kLog2E = 1.44269504088896340736;
170194
BENCHMARK(BM_Complexity_O_N_log_N)
171195
->RangeMultiplier(2)
172-
->Range(1 << 10, 1 << 16)
196+
->Range(1 << 10, 1U << 24)
197+
->UseManualTime()
173198
->Complexity(benchmark::oNLogN);
174199
BENCHMARK(BM_Complexity_O_N_log_N)
175200
->RangeMultiplier(2)
176-
->Range(1 << 10, 1 << 16)
201+
->Range(1 << 10, 1U << 24)
202+
->UseManualTime()
203+
->Complexity();
204+
BENCHMARK(BM_Complexity_O_N_log_N)
205+
->RangeMultiplier(2)
206+
->Range(1 << 10, 1U << 24)
207+
->UseManualTime()
177208
->Complexity([](benchmark::IterationCount n) {
178209
return kLog2E * static_cast<double>(n) * std::log(static_cast<double>(n));
179210
});
180-
BENCHMARK(BM_Complexity_O_N_log_N)
181-
->RangeMultiplier(2)
182-
->Range(1 << 10, 1 << 16)
183-
->Complexity();
184211

185-
const char *n_lg_n_test_name = "BM_Complexity_O_N_log_N";
186-
const char *big_o_n_lg_n_test_name = "BM_Complexity_O_N_log_N_BigO";
187-
const char *rms_o_n_lg_n_test_name = "BM_Complexity_O_N_log_N_RMS";
212+
const char *n_lg_n_test_name = "BM_Complexity_O_N_log_N/manual_time";
213+
const char *big_o_n_lg_n_test_name = "BM_Complexity_O_N_log_N/manual_time_BigO";
214+
const char *rms_o_n_lg_n_test_name = "BM_Complexity_O_N_log_N/manual_time_RMS";
188215
const char *enum_auto_big_o_n_lg_n = "NlgN";
189216
const char *lambda_big_o_n_lg_n = "f\\(N\\)";
190217

@@ -193,33 +220,48 @@ ADD_COMPLEXITY_CASES(n_lg_n_test_name, big_o_n_lg_n_test_name,
193220
rms_o_n_lg_n_test_name, enum_auto_big_o_n_lg_n,
194221
/*family_index=*/6);
195222

196-
// Add lambda tests
223+
// NOTE: auto big-o is wron.g
197224
ADD_COMPLEXITY_CASES(n_lg_n_test_name, big_o_n_lg_n_test_name,
198-
rms_o_n_lg_n_test_name, lambda_big_o_n_lg_n,
225+
rms_o_n_lg_n_test_name, enum_auto_big_o_n_lg_n,
199226
/*family_index=*/7);
200227

228+
//// Add lambda tests
229+
ADD_COMPLEXITY_CASES(n_lg_n_test_name, big_o_n_lg_n_test_name,
230+
rms_o_n_lg_n_test_name, lambda_big_o_n_lg_n,
231+
/*family_index=*/8);
232+
201233
// ========================================================================= //
202234
// -------- Testing formatting of Complexity with captured args ------------ //
203235
// ========================================================================= //
204236

205237
void BM_ComplexityCaptureArgs(benchmark::State &state, int n) {
206238
for (auto _ : state) {
207239
// This test requires a non-zero CPU time to avoid divide-by-zero
208-
auto iterations = state.iterations();
209-
benchmark::DoNotOptimize(iterations);
240+
benchmark::DoNotOptimize(state.iterations());
241+
double tmp = state.iterations();
242+
benchmark::DoNotOptimize(tmp);
243+
for (benchmark::IterationCount i = 0; i < state.iterations(); ++i) {
244+
benchmark::DoNotOptimize(state.iterations());
245+
tmp *= state.iterations();
246+
benchmark::DoNotOptimize(tmp);
247+
}
248+
249+
state.SetIterationTime(state.range(0) * 42 * 1e-9);
210250
}
211251
state.SetComplexityN(n);
212252
}
213253

214254
BENCHMARK_CAPTURE(BM_ComplexityCaptureArgs, capture_test, 100)
255+
->UseManualTime()
215256
->Complexity(benchmark::oN)
216257
->Ranges({{1, 2}, {3, 4}});
217258

218259
const std::string complexity_capture_name =
219-
"BM_ComplexityCaptureArgs/capture_test";
260+
"BM_ComplexityCaptureArgs/capture_test/manual_time";
220261

221262
ADD_COMPLEXITY_CASES(complexity_capture_name, complexity_capture_name + "_BigO",
222-
complexity_capture_name + "_RMS", "N", /*family_index=*/9);
263+
complexity_capture_name + "_RMS", "N",
264+
/*family_index=*/9);
223265

224266
// ========================================================================= //
225267
// --------------------------- TEST CASES END ------------------------------ //

‎test/diagnostics_test.cc

+2-2
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ void BM_diagnostic_test(benchmark::State& state) {
4949
if (called_once == false) try_invalid_pause_resume(state);
5050

5151
for (auto _ : state) {
52-
auto iterations = state.iterations();
52+
auto iterations = double(state.iterations()) * double(state.iterations());
5353
benchmark::DoNotOptimize(iterations);
5454
}
5555

@@ -65,7 +65,7 @@ void BM_diagnostic_test_keep_running(benchmark::State& state) {
6565
if (called_once == false) try_invalid_pause_resume(state);
6666

6767
while (state.KeepRunning()) {
68-
auto iterations = state.iterations();
68+
auto iterations = double(state.iterations()) * double(state.iterations());
6969
benchmark::DoNotOptimize(iterations);
7070
}
7171

‎test/link_main_test.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
void BM_empty(benchmark::State& state) {
44
for (auto _ : state) {
5-
auto iterations = state.iterations();
5+
auto iterations = double(state.iterations()) * double(state.iterations());
66
benchmark::DoNotOptimize(iterations);
77
}
88
}

‎test/memory_manager_test.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ class TestMemoryManager : public benchmark::MemoryManager {
1414

1515
void BM_empty(benchmark::State& state) {
1616
for (auto _ : state) {
17-
auto iterations = state.iterations();
17+
auto iterations = double(state.iterations()) * double(state.iterations());
1818
benchmark::DoNotOptimize(iterations);
1919
}
2020
}

‎test/perf_counters_test.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ BM_DECLARE_string(benchmark_perf_counters);
1414

1515
static void BM_Simple(benchmark::State& state) {
1616
for (auto _ : state) {
17-
auto iterations = state.iterations();
17+
auto iterations = double(state.iterations()) * double(state.iterations());
1818
benchmark::DoNotOptimize(iterations);
1919
}
2020
}

0 commit comments

Comments
 (0)