Fixing static initilize fiasco in Benchmark.cpp
authorMaxim Sokolov <maxim@fb.com>
Tue, 2 Dec 2014 00:03:36 +0000 (16:03 -0800)
committerDave Watson <davejwatson@fb.com>
Thu, 11 Dec 2014 15:59:11 +0000 (07:59 -0800)
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

folly/Benchmark.cpp

index 66bcf7c1e76c4b0a9f7bb1277d9febddc2ad897e..de379ec598367516f421c3c46aeb6af2c43b7307 100644 (file)
@@ -28,6 +28,7 @@
 #include <limits>
 #include <utility>
 #include <vector>
+#include <cstring>
 
 using namespace std;
 
@@ -52,10 +53,18 @@ namespace folly {
 BenchmarkSuspender::NanosecondsSpent BenchmarkSuspender::nsSpent;
 
 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
-BENCHMARK(globalBenchmarkBaseline) {
+BENCHMARK(FB_FOLLY_GLOBAL_BENCHMARK_BASELINE) {
 #ifdef _MSC_VER
   _ReadWriteBarrier();
 #else
@@ -63,9 +72,25 @@ BENCHMARK(globalBenchmarkBaseline) {
 #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,
                               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(
 
   // Compute the longest benchmark name
   size_t longestName = 0;
-  FOR_EACH_RANGE (i, 1, benchmarks.size()) {
-    longestName = max(longestName, strlen(get<1>(benchmarks[i])));
+  FOR_EACH_RANGE (i, 1, benchmarks().size()) {
+    longestName = max(longestName, strlen(get<1>(benchmarks()[i])));
   }
 
   // Print a horizontal rule
@@ -413,10 +438,10 @@ static void printBenchmarkResults(
 }
 
 void runBenchmarks() {
-  CHECK(!benchmarks.empty());
+  CHECK(!benchmarks().empty());
 
   vector<tuple<const char*, const char*, double>> results;
-  results.reserve(benchmarks.size() - 1);
+  results.reserve(benchmarks().size() - 1);
 
   std::unique_ptr<boost::regex> bmRegex;
   if (!FLAGS_bm_regex.empty()) {
@@ -425,19 +450,24 @@ void runBenchmarks() {
 
   // PLEASE KEEP QUIET. MEASUREMENTS IN PROGRESS.
 
-  auto const globalBaseline = runBenchmarkGetNSPerIteration(
-    get<2>(benchmarks.front()), 0);
-  FOR_EACH_RANGE (i, 1, benchmarks.size()) {
+  unsigned int baselineIndex = getGlobalBenchmarkBaselineIndex();
+
+  auto const globalBaseline =
+      runBenchmarkGetNSPerIteration(get<2>(benchmarks()[baselineIndex]), 0);
+  FOR_EACH_RANGE (i, 0, benchmarks().size()) {
+    if (i == baselineIndex) {
+      continue;
+    }
     double elapsed = 0.0;
-    if (strcmp(get<1>(benchmarks[i]), "-") != 0) { // skip separators
-      if (bmRegex && !boost::regex_search(get<1>(benchmarks[i]), *bmRegex)) {
+    if (strcmp(get<1>(benchmarks()[i]), "-") != 0) { // skip separators
+      if (bmRegex && !boost::regex_search(get<1>(benchmarks()[i]), *bmRegex)) {
         continue;
       }
-      elapsed = runBenchmarkGetNSPerIteration(get<2>(benchmarks[i]),
+      elapsed = runBenchmarkGetNSPerIteration(get<2>(benchmarks()[i]),
                                               globalBaseline);
     }
-    results.emplace_back(get<0>(benchmarks[i]),
-                         get<1>(benchmarks[i]), elapsed);
+    results.emplace_back(get<0>(benchmarks()[i]),
+                         get<1>(benchmarks()[i]), elapsed);
   }
 
   // PLEASE MAKE NOISE. MEASUREMENTS DONE.