heap: Generate more GCInfo entries to avoid empty finalizers

Before this patch, in order to support finalization of classes in a
hierarchy, the base class must have had a destructor declared as virtual.
This hurt sweeping time as the sweeper had to execute finalizers for
classes that didn't really have any finalization logic. The patch adds
support for non-virtual destructors in Oilpan that provides the basis
for reduction of finalizers in the followup CLs.

In the backend the logic remained the same for classes that have either
 - virtual Base::~Base or
 - trivial Base::~Base and trivial Derived::~Derived or
 - Base::FinalizeGarbageCollectedObject.
The new logic is:
 - if all of the conditions above don't satisfy, separate GCInfo entries
   will be created for Derived classes.

Bug: 1015427

Change-Id: I99481f0508d99668558283417a15cdf0bfe1ae6f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1866712
Commit-Queue: Anton Bikineev <bikineev@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#707365}

third_party/blink/renderer/core/css/css_value.h

@@ -33,13 +33,14 @@ class Length;
 
 class CORE_EXPORT CSSValue : public GarbageCollected<CSSValue> {
  public:
+  template <typename T>
   static void* AllocateObject(size_t size) {
     ThreadState* state =
         ThreadStateFor<ThreadingTrait<CSSValue>::kAffinity>::GetState();
     const char* type_name = "blink::CSSValue";
     return state->Heap().AllocateOnArenaIndex(
         state, size, BlinkGC::kCSSValueArenaIndex,
-        GCInfoTrait<CSSValue>::Index(), type_name);
+        GCInfoTrait<GCInfoFoldedType<CSSValue>>::Index(), type_name);
   }
 
   // TODO(sashab): Remove this method and move logic to the caller.

third_party/blink/renderer/core/dom/node.h

@@ -163,15 +163,14 @@ class CORE_EXPORT Node : public EventTarget {
     kDocumentPositionImplementationSpecific = 0x20,
   };
 
-  // Override operator new to allocate Node subtype objects onto
-  // a dedicated heap.
+  template <typename T>
   static void* AllocateObject(size_t size) {
     ThreadState* state =
         ThreadStateFor<ThreadingTrait<Node>::kAffinity>::GetState();
     const char* type_name = "blink::Node";
     return state->Heap().AllocateOnArenaIndex(
         state, size, BlinkGC::kNodeArenaIndex,
-        GCInfoTrait<EventTarget>::Index(), type_name);
+        GCInfoTrait<GCInfoFoldedType<T>>::Index(), type_name);
   }
 
   static void DumpStatistics();

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

