Added a class |HeapNewLinkedHashSet|

Implemented |HeapNewLinkedHashSet|, which inherits from NewLinkedHashSet.
Along with this change, added allocator to template parameter of NewLinkedHashSet and VectorBackedLinkedList.
Also added a test for it.

Change-Id: I277b8542924d52c1fa4eb7815f2feac0074ed4c4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2105667
Commit-Queue: Kei Nakashima <keinakashima@google.com>
Reviewed-by: Bartek Nowierski <bartekn@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Reviewed-by: Keishi Hattori <keishi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#751638}

third_party/blink/renderer/platform/heap/concurrent_marking_test.cc

@@ -220,6 +220,37 @@ TEST_F(ConcurrentMarkingTest, SwapLinkedHashSet) {
   SwapCollections<HeapLinkedHashSetAdapter<Member<IntegerObject>>>();
 }
 
+// HeapNewLinkedHashSet
+template <typename T>
+class HeapNewLinkedHashSetAdapter : public HeapNewLinkedHashSet<T> {
+ public:
+  ALWAYS_INLINE void swap(HeapNewLinkedHashSetAdapter<T>& other) {
+    HeapNewLinkedHashSet<T>::Swap(other);
+  }
+};
+
+TEST_F(ConcurrentMarkingTest, AddToNewLinkedHashSet) {
+  AddToCollection<HeapNewLinkedHashSetAdapter<Member<IntegerObject>>>();
+}
+TEST_F(ConcurrentMarkingTest, RemoveFromBeginningOfNewLinkedHashSet) {
+  RemoveFromBeginningOfCollection<
+      HeapNewLinkedHashSetAdapter<Member<IntegerObject>>>();
+}
+TEST_F(ConcurrentMarkingTest, RemoveFromMiddleOfNewLinkedHashSet) {
+  RemoveFromMiddleOfCollection<
+      HeapNewLinkedHashSetAdapter<Member<IntegerObject>>>();
+}
+TEST_F(ConcurrentMarkingTest, RemoveFromEndOfNewLinkedHashSet) {
+  RemoveFromEndOfCollection<
+      HeapNewLinkedHashSetAdapter<Member<IntegerObject>>>();
+}
+TEST_F(ConcurrentMarkingTest, ClearNewLinkedHashSet) {
+  ClearCollection<HeapNewLinkedHashSetAdapter<Member<IntegerObject>>>();
+}
+TEST_F(ConcurrentMarkingTest, SwapNewLinkedHashSet) {
+  SwapCollections<HeapNewLinkedHashSetAdapter<Member<IntegerObject>>>();
+}
+
 // HeapListHashSet
 
 template <typename T>

third_party/blink/renderer/platform/heap/heap_allocator.h

@@ -511,6 +511,41 @@ template <typename T, typename U, typename V>
 struct GCInfoTrait<HeapLinkedHashSet<T, U, V>>
     : public GCInfoTrait<LinkedHashSet<T, U, V, HeapAllocator>> {};
 
+// This class is still experimental. Do not use this class.
+template <typename ValueArg>
+class HeapNewLinkedHashSet : public NewLinkedHashSet<ValueArg, HeapAllocator> {
+  IS_GARBAGE_COLLECTED_CONTAINER_TYPE();
+  DISALLOW_NEW();
+
+  static void CheckType() {
+    // TODO(keinakashima): support WeakMember<T>
+    static_assert(internal::IsMember<ValueArg>,
+                  "HeapNewLinkedHashSet supports only Member.");
+    // If not trivially destructible, we have to add a destructor which will
+    // hinder performance.
+    static_assert(std::is_trivially_destructible<HeapNewLinkedHashSet>::value,
+                  "HeapNewLinkedHashSet must be trivially destructible.");
+    static_assert(
+        IsAllowedInContainer<ValueArg>::value,
+        "Not allowed to directly nest type. Use Member<> indirection instead.");
+    static_assert(WTF::IsTraceable<ValueArg>::value,
+                  "For sets without traceable elements, use NewLinkedHashSet<> "
+                  "instead of HeapNewLinkedHashSet<>.");
+  }
+
+ public:
+  template <typename>
+  static void* AllocateObject(size_t size) {
+    return ThreadHeap::Allocate<HeapNewLinkedHashSet<ValueArg>>(size);
+  }
+
+  HeapNewLinkedHashSet() { CheckType(); }
+};
+
+template <typename T>
+struct GCInfoTrait<HeapNewLinkedHashSet<T>>
+    : public GCInfoTrait<NewLinkedHashSet<T, HeapAllocator>> {};
+
 template <typename ValueArg,
           wtf_size_t inlineCapacity =
               0,  // The inlineCapacity is just a dummy to

