[PATCH] D16360: unordered_map: Avoid unnecessary mallocs when no insert occurs

[Eric Fiselier via cfe-commits]

EricWF added a comment.

Overall this patch is *almost* there.  My only objection is that this optimization neglects "unordered_set". Optimally we would also catch "unordered_set<int>.emplace(42)"


================
Comment at: include/__hash_table:103
@@ -102,1 +102,3 @@
 
+template <class _Pair, class _Key,
+          class _RawPair = typename __uncvref<_Pair>::type>
----------------
The traits LGTM.

================
Comment at: include/__hash_table:1062
@@ +1061,3 @@
+      return __emplace_unique_extract_key(_VSTD::forward<_Pp>(__x),
+                                          __is_pair_with_key<_Pp, key_type>());
+    }
----------------
It took me a minute to understand why this doesn't break `std::unordered_set<std::pair<T, U>>`, could you make either the trait name, or implementation more explicit about the fact that it only ever returns true for "unordered_map"?

================
Comment at: test/libcxx/containers/unord/unord.map/insert_dup_alloc.pass.cpp:23
@@ +22,3 @@
+
+    for (int i = 0; i < 100; ++i) {
+      std::pair<const int, int> P(3, i);
----------------
I don't think we need to test 100 different values for the mapped type. Also you should consider using 'DisableAllocationGuard' instead of 'globalMemCounter` directly. For example:

```
Container c;
c.insert(std::make_pair(1, 1)); // Don't worry about the allocation behavior here.
{
  DisableAllocationGuard g;
  // test duplicates that should not allocate in this scope.
}
```

================
Comment at: test/libcxx/containers/unord/unord.map/insert_dup_alloc.pass.cpp:29
@@ +28,3 @@
+      s.insert(s.end(), std::make_pair(3, i));
+#ifndef _LIBCPP_HAS_NO_RVALUE_REFERENCES
+      s.emplace(std::make_pair(3, i));
----------------
Please use "TEST_STD_VER >= 11" instead. The macro is defined it "test_macros.h".


http://reviews.llvm.org/D16360