impl: a benchmark and two small (non-)optimizations for Options (#8487)

devbww · web-flow · commit c6876e24fa1d · 2022-03-02T20:06:19.000-05:00
With the growing ubiquity of `Options`, it seems like a good
idea to keep an eye on its performance.  To that end, add a
micro benchmark which can be expanded as we see fit.  For now,
simply benchmark `get&lt;T&gt;()` performance on both present and
default option types.

Time:
The original motivation for this PR was removing the function-
static synchronization from the "present" path, however the
benchmark indicates that this is only a slight win (&lt;0.5%),
and it seems to cost us a similar amount in the "default"
path.  At least it makes those cases similar in performance,
which is a good thing given that it is unclear which is more
common.  (I guess it is also a good thing to confirm that
function-static synchronization is fast.)

Space:
Template the allocation of the default value for an option on
`FooOption::Type` rather than `FooOption`.  This means, for
example, that we only allocate a single `std::string` default
value for all `std::string`-based options.

Also add a note about how `unordered_map&lt;std::type_index, ...&gt;`
works when indexed by `typeid(T)`.
diff --git a/google/cloud/CMakeLists.txt b/google/cloud/CMakeLists.txt
@@ -177,7 +177,7 @@ function (google_cloud_cpp_common_define_benchmarks)
     find_package(benchmark CONFIG REQUIRED)
 
     set(google_cloud_cpp_common_benchmarks # cmake-format: sort
-    )
+                                           options_benchmark.cc)
 
     # Export the list of benchmarks to a .bzl file so we do not need to maintain
     # the list in two places.
diff --git a/google/cloud/google_cloud_cpp_common_benchmarks.bzl b/google/cloud/google_cloud_cpp_common_benchmarks.bzl
@@ -17,4 +17,5 @@
 """Automatically generated unit tests list - DO NOT EDIT."""
 
 google_cloud_cpp_common_benchmarks = [
+    "options_benchmark.cc",
 ]
diff --git a/google/cloud/options.h b/google/cloud/options.h
@@ -33,6 +33,11 @@ namespace internal {
 Options MergeOptions(Options, Options);
 void CheckExpectedOptionsImpl(std::set<std::type_index> const&, Options const&,
                               char const*);
+template <typename T>
+inline T const& DefaultValue() {
+  static auto const* const kDefaultValue = new T{};
+  return *kDefaultValue;
+}
 }  // namespace internal
 
 /**
@@ -163,9 +168,8 @@ class Options {
    */
   template <typename T>
   ValueTypeT<T> const& get() const {
-    static auto const* const kDefaultValue = new ValueTypeT<T>{};
     auto const it = m_.find(typeid(T));
-    if (it == m_.end()) return *kDefaultValue;
+    if (it == m_.end()) return internal::DefaultValue<ValueTypeT<T>>();
     auto const* value = it->second->data_address();
     return *reinterpret_cast<ValueTypeT<T> const*>(value);
   }
@@ -232,6 +236,9 @@ class Options {
     ValueTypeT<T> value_;
   };
 
+  // Note that (1) `typeid(T)` returns a `std::type_info const&`, but that
+  // implicitly converts to a `std::type_index`, and (2) `std::hash<>` is
+  // specialized for `std::type_index` to use `std::type_index::hash_code()`.
   std::unordered_map<std::type_index, std::unique_ptr<DataHolder>> m_;
 };
 
diff --git a/google/cloud/options_benchmark.cc b/google/cloud/options_benchmark.cc
@@ -0,0 +1,65 @@
+// Copyright 2022 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "google/cloud/options.h"
+#include <benchmark/benchmark.h>
+#include <string>
+
+namespace google {
+namespace cloud {
+GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
+namespace {
+
+// Run on (96 X 2000 MHz CPU s)
+// CPU Caches:
+//   L1 Data 32 KiB (x48)
+//   L1 Instruction 32 KiB (x48)
+//   L2 Unified 1024 KiB (x48)
+//   L3 Unified 39424 KiB (x2)
+// Load Average: 0.49, 0.35, 0.89
+// ----------------------------------------------------------------------
+// Benchmark                            Time             CPU   Iterations
+// ----------------------------------------------------------------------
+// BM_OptionsOneElementDefault       25.6 ns         25.6 ns     27332591
+// BM_OptionsOneElementPresent       28.0 ns         28.0 ns     24999101
+
+struct StringOptionDefault {
+  using Type = std::string;
+};
+struct StringOptionPresent {
+  using Type = std::string;
+};
+
+void BM_OptionsOneElementDefault(benchmark::State& state) {
+  auto const opts = Options{}.set<StringOptionPresent>(
+      "You will do foolish things, but do them with enthusiasm.");
+  for (auto _ : state) {
+    benchmark::DoNotOptimize(opts.get<StringOptionDefault>());
+  }
+}
+BENCHMARK(BM_OptionsOneElementDefault);
+
+void BM_OptionsOneElementPresent(benchmark::State& state) {
+  auto const opts = Options{}.set<StringOptionPresent>(
+      "You will do foolish things, but do them with enthusiasm.");
+  for (auto _ : state) {
+    benchmark::DoNotOptimize(opts.get<StringOptionPresent>());
+  }
+}
+BENCHMARK(BM_OptionsOneElementPresent);
+
+}  // namespace
+GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END
+}  // namespace cloud
+}  // namespace google

Original file line number	Diff line number	Diff line change
`@@ -17,4 +17,5 @@`
`17`	`17`	`"""Automatically generated unit tests list - DO NOT EDIT."""`
`18`	`18`
`19`	`19`	`google_cloud_cpp_common_benchmarks = [`
	`20`	`+ "options_benchmark.cc",`
`20`	`21`	`]`