From d4d49d6c7ab3bf9ae03b0154e0471bfb56e1c2de Mon Sep 17 00:00:00 2001
From: Aaryaman Sagar <aary@instagram.com>
Date: Mon, 20 Nov 2017 15:59:34 -0800
Subject: [PATCH] Split get_default() into two for deferred default
 construction

Summary:
As it stood currently folly::get_default() would unnecessarily
construct a value into the third parameter, which was unnecessary in the fast
path where the element was found in the map

Reviewed By: yfeldblum

Differential Revision: D6366352

fbshipit-source-id: db55b944ca63e565997094c11b90c4ebe98531ce
---
 folly/MapUtil.h            | 27 +++++++++++++++++++------
 folly/test/MapUtilTest.cpp | 40 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 61 insertions(+), 6 deletions(-)

diff --git a/folly/MapUtil.h b/folly/MapUtil.h
index b1fbfadc..b95da417 100644
--- a/folly/MapUtil.h
+++ b/folly/MapUtil.h
@@ -18,6 +18,7 @@
 
 #include <folly/Conv.h>
 #include <folly/Optional.h>
+#include <folly/functional/Invoke.h>
 #include <tuple>
 
 namespace folly {
@@ -26,13 +27,27 @@ namespace folly {
  * Given a map and a key, return the value corresponding to the key in the map,
  * or a given default value if the key doesn't exist in the map.
  */
-template <class Map, typename Key = typename Map::key_type>
-typename Map::mapped_type get_default(
-    const Map& map,
-    const Key& key,
-    const typename Map::mapped_type& dflt = typename Map::mapped_type()) {
+template <typename Map, typename Key>
+typename Map::mapped_type get_default(const Map& map, const Key& key) {
   auto pos = map.find(key);
-  return (pos != map.end() ? pos->second : dflt);
+  return (pos != map.end()) ? (pos->second) : (typename Map::mapped_type{});
+}
+template <
+    class Map,
+    typename Key = typename Map::key_type,
+    typename Value = typename Map::mapped_type,
+    typename std::enable_if<!is_invocable<Value>::value>::type* = nullptr>
+typename Map::mapped_type
+get_default(const Map& map, const Key& key, Value&& dflt) {
+  auto pos = map.find(key);
+  if (pos != map.end()) {
+    return pos->second;
+  } else {
+    // if elision from function parameters was allowed, then we could make the
+    // third parameter a value parameter and just elide that into the return
+    // value, but sadly that is not allowed (yet)
+    return std::forward<Value>(dflt);
+  }
 }
 
 /**
diff --git a/folly/test/MapUtilTest.cpp b/folly/test/MapUtilTest.cpp
index 52e63e81..693f7235 100644
--- a/folly/test/MapUtilTest.cpp
+++ b/folly/test/MapUtilTest.cpp
@@ -16,6 +16,7 @@
 
 #include <folly/MapUtil.h>
 
+#include <cstddef>
 #include <map>
 #include <unordered_map>
 
@@ -238,6 +239,7 @@ struct GetRefDefaultPathCompiles<
         std::declval<int>(),
         std::declval<int>(),
         std::declval<T>()))>> : std::true_type {};
+
 } // namespace
 
 TEST(MapUtil, get_ref_default_path_temporary) {
@@ -246,3 +248,41 @@ TEST(MapUtil, get_ref_default_path_temporary) {
   EXPECT_FALSE(GetRefDefaultPathCompiles<const int&&>::value);
   EXPECT_FALSE(GetRefDefaultPathCompiles<int&&>::value);
 }
+
+namespace {
+
+class TestConstruction {
+ public:
+  static std::size_t numberDefaultConstructs;
+  TestConstruction() {
+    ++numberDefaultConstructs;
+  }
+  TestConstruction(TestConstruction&&) = default;
+  TestConstruction(const TestConstruction&) = default;
+
+  TestConstruction& operator=(const TestConstruction&) = delete;
+  TestConstruction& operator=(TestConstruction&&) = delete;
+};
+
+std::size_t TestConstruction::numberDefaultConstructs = 0;
+
+} // namespace
+
+TEST(MapUtil, test_get_default_deferred_construction) {
+  auto map = std::unordered_map<int, TestConstruction>{};
+  map.insert({1, TestConstruction{}});
+
+  EXPECT_EQ(TestConstruction::numberDefaultConstructs, 1);
+
+  {
+    auto val = get_default(map, 1);
+    EXPECT_EQ(TestConstruction::numberDefaultConstructs, 1);
+    static_cast<void>(val);
+  }
+
+  {
+    auto val = get_default(map, 1, TestConstruction{});
+    EXPECT_EQ(TestConstruction::numberDefaultConstructs, 2);
+    static_cast<void>(val);
+  }
+}
-- 
2.34.1