From 306f83119b404eecaf28697e81fa8360a5237e5f Mon Sep 17 00:00:00 2001
From: leszeks <leszeks@chromium.org>
Date: Mon, 3 Oct 2016 08:07:13 -0700
Subject: [PATCH] [base] Optimise hashmaps with simple key equality

Hashmaps with a simple key equality method (comparing pointers) don't
need to waste cycles (and branches) comparing hash values, as the key
comparison is cheap.

This patch modifies the hashmap's MatchFun to take the hashes as well as
the keys, thus allowing the MatchFun to ignore the hashes. This allows
slightly cleaner generated code, especially when the MatchFun is
inlined.

BUG=

Review-Url: https://codereview.chromium.org/2381303002
Cr-Commit-Position: refs/heads/master@{#39932}
---
 src/base/hashmap.h                        | 60 ++++++++++++++++++-----
 src/interpreter/constant-array-builder.cc |  3 +-
 src/interpreter/constant-array-builder.h  |  2 +-
 3 files changed, 51 insertions(+), 14 deletions(-)

diff --git a/src/base/hashmap.h b/src/base/hashmap.h
index d8151861cb..54038c5ef3 100644
--- a/src/base/hashmap.h
+++ b/src/base/hashmap.h
@@ -269,7 +269,7 @@ TemplateHashMapImpl<Key, Value, MatchFun, AllocationPolicy>::Probe(
   DCHECK(map_ <= entry && entry < end);
 
   DCHECK(occupancy_ < capacity_);  // Guarantees loop termination.
-  while (entry->exists() && (hash != entry->hash || !match_(key, entry->key))) {
+  while (entry->exists() && !match_(hash, entry->hash, key, entry->key)) {
     entry++;
     if (entry >= end) {
       entry = map_;
@@ -337,12 +337,31 @@ void TemplateHashMapImpl<Key, Value, MatchFun, AllocationPolicy>::Resize(
   AllocationPolicy::Delete(map);
 }
 
+// Match function which compares hashes before executing a (potentially
+// expensive) key comparison.
+template <typename Key, typename MatchFun>
+struct HashEqualityThenKeyMatcher {
+  explicit HashEqualityThenKeyMatcher(MatchFun match) : match_(match) {}
+
+  bool operator()(uint32_t hash1, uint32_t hash2, const Key& key1,
+                  const Key& key2) const {
+    return hash1 == hash2 && match_(key1, key2);
+  }
+
+ private:
+  MatchFun match_;
+};
+
+// Hashmap<void*, void*> which takes a custom key comparison function pointer.
 template <typename AllocationPolicy>
 class CustomMatcherTemplateHashMapImpl
-    : public TemplateHashMapImpl<void*, void*, bool (*)(void*, void*),
-                                 AllocationPolicy> {
-  typedef TemplateHashMapImpl<void*, void*, bool (*)(void*, void*),
-                              AllocationPolicy>
+    : public TemplateHashMapImpl<
+          void*, void*,
+          HashEqualityThenKeyMatcher<void*, bool (*)(void*, void*)>,
+          AllocationPolicy> {
+  typedef TemplateHashMapImpl<
+      void*, void*, HashEqualityThenKeyMatcher<void*, bool (*)(void*, void*)>,
+      AllocationPolicy>
       Base;
 
  public:
@@ -351,24 +370,35 @@ class CustomMatcherTemplateHashMapImpl
   CustomMatcherTemplateHashMapImpl(
       MatchFun match, uint32_t capacity = Base::kDefaultHashMapCapacity,
       AllocationPolicy allocator = AllocationPolicy())
-      : Base(capacity, match, allocator) {}
+      : Base(capacity, HashEqualityThenKeyMatcher<void*, MatchFun>(match),
+             allocator) {}
 };
 
 typedef CustomMatcherTemplateHashMapImpl<DefaultAllocationPolicy>
     CustomMatcherHashMap;
 
+// Match function which compares keys directly by equality.
+template <typename Key>
+struct KeyEqualityMatcher {
+  bool operator()(uint32_t hash1, uint32_t hash2, const Key& key1,
+                  const Key& key2) const {
+    return key1 == key2;
+  }
+};
+
+// Hashmap<void*, void*> which compares the key pointers directly.
 template <typename AllocationPolicy>
 class PointerTemplateHashMapImpl
-    : public TemplateHashMapImpl<void*, void*, std::equal_to<void*>,
+    : public TemplateHashMapImpl<void*, void*, KeyEqualityMatcher<void*>,
                                  AllocationPolicy> {
-  typedef TemplateHashMapImpl<void*, void*, std::equal_to<void*>,
+  typedef TemplateHashMapImpl<void*, void*, KeyEqualityMatcher<void*>,
                               AllocationPolicy>
       Base;
 
  public:
   PointerTemplateHashMapImpl(uint32_t capacity = Base::kDefaultHashMapCapacity,
                              AllocationPolicy allocator = AllocationPolicy())
-      : Base(capacity, std::equal_to<void*>(), allocator) {}
+      : Base(capacity, KeyEqualityMatcher<void*>(), allocator) {}
 };
 
 typedef PointerTemplateHashMapImpl<DefaultAllocationPolicy> HashMap;
@@ -376,8 +406,13 @@ typedef PointerTemplateHashMapImpl<DefaultAllocationPolicy> HashMap;
 // A hash map for pointer keys and values with an STL-like interface.
 template <class Key, class Value, class MatchFun, class AllocationPolicy>
 class TemplateHashMap
-    : private TemplateHashMapImpl<void*, void*, MatchFun, AllocationPolicy> {
-  typedef TemplateHashMapImpl<void*, void*, MatchFun, AllocationPolicy> Base;
+    : private TemplateHashMapImpl<void*, void*,
+                                  HashEqualityThenKeyMatcher<void*, MatchFun>,
+                                  AllocationPolicy> {
+  typedef TemplateHashMapImpl<void*, void*,
+                              HashEqualityThenKeyMatcher<void*, MatchFun>,
+                              AllocationPolicy>
+      Base;
 
  public:
   STATIC_ASSERT(sizeof(Key*) == sizeof(void*));    // NOLINT
@@ -409,7 +444,8 @@ class TemplateHashMap
 
   TemplateHashMap(MatchFun match,
                   AllocationPolicy allocator = AllocationPolicy())
-      : Base(Base::kDefaultHashMapCapacity, match, allocator) {}
+      : Base(Base::kDefaultHashMapCapacity,
+             HashEqualityThenKeyMatcher<void*, MatchFun>(match), allocator) {}
 
   Iterator begin() const { return Iterator(this, this->Start()); }
   Iterator end() const { return Iterator(this, nullptr); }
diff --git a/src/interpreter/constant-array-builder.cc b/src/interpreter/constant-array-builder.cc
index 145d2d228c..d2b7995623 100644
--- a/src/interpreter/constant-array-builder.cc
+++ b/src/interpreter/constant-array-builder.cc
@@ -73,7 +73,8 @@ STATIC_CONST_MEMBER_DEFINITION const size_t
 
 ConstantArrayBuilder::ConstantArrayBuilder(Zone* zone,
                                            Handle<Object> the_hole_value)
-    : constants_map_(16, std::equal_to<Address>(), ZoneAllocationPolicy(zone)),
+    : constants_map_(16, base::KeyEqualityMatcher<Address>(),
+                     ZoneAllocationPolicy(zone)),
       smi_map_(zone),
       smi_pairs_(zone),
       zone_(zone),
diff --git a/src/interpreter/constant-array-builder.h b/src/interpreter/constant-array-builder.h
index f0c4eae64d..78d36f5044 100644
--- a/src/interpreter/constant-array-builder.h
+++ b/src/interpreter/constant-array-builder.h
@@ -107,7 +107,7 @@ class ConstantArrayBuilder final BASE_EMBEDDED {
   Handle<Object> the_hole_value() const { return the_hole_value_; }
 
   ConstantArraySlice* idx_slice_[3];
-  base::TemplateHashMapImpl<Address, index_t, std::equal_to<Address>,
+  base::TemplateHashMapImpl<Address, index_t, base::KeyEqualityMatcher<Address>,
                             ZoneAllocationPolicy>
       constants_map_;
   ZoneMap<Smi*, index_t> smi_map_;