Incorporate pull request feedback

src/benchmark_api_internal.cc

@@ -47,19 +47,12 @@ BenchmarkInstance::BenchmarkInstance(Benchmark* benchmark, int family_idx,
     }
 
     // formatting args either in default mode or in human-readable
-    auto formattedArgument = StrFormat("%" PRId64, arg);
-
     if (FLAGS_benchmark_human_readable) {
-      if ((arg & (arg - 1)) == 0 && (arg > 1)) {
-        // power of two human format
-        formattedArgument = StrFormat("2^%.0f", std::log2(arg));
-      } else if (arg % 10 == 0 && (arg > 0)) {
-        // base 10 human format
-        formattedArgument = Base10HumanReadableFormat::get(arg);
-      }
+      name_.args += FormatHumanReadable(arg);
+    } else {
+      name_.args += StrFormat("%" PRId64, arg);
     }
 
-    name_.args += formattedArgument;
     ++arg_i;
   }
 
@@ -131,53 +124,12 @@ void BenchmarkInstance::Teardown() const {
   }
 }
 
-const std::array<Base10HumanReadableFormat, 5>
-    Base10HumanReadableFormat::ranges = {
-        Base10HumanReadableFormat{1, ""},             // from 0 to 10^3-1
-        Base10HumanReadableFormat{1000, "k"},         // from 10^3 to 10^6-1
-        Base10HumanReadableFormat{1000000, "m"},      // from 10^6 to 10^9-1
-        Base10HumanReadableFormat{1000000000, "bn"},  // from 10^9 to 10^12-1
-        Base10HumanReadableFormat{1000000000000, ""}  // above show as full
-};
-
-std::string Base10HumanReadableFormat::get(const int64_t& arg) {
-  // binary search for the best match
-  std::size_t left = 0;
-  std::size_t right = ranges.size() - 1;
-
-  while (left <= right) {
-    const std::size_t middle = left + std::floor((right - left) / 2);
-    const auto& middle_val = ranges[middle].entry_val_;
-
-    if (middle_val == arg) {
-      left = middle;
-      break;
-    }
-
-    if (arg < middle_val) {
-      right = middle - 1;
-    } else {
-      left = middle + 1;
-    }
-  }
-
-  const auto min_index = std::min(left, right);
-
-  // in case we get the "above" we return the '1' just for simplicity.
-  // because we use the "entry_val_" in later stages for division.
-  const auto& readable_format =
-      ranges[(min_index == ranges.size() - 1 ? 0 : min_index)];
-
-  // if it is not a perfect match we return the value
-  // e.g. 1030 we do not want 10k we want 1030
-  if (arg % readable_format.entry_val_ != 0) {
-    return StrFormat("%" PRId64, arg);
-  }
-
-  // for everything else we want the abbreviation
-  const auto reduced_val = arg / readable_format.entry_val_;
-  return StrFormat("%" PRId64 "%s", reduced_val,
-                   readable_format.abbreviation_.c_str());
+/**
+ * Check whether the given value is a power of two or not.
+ * @param val the value to check
+ */
+bool IsPowerOfTwo(const int64_t& val) {
+  return (val & (val - 1)) == 0 && (val > 1);
 }
 
 }  // namespace internal

src/benchmark_api_internal.h

