Support time based user metrics in Folly::benchmark

Summary:
Fix for issue #1 in Post [Truncation of UserMetric::Time](https://fb.workplace.com/groups/560979627394613/permalink/3320975644728317/)

Our team uses [BENCHMARK_IMPL_COUNTERS](https://www.internalfb.com/code/fbsource/[f51516f2368d]/fbcode/folly/Benchmark.h?lines=379) to measure custom metrics during Folly::benchmarks execution. While working on updates to NodeAPI::SlaBenchmark in D77320464 I noticed Folly `UserMetric` accepts integer(Long) type values which works great for `UserMetric::Type::Custom` and `UserMetric::Type::Metric`, but causes issues for `UserMetric::Type::Time` values.

When [time metrics](https://www.internalfb.com/code/fbsource/[f51516f2368d8097604a13de292824a7d85d7de1]/fbcode/folly/Benchmark.cpp?lines=529) are formated for display the value is implicitly cast(`Long` -> `Double`) resulting in loss of precision. This results in any value smaller than 1s being displayed as 0.00fs due to the truncation making it impossible to display small time values.

~~This diff adds support for users to provide floating point values as input to `UserMetric` allowing precision to be retained through a new `time_value` field, while remaining backward compatible with existing usage that relies on the existing `value` field.~~ See discussion below

Reviewed By: yfeldblum

Differential Revision: D77319941

fbshipit-source-id: a4d55165b5429fa602ea357764db03a81b9ac760

Author: Nate Jensvold

third-party/folly/src/folly/Benchmark.cpp

@@ -29,6 +29,7 @@
 
 #include <folly/FileUtil.h>
 #include <folly/MapUtil.h>
+#include <folly/Overload.h>
 #include <folly/String.h>
 #include <folly/detail/PerfScoped.h>
 #include <folly/json/json.h>
@@ -522,21 +523,32 @@ class BenchmarkResultsPrinter {
       for (auto const& name : counterNames_) {
         if (auto ptr = folly::get_ptr(datum.counters, name)) {
           switch (ptr->type) {
+            // UserMetrics constructed as precision_values avoid the
+            // implicit cast from long to double when formatting the output
             case UserMetric::Type::TIME:
-              printf(
-                  "  %*s",
-                  int(name.length()),
-                  readableTime(ptr->value, 2).c_str());
+              folly::variant_match(ptr->value, [&](auto value) {
+                printf(
+                    "  %*s",
+                    int(name.length()),
+                    readableTime(value, 2).c_str());
+              });
               break;
             case UserMetric::Type::METRIC:
-              printf(
-                  "  %*s",
-                  int(name.length()),
-                  metricReadable(ptr->value, 2).c_str());
+              folly::variant_match(ptr->value, [&](auto value) {
+                printf(
+                    "  %*s",
+                    int(name.length()),
+                    metricReadable(value, 2).c_str());
+              });
               break;
             case UserMetric::Type::CUSTOM:
             default:
-              printf("  %*" PRId64, int(name.length()), ptr->value);
+              folly::variant_match(ptr->value, [&](auto value) {
+                printf(
+                    "  %*" PRId64,
+                    int(name.length()),
+                    static_cast<int64_t>(value));
+              });
           }
         } else {
           printf("  %*s", int(name.length()), "NaN");
@@ -577,7 +589,9 @@ void benchmarkResultsToDynamic(
       dynamic obj = dynamic::object;
       for (auto& counter : datum.counters) {
         dynamic counterInfo = dynamic::object;
-        counterInfo["value"] = counter.second.value;
+        folly::variant_match(counter.second.value, [&](auto value) {
+          counterInfo["value"] = value;
+        });
         counterInfo["type"] = static_cast<int>(counter.second.type);
         obj[counter.first] = counterInfo;
       }

third-party/folly/src/folly/Benchmark.h

@@ -35,6 +35,7 @@
 #include <set>
 #include <type_traits>
 #include <unordered_map>
+#include <variant>
 
 #include <boost/function_types/function_arity.hpp>
 #include <glog/logging.h>
@@ -63,20 +64,29 @@ inline bool runBenchmarksOnFlag() {
 class UserMetric {
  public:
   enum class Type { CUSTOM, TIME, METRIC };
-
-  int64_t value{};
+  std::variant<int64_t, double> value;
   Type type{Type::CUSTOM};
 
   UserMetric() = default;
   /* implicit */ UserMetric(int64_t val, Type typ = Type::CUSTOM)
       : value(val), type(typ) {}
 
+  // Allow users to provide precision values
+  template <
+      typename T,
+      typename = std::enable_if_t<std::is_floating_point_v<T>>>
+  explicit UserMetric(T precision_val, Type typ = Type::CUSTOM)
+      : value(convert_helper(precision_val)), type(typ) {}
+
   friend bool operator==(const UserMetric& x, const UserMetric& y) {
     return x.value == y.value && x.type == y.type;
   }
   friend bool operator!=(const UserMetric& x, const UserMetric& y) {
     return !(x == y);
   }
+
+ private:
+  double convert_helper(double val) { return val; }
 };
 
 using UserCounters = std::unordered_map<std::string, UserMetric>;

third-party/folly/src/folly/test/BenchmarkTest.cpp

@@ -349,6 +349,104 @@ TEST_F(BenchmarkingStateTest, ListTests) {
   }
 }
 
+TEST_F(BenchmarkingStateTest, UserMetricInt) {
+  std::string output;
+
+  {
+    addBenchmark(__FILE__, "int_custom", [&](UserCounters& counter) {
+      counter["int_custom_count"] =
+          UserMetric(123456, UserMetric::Type::CUSTOM);
+      doBaseline();
+      TestClock::advance(std::chrono::nanoseconds(1));
+      return 1;
+    });
+
+    ::testing::internal::CaptureStdout();
+    runBenchmarks();
+    output = ::testing::internal::GetCapturedStdout();
+    ASSERT_THAT(output, ::testing::HasSubstr("123456"));
+  }
+
+  {
+    addBenchmark(__FILE__, "int_metric", [&](UserCounters& counter) {
+      counter["int_metric_count"] =
+          UserMetric(234567, UserMetric::Type::METRIC);
+      doBaseline();
+      TestClock::advance(std::chrono::nanoseconds(1));
+      return 1;
+    });
+
+    ::testing::internal::CaptureStdout();
+    runBenchmarks();
+    output = ::testing::internal::GetCapturedStdout();
+    ASSERT_THAT(output, ::testing::HasSubstr("234.57K"));
+  }
+
+  {
+    addBenchmark(__FILE__, "int_time", [&](UserCounters& counter) {
+      counter["int_time_count"] = UserMetric(10, UserMetric::Type::TIME);
+      doBaseline();
+      TestClock::advance(std::chrono::nanoseconds(1));
+      return 1;
+    });
+
+    ::testing::internal::CaptureStdout();
+    runBenchmarks();
+    output = ::testing::internal::GetCapturedStdout();
+    std::cout << output << std::endl;
+    ASSERT_THAT(output, ::testing::HasSubstr("10.00s"));
+  }
+}
+
+TEST_F(BenchmarkingStateTest, UserMetricPrecise) {
+  std::string output;
+
+  {
+    addBenchmark(__FILE__, "double_custom", [&](UserCounters& counter) {
+      counter["double_custom_count"] =
+          UserMetric(1234.56, UserMetric::Type::CUSTOM);
+      doBaseline();
+      TestClock::advance(std::chrono::nanoseconds(1));
+      return 1;
+    });
+
+    ::testing::internal::CaptureStdout();
+    runBenchmarks();
+    output = ::testing::internal::GetCapturedStdout();
+    ASSERT_THAT(output, ::testing::HasSubstr("1234"));
+  }
+
+  {
+    addBenchmark(__FILE__, "double_metric", [&](UserCounters& counter) {
+      counter["double_metric_count"] =
+          UserMetric(23.456, UserMetric::Type::METRIC);
+      doBaseline();
+      TestClock::advance(std::chrono::nanoseconds(1));
+      return 1;
+    });
+
+    ::testing::internal::CaptureStdout();
+    runBenchmarks();
+    output = ::testing::internal::GetCapturedStdout();
+    ASSERT_THAT(output, ::testing::HasSubstr("23.46"));
+  }
+
+  {
+    addBenchmark(__FILE__, "double_time", [&](UserCounters& counter) {
+      counter["double_time_count"] = UserMetric(34.567, UserMetric::Type::TIME);
+      doBaseline();
+      TestClock::advance(std::chrono::nanoseconds(1));
+      return 1;
+    });
+
+    ::testing::internal::CaptureStdout();
+    runBenchmarks();
+    output = ::testing::internal::GetCapturedStdout();
+    std::cout << output << std::endl;
+    ASSERT_THAT(output, ::testing::HasSubstr("34.57s"));
+  }
+}
+
 } // namespace
 } // namespace detail
 } // namespace folly