heap: Keep object header on pre-finalizer registration

Pre-finalizers are heavy and not supposed to be used on short-living small
objects. Spend an additional word to store the header on registration. This
allows inspecting liveness without going through a call on every garbage
collection.

Bug: 982754
Change-Id: I30a2d60198dd6b2b49febcd49983d17175874dad
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1904177
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Omer Katz <omerkatz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#713880}

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

@@ -1078,6 +1078,21 @@ void ThreadState::FreePersistentNode(PersistentRegion* persistent_region,
     DCHECK(!static_persistents_.Contains(persistent_node));
 }
 
+void ThreadState::RegisterPreFinalizer(void* object,
+                                       PreFinalizerCallback callback) {
+#if DCHECK_IS_ON()
+  DCHECK(CheckThread());
+#endif
+  DCHECK(!SweepForbidden());
+
+  HeapObjectHeader* header = HeapObjectHeader::FromInnerAddress(object);
+  DCHECK(ordered_pre_finalizers_.end() ==
+         std::find(ordered_pre_finalizers_.begin(),
+                   ordered_pre_finalizers_.end(),
+                   PreFinalizer{header, object, callback}));
+  ordered_pre_finalizers_.push_back(PreFinalizer{header, object, callback});
+}
+
 void ThreadState::InvokePreFinalizers() {
   DCHECK(CheckThread());
   DCHECK(!SweepForbidden());
@@ -1085,23 +1100,22 @@ void ThreadState::InvokePreFinalizers() {
   ThreadHeapStatsCollector::Scope stats_scope(
       Heap().stats_collector(), ThreadHeapStatsCollector::kInvokePreFinalizers);
   SweepForbiddenScope sweep_forbidden(this);
-  // Pre finalizers are forbidden from allocating objects
+  // Pre finalizers are forbidden from allocating objects.
   NoAllocationScope no_allocation_scope(this);
 
   // Call the prefinalizers in the opposite order to their registration.
-  //
-  // LinkedHashSet does not support modification during iteration, so
-  // copy items first.
-  //
-  // The prefinalizer callback wrapper returns |true| when its associated
-  // object is unreachable garbage and the prefinalizer callback has run.
-  // The registered prefinalizer entry must then be removed and deleted.
   Deque<PreFinalizer> remaining_ordered_pre_finalizers;
   for (auto rit = ordered_pre_finalizers_.rbegin();
        rit != ordered_pre_finalizers_.rend(); ++rit) {
     const PreFinalizer& pre_finalizer = *rit;
-    if (!(pre_finalizer.second)(pre_finalizer.first))
+    // Check if pre-finalizer should be executed.
+    if (pre_finalizer.header->IsMarked()) {
+      // Re-queue for checking in next garbage collection.
       remaining_ordered_pre_finalizers.push_front(pre_finalizer);
+    } else {
+      // Execute pre-finalizer.
+      pre_finalizer.callback(pre_finalizer.object);
+    }
   }
 
   ordered_pre_finalizers_ = std::move(remaining_ordered_pre_finalizers);

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

@@ -62,6 +62,7 @@ class IncrementalMarkingScope;
 }  // namespace incremental_marking_test
 
 class CancelableTaskScheduler;
+class HeapObjectHeader;
 class MarkingVisitor;
 class PersistentNode;
 class PersistentRegion;
@@ -93,17 +94,13 @@ class Visitor;
 //     Member<Bar> bar_;
 //   };
 #define USING_PRE_FINALIZER(Class, preFinalizer)                          \
- public:                                                                  \
-  static bool InvokePreFinalizer(void* object) {                          \
-    Class* self = reinterpret_cast<Class*>(object);                       \
-    if (ThreadHeap::IsHeapObjectAlive(self))                              \
-      return false;                                                       \
-    self->Class::preFinalizer();                                          \
-    return true;                                                          \
+ private:                                                                 \
+  static void PreFinalizerDispatch(void* object) {                        \
+    reinterpret_cast<Class*>(object)->Class::preFinalizer();              \
   }                                                                       \
                                                                           \
- private:                                                                 \
-  ThreadState::PrefinalizerRegistration<Class> prefinalizer_dummy_{this}; \
+  friend class ThreadState::PreFinalizerRegistration<Class>;              \
+  ThreadState::PreFinalizerRegistration<Class> prefinalizer_dummy_{this}; \
   using UsingPreFinalizerMacroNeedsTrailingSemiColon = char
 
 class PLATFORM_EXPORT BlinkGCObserver {
@@ -133,24 +130,16 @@ class PLATFORM_EXPORT ThreadState final {
   // Register the pre-finalizer for the |self| object. The class T be using
   // USING_PRE_FINALIZER() macro.
   template <typename T>
-  class PrefinalizerRegistration final {
+  class PreFinalizerRegistration final {
     DISALLOW_NEW();
 
    public:
-    PrefinalizerRegistration(T* self) {
-      static_assert(sizeof(&T::InvokePreFinalizer) > 0,
+    PreFinalizerRegistration(T* self) {
+      static_assert(sizeof(&T::PreFinalizerDispatch) > 0,
                     "USING_PRE_FINALIZER(T) must be defined.");
       ThreadState* state =
           ThreadStateFor<ThreadingTrait<T>::kAffinity>::GetState();
-#if DCHECK_IS_ON()
-      DCHECK(state->CheckThread());
-#endif
-      DCHECK(!state->SweepForbidden());
-      DCHECK(std::find(state->ordered_pre_finalizers_.begin(),
-                       state->ordered_pre_finalizers_.end(),
-                       PreFinalizer(self, T::InvokePreFinalizer)) ==
-             state->ordered_pre_finalizers_.end());
-      state->ordered_pre_finalizers_.emplace_back(self, T::InvokePreFinalizer);
+      state->RegisterPreFinalizer(self, T::PreFinalizerDispatch);
     }
   };
 
@@ -385,6 +374,17 @@ class PLATFORM_EXPORT ThreadState final {
  private:
   class IncrementalMarkingScheduler;
 
+  using PreFinalizerCallback = void (*)(void*);
+  struct PreFinalizer {
+    HeapObjectHeader* header;
+    void* object;
+    PreFinalizerCallback callback;
+
+    bool operator==(const PreFinalizer& other) const {
+      return object == other.object && callback == other.callback;
+    }
+  };
+
   // Duration of one incremental marking step. Should be short enough that it
   // doesn't cause jank even though it is scheduled as a normal task.
   static constexpr base::TimeDelta kDefaultIncrementalMarkingStepDuration =
@@ -509,6 +509,7 @@ class PLATFORM_EXPORT ThreadState final {
 
   void SynchronizeAndFinishConcurrentSweeping();
 
+  void RegisterPreFinalizer(void*, PreFinalizerCallback);
   void InvokePreFinalizers();
 
   // Adds the given observer to the ThreadState's observer list. This doesn't
@@ -557,9 +558,6 @@ class PLATFORM_EXPORT ThreadState final {
   BlinkGC::GCReason reason_for_scheduled_gc_ =
       BlinkGC::GCReason::kForcedGCForTesting;
 
-  using PreFinalizerCallback = bool (*)(void*);
-  using PreFinalizer = std::pair<void*, PreFinalizerCallback>;
-
   // Pre-finalizers are called in the reverse order in which they are
   // registered by the constructors (including constructors of Mixin objects)
   // for an object, by processing the ordered_pre_finalizers_ back-to-front.
@@ -607,7 +605,7 @@ class PLATFORM_EXPORT ThreadState final {
   friend class IncrementalMarkingTestDriver;
   friend class HeapAllocator;
   template <typename T>
-  friend class PrefinalizerRegistration;
+  friend class PreFinalizerRegistration;
   friend class TestGCScope;
   friend class ThreadStateSchedulingTest;
   friend class UnifiedHeapController;