@@ -38,18 +38,27 @@ template <typename T>
 struct FinalizerTraitImpl<T, true> {
  private:
   STATIC_ONLY(FinalizerTraitImpl);
-  struct CustomDispatch {
+  struct Custom {
     static void Call(void* obj) {
       static_cast<T*>(obj)->FinalizeGarbageCollectedObject();
     }
   };
-  struct DestructorDispatch {
-    static void Call(void* obj) { static_cast<T*>(obj)->~T(); }
+  struct Destructor {
+    static void Call(void* obj) {
+// The garbage collector differs from regular C++ here as it remembers whether
+// an object's base class has a virtual destructor. In case there is no virtual
+// destructor present, the object is always finalized through its leaf type. In
+// other words: there is no finalization through a base pointer.
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wdelete-non-abstract-non-virtual-dtor"
+      static_cast<T*>(obj)->~T();
+#pragma GCC diagnostic pop
+    }
   };
   using FinalizeImpl =
       std::conditional_t<HasFinalizeGarbageCollectedObject<T>::value,
-                         CustomDispatch,
-                         DestructorDispatch>;
+                         Custom,
+                         Destructor>;
 
  public:
   static void Finalize(void* obj) {

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

@@ -93,11 +93,9 @@ class PLATFORM_EXPORT GCInfoTable {
   Mutex table_mutex_;
 };
 
-// GCInfoAtBaseType should be used when returning a unique 14 bit integer
-// for a given gcInfo.
 template <typename T>
-struct GCInfoAtBaseType {
-  STATIC_ONLY(GCInfoAtBaseType);
+struct GCInfoTrait {
+  STATIC_ONLY(GCInfoTrait);
   static uint32_t Index() {
     static_assert(sizeof(T), "T must be fully defined");
     static const GCInfo kGcInfo = {
@@ -117,31 +115,6 @@ struct GCInfoAtBaseType {
   }
 };
 
-template <typename T,
-          bool = WTF::IsSubclassOfTemplate<typename std::remove_const<T>::type,
-                                           GarbageCollected>::value>
-struct GetGarbageCollectedType;
-
-template <typename T>
-struct GetGarbageCollectedType<T, true> {
-  STATIC_ONLY(GetGarbageCollectedType);
-  using type = typename T::GarbageCollectedType;
-};
-
-template <typename T>
-struct GetGarbageCollectedType<T, false> {
-  STATIC_ONLY(GetGarbageCollectedType);
-  using type = T;
-};
-
-template <typename T>
-struct GCInfoTrait {
-  STATIC_ONLY(GCInfoTrait);
-  static uint32_t Index() {
-    return GCInfoAtBaseType<typename GetGarbageCollectedType<T>::type>::Index();
-  }
-};
-
 template <typename U>
 class GCInfoTrait<const U> : public GCInfoTrait<U> {};
 

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

@@ -518,22 +518,48 @@ class GarbageCollected {
 #endif
 
  public:
-  using GarbageCollectedType = T;
+  using ParentMostGarbageCollectedType = T;
 
   void* operator new(size_t size) = delete;  // Must use MakeGarbageCollected.
 
+  template <typename Derived>
   static void* AllocateObject(size_t size) {
     if (IsGarbageCollectedMixin<T>::value) {
       // Ban large mixin so we can use PageFromObject() on them.
       CHECK_GE(kLargeObjectSizeThreshold, size)
           << "GarbageCollectedMixin may not be a large object";
     }
-    return ThreadHeap::Allocate<T>(size);
+    return ThreadHeap::Allocate<GCInfoFoldedType<Derived>>(size);
   }
 
   void operator delete(void* p) { NOTREACHED(); }
 
  protected:
+  // This trait in theory can be moved to gc_info.h, but that would cause
+  // significant memory bloat caused by huge number of ThreadHeap::Allocate<>
+  // instantiations, which linker is not able to fold.
+  template <typename Derived>
+  class GCInfoFolded {
+    static constexpr bool is_virtual_destructor_at_base =
+        std::has_virtual_destructor<ParentMostGarbageCollectedType>::value;
+    static constexpr bool both_trivially_destructible =
+        std::is_trivially_destructible<ParentMostGarbageCollectedType>::value &&
+        std::is_trivially_destructible<Derived>::value;
+    static constexpr bool has_custom_dispatch_at_base =
+        internal::HasFinalizeGarbageCollectedObject<
+            ParentMostGarbageCollectedType>::value;
+
+   public:
+    using Type = std::conditional_t<is_virtual_destructor_at_base ||
+                                        both_trivially_destructible ||
+                                        has_custom_dispatch_at_base,
+                                    ParentMostGarbageCollectedType,
+                                    Derived>;
+  };
+
+  template <typename Derived>
+  using GCInfoFoldedType = typename GCInfoFolded<Derived>::Type;
+
   GarbageCollected() = default;
 
   DISALLOW_COPY_AND_ASSIGN(GarbageCollected);
@@ -552,7 +578,7 @@ T* MakeGarbageCollected(Args&&... args) {
                     internal::HasFinalizeGarbageCollectedObject<T>::value,
                 "Finalized GarbageCollected class should either have a virtual "
                 "destructor or be marked as final.");
-  void* memory = T::AllocateObject(sizeof(T));
+  void* memory = T::template AllocateObject<T>(sizeof(T));
   HeapObjectHeader* header = HeapObjectHeader::FromPayload(memory);
   // Placement new as regular operator new() is deleted.
   T* object = ::new (memory) T(std::forward<Args>(args)...);
@@ -579,7 +605,8 @@ T* MakeGarbageCollected(AdditionalBytes additional_bytes, Args&&... args) {
                     internal::HasFinalizeGarbageCollectedObject<T>::value,
                 "Finalized GarbageCollected class should either have a virtual "
                 "destructor or be marked as final.");
-  void* memory = T::AllocateObject(sizeof(T) + additional_bytes.value);
+  void* memory =
+      T::template AllocateObject<T>(sizeof(T) + additional_bytes.value);
   HeapObjectHeader* header = HeapObjectHeader::FromPayload(memory);
   // Placement new as regular operator new() is deleted.
   T* object = ::new (memory) T(std::forward<Args>(args)...);

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

@@ -510,6 +510,7 @@ class HeapHashMap : public HashMap<KeyArg,
   }
 
  public:
+  template <typename>
   static void* AllocateObject(size_t size) {
     return ThreadHeap::Allocate<
         HeapHashMap<KeyArg, MappedArg, HashArg, KeyTraitsArg, MappedTraitsArg>>(
@@ -539,6 +540,7 @@ class HeapHashSet
   }
 
  public:
+  template <typename>
   static void* AllocateObject(size_t size) {
     return ThreadHeap::Allocate<HeapHashSet<ValueArg, HashArg, TraitsArg>>(
         size);
@@ -568,6 +570,7 @@ class HeapLinkedHashSet
   }
 
  public:
+  template <typename>
   static void* AllocateObject(size_t size) {
     return ThreadHeap::Allocate<
         HeapLinkedHashSet<ValueArg, HashArg, TraitsArg>>(size);
@@ -601,6 +604,7 @@ class HeapListHashSet
   }
 
  public:
+  template <typename>
   static void* AllocateObject(size_t size) {
     return ThreadHeap::Allocate<
         HeapListHashSet<ValueArg, inlineCapacity, HashArg>>(size);
@@ -629,6 +633,7 @@ class HeapHashCountedSet
   }
 
  public:
+  template <typename>
   static void* AllocateObject(size_t size) {
     return ThreadHeap::Allocate<
         HeapHashCountedSet<Value, HashFunctions, Traits>>(size);
@@ -655,6 +660,7 @@ class HeapVector : public Vector<T, inlineCapacity, HeapAllocator> {
   }
 
  public:
+  template <typename>
   static void* AllocateObject(size_t size) {
     // On-heap HeapVectors generally should not have inline capacity, but it is
     // hard to avoid when using a type alias. Hence we only disallow the
@@ -706,6 +712,7 @@ class HeapDeque : public Deque<T, inlineCapacity, HeapAllocator> {
   }
 
  public:
+  template <typename>
   static void* AllocateObject(size_t size) {
     // On-heap HeapDeques generally should not have inline capacity, but it is
     // hard to avoid when using a type alias. Hence we only disallow the

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

@@ -6100,4 +6100,24 @@ TEST_F(HeapTest, AccessDeletedBackingStore) {
   }
 }
 
+class GCBase : public GarbageCollected<GCBase> {
+ public:
+  virtual void Trace(Visitor*) {}
+};
+
+class GCDerived final : public GCBase {
+ public:
+  static int destructor_called;
+  void Trace(Visitor*) override {}
+  ~GCDerived() { ++destructor_called; }
+};
+
+int GCDerived::destructor_called = 0;
+
+TEST_F(HeapTest, CallMostDerivedFinalizer) {
+  MakeGarbageCollected<GCDerived>();
+  PreciselyCollectGarbage();
+  EXPECT_EQ(1, GCDerived::destructor_called);
+}
+
 }  // namespace blink