third_party/blink/renderer/platform/heap/heap_compact_test.cc

@@ -337,6 +337,135 @@ TEST_F(HeapCompactTest, CompactLinkedHashSetNested) {
   }
 }
 
+TEST_F(HeapCompactTest, CompactNewLinkedHashSet) {
+  using OrderedHashSet = HeapNewLinkedHashSet<Member<IntWrapper>>;
+  Persistent<OrderedHashSet> set = MakeGarbageCollected<OrderedHashSet>();
+  for (int i = 0; i < 13; ++i) {
+    IntWrapper* value = IntWrapper::Create(i, HashTablesAreCompacted);
+    set->insert(value);
+  }
+  EXPECT_EQ(13u, set->size());
+
+  int expected = 0;
+  for (IntWrapper* v : *set) {
+    EXPECT_EQ(expected, v->Value());
+    expected++;
+  }
+
+  for (int i = 1; i < 13; i += 2) {
+    auto it = set->begin();
+    for (int j = 0; j < (i + 1) / 2; ++j) {
+      ++it;
+    }
+    set->erase(it);
+  }
+  EXPECT_EQ(7u, set->size());
+
+  expected = 0;
+  for (IntWrapper* v : *set) {
+    EXPECT_EQ(expected, v->Value());
+    expected += 2;
+  }
+
+  PerformHeapCompaction();
+  EXPECT_TRUE(IntWrapper::did_verify_at_least_once);
+
+  expected = 0;
+  for (IntWrapper* v : *set) {
+    EXPECT_EQ(expected, v->Value());
+    expected += 2;
+  }
+  EXPECT_EQ(7u, set->size());
+}
+
+TEST_F(HeapCompactTest, CompactNewLinkedHashSetVector) {
+  using OrderedHashSet = HeapNewLinkedHashSet<Member<IntVector>>;
+  Persistent<OrderedHashSet> set = MakeGarbageCollected<OrderedHashSet>();
+  for (int i = 0; i < 13; ++i) {
+    IntWrapper* value = IntWrapper::Create(i);
+    IntVector* vector = MakeGarbageCollected<IntVector>(19, value);
+    set->insert(vector);
+  }
+  EXPECT_EQ(13u, set->size());
+
+  int expected = 0;
+  for (IntVector* v : *set) {
+    EXPECT_EQ(expected, (*v)[0]->Value());
+    expected++;
+  }
+
+  PerformHeapCompaction();
+  EXPECT_TRUE(IntWrapper::did_verify_at_least_once);
+
+  expected = 0;
+  for (IntVector* v : *set) {
+    EXPECT_EQ(expected, (*v)[0]->Value());
+    expected++;
+  }
+}
+
+TEST_F(HeapCompactTest, CompactNewLinkedHashSetMap) {
+  using Inner = HeapHashSet<Member<IntWrapper>>;
+  using OrderedHashSet = HeapNewLinkedHashSet<Member<Inner>>;
+
+  Persistent<OrderedHashSet> set = MakeGarbageCollected<OrderedHashSet>();
+  for (int i = 0; i < 13; ++i) {
+    IntWrapper* value = IntWrapper::Create(i);
+    Inner* inner = MakeGarbageCollected<Inner>();
+    inner->insert(value);
+    set->insert(inner);
+  }
+  EXPECT_EQ(13u, set->size());
+
+  int expected = 0;
+  for (const Inner* v : *set) {
+    EXPECT_EQ(1u, v->size());
+    EXPECT_EQ(expected, (*v->begin())->Value());
+    expected++;
+  }
+
+  PerformHeapCompaction();
+  EXPECT_TRUE(IntWrapper::did_verify_at_least_once);
+
+  expected = 0;
+  for (const Inner* v : *set) {
+    EXPECT_EQ(1u, v->size());
+    EXPECT_EQ(expected, (*v->begin())->Value());
+    expected++;
+  }
+}
+
+TEST_F(HeapCompactTest, CompactNewLinkedHashSetNested) {
+  using Inner = HeapNewLinkedHashSet<Member<IntWrapper>>;
+  using OrderedHashSet = HeapNewLinkedHashSet<Member<Inner>>;
+
+  Persistent<OrderedHashSet> set = MakeGarbageCollected<OrderedHashSet>();
+  for (int i = 0; i < 13; ++i) {
+    IntWrapper* value = IntWrapper::Create(i);
+    Inner* inner = MakeGarbageCollected<Inner>();
+    inner->insert(value);
+    set->insert(inner);
+  }
+  EXPECT_EQ(13u, set->size());
+
+  int expected = 0;
+  for (const Inner* v : *set) {
+    EXPECT_EQ(1u, v->size());
+    EXPECT_EQ(expected, (*v->begin())->Value());
+    expected++;
+  }
+
+  PerformHeapCompaction();
+  EXPECT_TRUE(IntWrapper::did_verify_at_least_once);
+
+  expected = 0;
+  for (const Inner* v : *set) {
+    EXPECT_EQ(1u, v->size());
+    EXPECT_EQ(expected, (*v->begin())->Value());
+    expected++;
+  }
+}
+
 TEST_F(HeapCompactTest, CompactInlinedBackingStore) {
   // Regression test: https://crbug.com/875044
   //

third_party/blink/renderer/platform/heap/heap_test.cc

@@ -3128,6 +3128,9 @@ TEST_F(HeapTest, HeapWeakLinkedHashSet) {
   OrderedSetHelper<HeapLinkedHashSet<WeakMember<IntWrapper>>>(false);
   ClearOutOldGarbage();
   OrderedSetHelper<HeapListHashSet<Member<IntWrapper>>>(true);
+  ClearOutOldGarbage();
+  // TODO(keinakashiima): add a test case for WeakMember once it's supported
+  OrderedSetHelper<HeapNewLinkedHashSet<Member<IntWrapper>>>(true);
 }
 
 class ThingWithDestructor {
@@ -5223,6 +5226,9 @@ TEST_F(HeapTest, IsGarbageCollected) {
   static_assert(
       WTF::IsGarbageCollectedType<HeapLinkedHashSet<Member<IntWrapper>>>::value,
       "HeapLinkedHashSet");
+  static_assert(WTF::IsGarbageCollectedType<
+                    HeapNewLinkedHashSet<Member<IntWrapper>>>::value,
+                "HeapNewLinkedHashSet");
   static_assert(
       WTF::IsGarbageCollectedType<HeapListHashSet<Member<IntWrapper>>>::value,
       "HeapListHashSet");

third_party/blink/renderer/platform/heap/incremental_marking_test.cc

@@ -961,6 +961,9 @@ TEST_F(IncrementalMarkingTest, HeapLinkedHashSetSwap) {
   Swap<HeapLinkedHashSet<WeakMember<Object>>>();
 }
 
+// TODO(keinakashima): add tests for NewLinkedHashSet after supporting
+// WeakMember
+
 // =============================================================================
 // HeapHashCountedSet support. =================================================
 // =============================================================================

third_party/blink/renderer/platform/heap/weakness_marking_test.cc

@@ -198,6 +198,8 @@ TEST_F(WeaknessMarkingTest, TracableEphemeronIsRegsitered) {
   EXPECT_NE(old_ephemeron_count, ephemeron_count);
 }
 
+// TODO(keinakashima): add tests for NewLinkedHashSet after supporting
+// WeakMember
 TEST_F(WeaknessMarkingTest, SwapIntoAlreadyProcessedWeakSet) {
   // Regression test: https://crbug.com/1038623
   //

third_party/blink/renderer/platform/wtf/linked_hash_set.h

@@ -1029,7 +1029,7 @@ inline void swap(LinkedHashSetNode<T>& a, LinkedHashSetNode<T>& b) {
 // TODO(keinakashima): replace existing LinkedHashSet with NewLinkedHashSet
 // after completion
 
-// This class is yet experimental. Do not use this class.
+// This class is still experimental. Do not use this class.
 
 // LinkedHashSet provides a Set interface like HashSet, but also has a
 // predictable iteration order. It has O(1) insertion, removal, and test for
@@ -1041,18 +1041,25 @@ inline void swap(LinkedHashSetNode<T>& a, LinkedHashSetNode<T>& b) {
 // TODO(keinakashima): implement NewLinkedHashTraits (now we cannot insert
 // deleted/empty value) and add it to template parameter
 
-template <typename ValueArg>
+template <typename ValueArg, typename Allocator = PartitionAllocator>
 class NewLinkedHashSet {
-  USING_FAST_MALLOC(NewLinkedHashSet);
+  USE_ALLOCATOR(NewLinkedHashSet, Allocator);
 
  private:
   using Value = ValueArg;
-  using Map = HashMap<Value, wtf_size_t>;
+  using Map = HashMap<Value,
+                      wtf_size_t,
+                      typename DefaultHash<Value>::Hash,
+                      HashTraits<Value>,
+                      HashTraits<wtf_size_t>,
+                      Allocator>;
+  using ListType = VectorBackedLinkedList<Value, Allocator>;
 
  public:
-  using const_iterator = typename VectorBackedLinkedList<Value>::const_iterator;
-  using const_reverse_iterator =
-      typename VectorBackedLinkedList<Value>::const_reverse_iterator;
+  using iterator = typename ListType::const_iterator;
+  using reverse_iterator = typename ListType::const_reverse_iterator;
+  using const_iterator = typename ListType::const_iterator;
+  using const_reverse_iterator = typename ListType::const_reverse_iterator;
 
   // TODO(keinakashima): add security check
   struct AddResult final {
@@ -1117,7 +1124,11 @@ class NewLinkedHashSet {
     list_.clear();
   }
 
-  // TODO(keinakashima): implement Trace
+  template <typename VisitorDispatcher, typename A = Allocator>
+  std::enable_if_t<A::kIsGarbageCollected> Trace(VisitorDispatcher visitor) {
+    value_to_index_.Trace(visitor);
+    list_.Trace(visitor);
+  }
 
  private:
   enum class MoveType {
@@ -1128,47 +1139,52 @@ class NewLinkedHashSet {
   template <typename IncomingValueType>
   AddResult InsertOrMoveBefore(const_iterator, IncomingValueType&&, MoveType);
 
-  HashMap<Value, wtf_size_t> value_to_index_;
-  VectorBackedLinkedList<Value> list_;
+  Map value_to_index_;
+  ListType list_;
 };
 
-template <typename T>
-NewLinkedHashSet<T>::NewLinkedHashSet() {
-  // TODO(keinakashima): add assertion when considering GC
+template <typename T, typename Allocator>
+NewLinkedHashSet<T, Allocator>::NewLinkedHashSet() {
+  static_assert(Allocator::kIsGarbageCollected ||
+                    !IsPointerToGarbageCollectedType<T>::value,
+                "Cannot put raw pointers to garbage-collected classes into "
+                "an off-heap NewLinkedHashSet. Use "
+                "HeapNewLinkedHashSet<Member<T>> instead.");
 }
 
 // TODO(keinakashima): add copy constructor after implementing iterator if
 // anybody uses it.
 
-template <typename T>
-inline NewLinkedHashSet<T>::NewLinkedHashSet(NewLinkedHashSet&& other) {
+template <typename T, typename Allocator>
+inline NewLinkedHashSet<T, Allocator>::NewLinkedHashSet(
+    NewLinkedHashSet&& other) {
   Swap(other);
 }
 
-template <typename T>
-inline NewLinkedHashSet<T>& NewLinkedHashSet<T>::operator=(
-    const NewLinkedHashSet& other) {
+template <typename T, typename Allocator>
+inline NewLinkedHashSet<T, Allocator>& NewLinkedHashSet<T, Allocator>::
+operator=(const NewLinkedHashSet& other) {
   NewLinkedHashSet tmp(other);
   Swap(tmp);
   return *this;
 }
 
-template <typename T>
-inline NewLinkedHashSet<T>& NewLinkedHashSet<T>::operator=(
-    NewLinkedHashSet&& other) {
+template <typename T, typename Allocator>
+inline NewLinkedHashSet<T, Allocator>& NewLinkedHashSet<T, Allocator>::
+operator=(NewLinkedHashSet&& other) {
   Swap(other);
   return *this;
 }
 
-template <typename T>
-inline void NewLinkedHashSet<T>::Swap(NewLinkedHashSet& other) {
+template <typename T, typename Allocator>
+inline void NewLinkedHashSet<T, Allocator>::Swap(NewLinkedHashSet& other) {
   value_to_index_.swap(other.value_to_index_);
   list_.swap(other.list_);
 }
 
-template <typename T>
-typename NewLinkedHashSet<T>::const_iterator NewLinkedHashSet<T>::find(
-    ValuePeekInType value) const {
+template <typename T, typename Allocator>
+typename NewLinkedHashSet<T, Allocator>::const_iterator
+NewLinkedHashSet<T, Allocator>::find(ValuePeekInType value) const {
   typename Map::const_iterator it = value_to_index_.find(value);
 
   if (it == value_to_index_.end())
@@ -1176,76 +1192,77 @@ typename NewLinkedHashSet<T>::const_iterator NewLinkedHashSet<T>::find(
   return list_.MakeConstIterator(it->value);
 }
 
-template <typename T>
-bool NewLinkedHashSet<T>::Contains(ValuePeekInType value) const {
+template <typename T, typename Allocator>
+bool NewLinkedHashSet<T, Allocator>::Contains(ValuePeekInType value) const {
   return value_to_index_.Contains(value);
 }
 
-template <typename T>
+template <typename T, typename Allocator>
 template <typename IncomingValueType>
-typename NewLinkedHashSet<T>::AddResult NewLinkedHashSet<T>::insert(
-    IncomingValueType&& value) {
+typename NewLinkedHashSet<T, Allocator>::AddResult
+NewLinkedHashSet<T, Allocator>::insert(IncomingValueType&& value) {
   return InsertOrMoveBefore(end(), std::forward<IncomingValueType>(value),
                             MoveType::kDontMove);
 }
 
-template <typename T>
+template <typename T, typename Allocator>
 template <typename IncomingValueType>
-typename NewLinkedHashSet<T>::AddResult NewLinkedHashSet<T>::InsertBefore(
-    ValuePeekInType before_value,
-    IncomingValueType&& value) {
+typename NewLinkedHashSet<T, Allocator>::AddResult
+NewLinkedHashSet<T, Allocator>::InsertBefore(ValuePeekInType before_value,
+                                             IncomingValueType&& value) {
   return InsertOrMoveBefore(find(before_value),
                             std::forward<IncomingValueType>(value),
                             MoveType::kDontMove);
 }
 
-template <typename T>
+template <typename T, typename Allocator>
 template <typename IncomingValueType>
-typename NewLinkedHashSet<T>::AddResult NewLinkedHashSet<T>::AppendOrMoveToLast(
-    IncomingValueType&& value) {
+typename NewLinkedHashSet<T, Allocator>::AddResult
+NewLinkedHashSet<T, Allocator>::AppendOrMoveToLast(IncomingValueType&& value) {
   return InsertOrMoveBefore(end(), std::forward<IncomingValueType>(value),
                             MoveType::kMoveIfValueExists);
 }
 
-template <typename T>
+template <typename T, typename Allocator>
 template <typename IncomingValueType>
-typename NewLinkedHashSet<T>::AddResult
-NewLinkedHashSet<T>::PrependOrMoveToFirst(IncomingValueType&& value) {
+typename NewLinkedHashSet<T, Allocator>::AddResult
+NewLinkedHashSet<T, Allocator>::PrependOrMoveToFirst(
+    IncomingValueType&& value) {
   return InsertOrMoveBefore(begin(), std::forward<IncomingValueType>(value),
                             MoveType::kMoveIfValueExists);
 }
 
-template <typename T>
-inline void NewLinkedHashSet<T>::erase(ValuePeekInType value) {
+template <typename T, typename Allocator>
+inline void NewLinkedHashSet<T, Allocator>::erase(ValuePeekInType value) {
   erase(find(value));
 }
 
-template <typename T>
-inline void NewLinkedHashSet<T>::erase(const_iterator it) {
+template <typename T, typename Allocator>
+inline void NewLinkedHashSet<T, Allocator>::erase(const_iterator it) {
   if (it == end())
     return;
   value_to_index_.erase(*it);
   list_.erase(it);
 }
 
-template <typename T>
-inline void NewLinkedHashSet<T>::RemoveFirst() {
+template <typename T, typename Allocator>
+inline void NewLinkedHashSet<T, Allocator>::RemoveFirst() {
   DCHECK(!IsEmpty());
   erase(begin());
 }
 
-template <typename T>
-inline void NewLinkedHashSet<T>::pop_back() {
+template <typename T, typename Allocator>
+inline void NewLinkedHashSet<T, Allocator>::pop_back() {
   DCHECK(!IsEmpty());
   erase(--end());
 }
 
-template <typename T>
+template <typename T, typename Allocator>
 template <typename IncomingValueType>
-typename NewLinkedHashSet<T>::AddResult NewLinkedHashSet<T>::InsertOrMoveBefore(
-    const_iterator position,
-    IncomingValueType&& value,
-    MoveType type) {
+typename NewLinkedHashSet<T, Allocator>::AddResult
+NewLinkedHashSet<T, Allocator>::InsertOrMoveBefore(const_iterator position,
+                                                   IncomingValueType&& value,
+                                                   MoveType type) {
   typename Map::AddResult result = value_to_index_.insert(value, kNotFound);
 
   if (result.is_new_entry) {
@@ -1268,5 +1285,6 @@ typename NewLinkedHashSet<T>::AddResult NewLinkedHashSet<T>::InsertOrMoveBefore(
 }  // namespace WTF
 
 using WTF::LinkedHashSet;
+using WTF::NewLinkedHashSet;
 
 #endif /* WTF_LinkedHashSet_h */