@@ -17,24 +17,6 @@
 namespace benchmark {
 namespace internal {
 
-struct Base10HumanReadableFormat {
-  const int64_t entry_val_;
-  const std::string abbreviation_;
-
-  // holds all the possible abbreviation limits, easily expandable by adding a
-  // new line. Keep in mind they need to be in ascending order such that binary
-  // search works.
-  const static std::array<Base10HumanReadableFormat, 5> ranges;
-
-  /**
-   * Gets the human readable format for a given base10 value.
-   * In other words converts 1_000 to 1k, 40_000_000 to 40m etc
-   * @param arg the positive value to convert
-   * @return human readable formatted string
-   */
-  static std::string get(const int64_t& arg);
-};
-
 // Information kept per benchmark we may want to run
 class BenchmarkInstance {
  public:

src/string_util.cc

@@ -1,6 +1,7 @@
 #include "string_util.h"
 
 #include <array>
+#include <cinttypes>
 #ifdef BENCHMARK_STL_ANDROID_GNUSTL
 #include <cerrno>
 #endif
@@ -15,6 +16,11 @@
 namespace benchmark {
 namespace {
 
+// Thousands, Millions, Billions, Trillions, Quadrillions, Quintillions,
+// Sextillions, Septillions.
+const std::array<std::string, 8> base10Units = {"k", "M",  "B",  "T",
+                                                "Q", "Qi", "Sx", "Sp"};
+
 // kilo, Mega, Giga, Tera, Peta, Exa, Zetta, Yotta.
 const char kBigSIUnits[] = "kMGTPEZY";
 // Kibi, Mebi, Gibi, Tebi, Pebi, Exbi, Zebi, Yobi.
@@ -32,7 +38,8 @@ static const int64_t kUnitsSize = arraysize(kBigSIUnits);
 
 void ToExponentAndMantissa(double val, double thresh, int precision,
                            double one_k, std::string* mantissa,
-                           int64_t* exponent) {
+                           int64_t* exponent,
+                           bool inclusiveBigThreshhold = false) {
   std::stringstream mantissa_stream;
 
   if (val < 0) {
@@ -44,7 +51,8 @@ void ToExponentAndMantissa(double val, double thresh, int precision,
   // in 'precision' digits.
   const double adjusted_threshold =
       std::max(thresh, 1.0 / std::pow(10.0, precision));
-  const double big_threshold = adjusted_threshold * one_k;
+  const double big_threshold =
+      (adjusted_threshold * one_k) - inclusiveBigThreshhold;
   const double small_threshold = adjusted_threshold;
   // Values in ]simple_threshold,small_threshold[ will be printed as-is
   const double simple_threshold = 0.01;
@@ -262,4 +270,35 @@ double stod(const std::string& str, size_t* pos) {
 }
 #endif
 
+std::string ExponentToBase10Prefix(int64_t exponent) {
+  if (exponent == 0) return "";
+
+  const int64_t index = (exponent > 0 ? exponent - 1 : -exponent - 1);
+  if (index >= kUnitsSize) return "";
+
+  return base10Units[index];
+}
+
+std::string Base10HumanReadableFormat(const int64_t& arg) {
+  std::string mantissa;
+  int64_t exponent;
+  ToExponentAndMantissa(arg, 1, 1, 1000, &mantissa, &exponent, true);
+  return mantissa + ExponentToBase10Prefix(exponent);
+}
+
+bool IsPowerOfTwo(const int64_t& val) {
+  return (val & (val - 1)) == 0 && (val > 1);
+}
+
+std::string Base2HumanReadableFormat(const int64_t& arg) {
+  return StrFormat("2^%.0f", std::log2(arg));
+}
+
+std::string FormatHumanReadable(const int64_t& arg) {
+  if (IsPowerOfTwo(arg)) {
+    return Base2HumanReadableFormat(arg);
+  }
+  return Base10HumanReadableFormat(arg);
+}
+
 }  // end namespace benchmark

src/string_util.h

@@ -65,6 +65,35 @@ using std::stoul;  // NOLINT(misc-unused-using-decls)
 #endif
 // NOLINTEND
 
+/**
+ * Check whether the given value is a power of two or not.
+ * @param val the value to check
+ */
+bool IsPowerOfTwo(const int64_t& val);
+
+/**
+ * Gets the human readable format for a given base10 value.
+ * In other words converts 1_000 to 1k, 40_000_000 to 40m etc
+ * @param arg the positive value to convert
+ * @return human readable formatted string
+ */
+std::string Base10HumanReadableFormat(const int64_t& arg);
+
+/**
+ * Gets the human readable format for a given base2 value.
+ * In other words converts 64 to 2^6, 1024 to 2^10 etc
+ * @param arg the positive value to convert
+ * @return human readable formatted string
+ */
+std::string Base2HumanReadableFormat(const int64_t& arg);
+
+/**
+ * Formats an argument into a human readable format.
+ * @param arg the argument to format
+ * @return the argument formatted as human readable
+ */
+std::string FormatHumanReadable(const int64_t& arg);
+
 }  // end namespace benchmark
 
 #endif  // BENCHMARK_STRING_UTIL_H_

test/human_readable_formatting_test.cc

@@ -58,8 +58,8 @@ void BM_non_base_two_args(benchmark::State& state) {
   for (auto _ : state) {
   }
 }
-BENCHMARK(BM_non_base_two_args)->Arg(9)->Arg(19)->Arg(24)->Arg(1023);
-ADD_CASES("BM_non_base_two_args", {{"/9"}, {"/19"}, {"/24"}, {"/1023"}});
+BENCHMARK(BM_non_base_two_args)->Arg(9)->Arg(19)->Arg(24)->Arg(65);
+ADD_CASES("BM_non_base_two_args", {{"/9"}, {"/19"}, {"/24"}, {"/65"}});
 
 // ============== test case we base 2 args ============== //
 void BM_base_two_args(benchmark::State& state) {
@@ -98,8 +98,8 @@ ADD_CASES("BM_base_ten_args", {{"/1"},
                                {"/10k"},
                                {"/32k"},
                                {"/100k"},
-                               {"/1m"},
-                               {"/1bn"}});
+                               {"/1M"},
+                               {"/1B"}});
 
 int main(int argc, char* argv[]) {
   benchmark::Initialize(&argc, argv);

test/human_readable_gtest.cc

@@ -2,47 +2,68 @@
 // human_readable_test - Unit tests for human readable converters
 //===---------------------------------------------------------------------===//
 
-#include "../src/benchmark_api_internal.h"
+#include "../src/string_util.h"
 #include "gtest/gtest.h"
 
 namespace {
+
+TEST(HumanReadableTest, base2) {
+  for (int i = 2; i < 10; ++i) {
+    const auto res = benchmark::Base2HumanReadableFormat(1 << i);
+
+    const std::string expected = "2^" + std::to_string(i);
+    EXPECT_STREQ(res.c_str(), expected.c_str());
+  }
+}
+
 TEST(HumanReadableTest, base10) {
-  using Base10Human = benchmark::internal::Base10HumanReadableFormat;
   {
-    const auto res = Base10Human::get(10000);
+    const auto res = benchmark::Base10HumanReadableFormat(100);
+    EXPECT_STREQ(res.c_str(), "100");
+  }
+  {
+    const auto res = benchmark::Base10HumanReadableFormat(1000);
+    EXPECT_STREQ(res.c_str(), "1k");
+  }
+  {
+    const auto res = benchmark::Base10HumanReadableFormat(10000);
     EXPECT_STREQ(res.c_str(), "10k");
   }
   {
-    const auto res = Base10Human::get(20000);
+    const auto res = benchmark::Base10HumanReadableFormat(20000);
     EXPECT_STREQ(res.c_str(), "20k");
   }
   {
-    const auto res = Base10Human::get(32000);
+    const auto res = benchmark::Base10HumanReadableFormat(32000);
     EXPECT_STREQ(res.c_str(), "32k");
   }
   {
-    const auto res = Base10Human::get(100);
-    EXPECT_STREQ(res.c_str(), "100");
+    const auto res = benchmark::Base10HumanReadableFormat(1000000);
+    EXPECT_STREQ(res.c_str(), "1M");
+  }
+  {
+    const auto res = benchmark::Base10HumanReadableFormat(42000000);
+    EXPECT_STREQ(res.c_str(), "42M");
   }
   {
-    const auto res = Base10Human::get(1000000);
-    EXPECT_STREQ(res.c_str(), "1m");
+    const auto res = benchmark::Base10HumanReadableFormat(1000000000);
+    EXPECT_STREQ(res.c_str(), "1B");
   }
   {
-    const auto res = Base10Human::get(42000000);
-    EXPECT_STREQ(res.c_str(), "42m");
+    const auto res = benchmark::Base10HumanReadableFormat(4000000000);
+    EXPECT_STREQ(res.c_str(), "4B");
   }
   {
-    const auto res = Base10Human::get(4000000000);
-    EXPECT_STREQ(res.c_str(), "4bn");
+    const auto res = benchmark::Base10HumanReadableFormat(4200000000);
+    EXPECT_STREQ(res.c_str(), "4.2B");
   }
   {
-    const auto res = Base10Human::get(4200000000);
-    EXPECT_STREQ(res.c_str(), "4200000000");
+    const auto res = benchmark::Base10HumanReadableFormat(40200000);
+    EXPECT_STREQ(res.c_str(), "40.2M");
   }
   {
-    const auto res = Base10Human::get(40200000);
-    EXPECT_STREQ(res.c_str(), "40200000");
+    const auto res = benchmark::Base10HumanReadableFormat(4200000000000000000);
+    EXPECT_STREQ(res.c_str(), "4.2Qi");
   }
 }