Commit c5dea7b6 authored by Maxim Sokolov's avatar Maxim Sokolov Committed by Dave Watson

Fixing static initilize fiasco in Benchmark.cpp

Summary:
Benchmark.cpp code is dependent on initialization order, which leads
to benchmarks not running. The root cause is that on some linkers the code which
adds benchmarks to the benchmarks vector is run before the vector initialization.

The original bug report: https://github.com/facebook/folly/issues/18
See: http://www.parashift.com/c++-faq-lite/static-init-order.html

Test Plan: run benchmark_test and compare result with master output

Reviewed By: andrei.alexandrescu@fb.com, njormrod@fb.com

Subscribers: trunkagent, sdwilsh, folly-diffs@

FB internal diff: D1710588

Signature: t1:1710588:1417468175:fd2705a573cef5c4ff020b60c6aec3d83bcdbbab
parent d904cfb9
...@@ -28,6 +28,7 @@ ...@@ -28,6 +28,7 @@
#include <limits> #include <limits>
#include <utility> #include <utility>
#include <vector> #include <vector>
#include <cstring>
using namespace std; using namespace std;
...@@ -52,10 +53,18 @@ namespace folly { ...@@ -52,10 +53,18 @@ namespace folly {
BenchmarkSuspender::NanosecondsSpent BenchmarkSuspender::nsSpent; BenchmarkSuspender::NanosecondsSpent BenchmarkSuspender::nsSpent;
typedef function<detail::TimeIterPair(unsigned int)> BenchmarkFun; typedef function<detail::TimeIterPair(unsigned int)> BenchmarkFun;
static vector<tuple<const char*, const char*, BenchmarkFun>> benchmarks;
vector<tuple<const char*, const char*, BenchmarkFun>>& benchmarks() {
static vector<tuple<const char*, const char*, BenchmarkFun>> _benchmarks;
return _benchmarks;
}
#define FB_FOLLY_GLOBAL_BENCHMARK_BASELINE fbFollyGlobalBenchmarkBaseline
#define FB_STRINGIZE_X2(x) FB_STRINGIZE(x)
// Add the global baseline // Add the global baseline
BENCHMARK(globalBenchmarkBaseline) { BENCHMARK(FB_FOLLY_GLOBAL_BENCHMARK_BASELINE) {
#ifdef _MSC_VER #ifdef _MSC_VER
_ReadWriteBarrier(); _ReadWriteBarrier();
#else #else
...@@ -63,9 +72,25 @@ BENCHMARK(globalBenchmarkBaseline) { ...@@ -63,9 +72,25 @@ BENCHMARK(globalBenchmarkBaseline) {
#endif #endif
} }
int getGlobalBenchmarkBaselineIndex() {
const char *global = FB_STRINGIZE_X2(FB_FOLLY_GLOBAL_BENCHMARK_BASELINE);
auto it = std::find_if(
benchmarks().begin(),
benchmarks().end(),
[global](const tuple<const char*, const char*, BenchmarkFun> &v) {
return std::strcmp(get<1>(v), global) == 0;
}
);
CHECK(it != benchmarks().end());
return it - benchmarks().begin();
}
#undef FB_STRINGIZE_X2
#undef FB_FOLLY_GLOBAL_BENCHMARK_BASELINE
void detail::addBenchmarkImpl(const char* file, const char* name, void detail::addBenchmarkImpl(const char* file, const char* name,
BenchmarkFun fun) { BenchmarkFun fun) {
benchmarks.emplace_back(file, name, std::move(fun)); benchmarks().emplace_back(file, name, std::move(fun));
} }
/** /**
...@@ -328,8 +353,8 @@ static void printBenchmarkResultsAsTable( ...@@ -328,8 +353,8 @@ static void printBenchmarkResultsAsTable(
// Compute the longest benchmark name // Compute the longest benchmark name
size_t longestName = 0; size_t longestName = 0;
FOR_EACH_RANGE (i, 1, benchmarks.size()) { FOR_EACH_RANGE (i, 1, benchmarks().size()) {
longestName = max(longestName, strlen(get<1>(benchmarks[i]))); longestName = max(longestName, strlen(get<1>(benchmarks()[i])));
} }
// Print a horizontal rule // Print a horizontal rule
...@@ -413,10 +438,10 @@ static void printBenchmarkResults( ...@@ -413,10 +438,10 @@ static void printBenchmarkResults(
} }
void runBenchmarks() { void runBenchmarks() {
CHECK(!benchmarks.empty()); CHECK(!benchmarks().empty());
vector<tuple<const char*, const char*, double>> results; vector<tuple<const char*, const char*, double>> results;
results.reserve(benchmarks.size() - 1); results.reserve(benchmarks().size() - 1);
std::unique_ptr<boost::regex> bmRegex; std::unique_ptr<boost::regex> bmRegex;
if (!FLAGS_bm_regex.empty()) { if (!FLAGS_bm_regex.empty()) {
...@@ -425,19 +450,24 @@ void runBenchmarks() { ...@@ -425,19 +450,24 @@ void runBenchmarks() {
// PLEASE KEEP QUIET. MEASUREMENTS IN PROGRESS. // PLEASE KEEP QUIET. MEASUREMENTS IN PROGRESS.
auto const globalBaseline = runBenchmarkGetNSPerIteration( unsigned int baselineIndex = getGlobalBenchmarkBaselineIndex();
get<2>(benchmarks.front()), 0);
FOR_EACH_RANGE (i, 1, benchmarks.size()) { auto const globalBaseline =
runBenchmarkGetNSPerIteration(get<2>(benchmarks()[baselineIndex]), 0);
FOR_EACH_RANGE (i, 0, benchmarks().size()) {
if (i == baselineIndex) {
continue;
}
double elapsed = 0.0; double elapsed = 0.0;
if (strcmp(get<1>(benchmarks[i]), "-") != 0) { // skip separators if (strcmp(get<1>(benchmarks()[i]), "-") != 0) { // skip separators
if (bmRegex && !boost::regex_search(get<1>(benchmarks[i]), *bmRegex)) { if (bmRegex && !boost::regex_search(get<1>(benchmarks()[i]), *bmRegex)) {
continue; continue;
} }
elapsed = runBenchmarkGetNSPerIteration(get<2>(benchmarks[i]), elapsed = runBenchmarkGetNSPerIteration(get<2>(benchmarks()[i]),
globalBaseline); globalBaseline);
} }
results.emplace_back(get<0>(benchmarks[i]), results.emplace_back(get<0>(benchmarks()[i]),
get<1>(benchmarks[i]), elapsed); get<1>(benchmarks()[i]), elapsed);
} }
// PLEASE MAKE NOISE. MEASUREMENTS DONE. // PLEASE MAKE NOISE. MEASUREMENTS DONE.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment