[vm, gc] Don't double-visit remembered cards to decide if they should remain remembered.

TEST=ci
Change-Id: Ib92dbbabf90f80c6e984c21b9eaee67fd4205f8f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/365543
Commit-Queue: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Alexander Aprelev <aam@google.com>

runtime/vm/heap/page.cc

@@ -207,7 +207,7 @@ void Page::VisitObjectPointers(ObjectPointerVisitor* visitor) const {
   ASSERT(obj_addr == end_addr);
 }
 
-void Page::VisitRememberedCards(ObjectPointerVisitor* visitor) {
+void Page::VisitRememberedCards(PredicateObjectPointerVisitor* visitor) {
   ASSERT(Thread::Current()->OwnsGCSafepoint() ||
          (Thread::Current()->task_kind() == Thread::kScavengerTask));
   NoSafepointScope no_safepoint;
@@ -258,15 +258,9 @@ void Page::VisitRememberedCards(ObjectPointerVisitor* visitor) {
         card_to = obj_to;
       }
 
-      visitor->VisitCompressedPointers(heap_base, card_from, card_to);
+      bool has_new_target = visitor->PredicateVisitCompressedPointers(
+          heap_base, card_from, card_to);
 
-      bool has_new_target = false;
-      for (CompressedObjectPtr* slot = card_from; slot <= card_to; slot++) {
-        if ((*slot)->IsNewObjectMayBeSmi()) {
-          has_new_target = true;
-          break;
-        }
-      }
       if (!has_new_target) {
         cell ^= bit_mask;
       }

runtime/vm/heap/page.h

@@ -173,7 +173,7 @@ class Page {
     return IsCardRemembered(reinterpret_cast<uword>(slot));
   }
 #endif
-  void VisitRememberedCards(ObjectPointerVisitor* visitor);
+  void VisitRememberedCards(PredicateObjectPointerVisitor* visitor);
   void ResetProgressBar();
 
   Thread* owner() const { return owner_; }

runtime/vm/heap/pages.cc

@@ -676,7 +676,8 @@ void PageSpace::VisitObjectPointers(ObjectPointerVisitor* visitor) const {
   }
 }
 
-void PageSpace::VisitRememberedCards(ObjectPointerVisitor* visitor) const {
+void PageSpace::VisitRememberedCards(
+    PredicateObjectPointerVisitor* visitor) const {
   ASSERT(Thread::Current()->OwnsGCSafepoint() ||
          (Thread::Current()->task_kind() == Thread::kScavengerTask));
 

runtime/vm/heap/pages.h

@@ -230,7 +230,7 @@ class PageSpace {
   void VisitObjectsUnsafe(ObjectVisitor* visitor) const;
   void VisitObjectPointers(ObjectPointerVisitor* visitor) const;
 
-  void VisitRememberedCards(ObjectPointerVisitor* visitor) const;
+  void VisitRememberedCards(PredicateObjectPointerVisitor* visitor) const;
   void ResetProgressBars() const;
 
   // Collect the garbage in the page space using mark-sweep or mark-compact.

runtime/vm/heap/scavenger.cc

@@ -127,7 +127,8 @@ static void WriteHeaderRelaxed(ObjectPtr obj, uword header) {
 }
 
 template <bool parallel>
-class ScavengerVisitorBase : public ObjectPointerVisitor {
+class ScavengerVisitorBase : public ObjectPointerVisitor,
+                             public PredicateObjectPointerVisitor {
  public:
   explicit ScavengerVisitorBase(IsolateGroup* isolate_group,
                                 Scavenger* scavenger,
@@ -226,14 +227,35 @@ class ScavengerVisitorBase : public ObjectPointerVisitor {
     }
   }
 
+  bool PredicateVisitPointers(ObjectPtr* first, ObjectPtr* last) override {
+    bool has_new_target = false;
+    for (ObjectPtr* current = first; current <= last; current++) {
+      has_new_target |= PredicateScavengePointer(current);
+    }
+    return has_new_target;
+  }
+
 #if defined(DART_COMPRESSED_POINTERS)
+  bool PredicateVisitCompressedPointers(uword heap_base,
+                                        CompressedObjectPtr* first,
+                                        CompressedObjectPtr* last) override {
+    bool has_new_target = false;
+    for (CompressedObjectPtr* current = first; current <= last; current++) {
+      has_new_target |= PredicateScavengeCompressedPointer(heap_base, current);
+    }
+    return has_new_target;
+  }
+
   void VisitCompressedPointers(uword heap_base,
                                CompressedObjectPtr* first,
                                CompressedObjectPtr* last) override {
-    ASSERT(Utils::IsAligned(first, sizeof(*first)));
-    ASSERT(Utils::IsAligned(last, sizeof(*last)));
-    for (CompressedObjectPtr* current = first; current <= last; current++) {
-      ScavengeCompressedPointer(heap_base, current);
+    if (PredicateVisitCompressedPointers(heap_base, first, last)) {
+      // Update the store buffer as needed.
+      ObjectPtr visiting_object = visiting_old_object_;
+      if (visiting_object != nullptr &&
+          visiting_object->untag()->TryAcquireRememberedBit()) {
+        thread_->StoreBufferAddObjectGC(visiting_object);
+      }
     }
   }
 #endif
@@ -353,19 +375,24 @@ class ScavengerVisitorBase : public ObjectPointerVisitor {
 
  private:
   DART_FORCE_INLINE
-  void ScavengePointer(ObjectPtr* p) {
+  bool PredicateScavengePointer(ObjectPtr* p) {
     // ScavengePointer cannot be called recursively.
     ObjectPtr obj = *p;
 
     if (obj->IsImmediateOrOldObject()) {
-      return;
+      return false;
     }
 
     ObjectPtr new_obj = ScavengeObject(obj);
 
     // Update the reference.
     *p = new_obj;
-    if (new_obj->IsNewObject()) {
+    return new_obj->IsNewObject();
+  }
+
+  DART_FORCE_INLINE
+  void ScavengePointer(ObjectPtr* p) {
+    if (PredicateScavengePointer(p)) {
       // Update the store buffer as needed.
       ObjectPtr visiting_object = visiting_old_object_;
       if (visiting_object != nullptr &&
@@ -376,20 +403,26 @@ class ScavengerVisitorBase : public ObjectPointerVisitor {
   }
 
   DART_FORCE_INLINE
-  void ScavengeCompressedPointer(uword heap_base, CompressedObjectPtr* p) {
+  bool PredicateScavengeCompressedPointer(uword heap_base,
+                                          CompressedObjectPtr* p) {
     // ScavengePointer cannot be called recursively.
     ObjectPtr obj = p->Decompress(heap_base);
 
     // Could be tested without decompression.
     if (obj->IsImmediateOrOldObject()) {
-      return;
+      return false;
     }
 
     ObjectPtr new_obj = ScavengeObject(obj);
 
     // Update the reference.
     *p = new_obj;
-    if (new_obj->IsNewObject()) {
+    return new_obj->IsNewObject();
+  }
+
+  DART_FORCE_INLINE
+  void ScavengeCompressedPointer(uword heap_base, CompressedObjectPtr* p) {
+    if (PredicateScavengeCompressedPointer(heap_base, p)) {
       // Update the store buffer as needed.
       ObjectPtr visiting_object = visiting_old_object_;
       if (visiting_object != nullptr &&

runtime/vm/visitor.h

@@ -87,6 +87,24 @@ class ObjectPointerVisitor {
   DISALLOW_IMPLICIT_CONSTRUCTORS(ObjectPointerVisitor);
 };
 
+class PredicateObjectPointerVisitor {
+ public:
+  PredicateObjectPointerVisitor() {}
+  virtual ~PredicateObjectPointerVisitor() {}
+  virtual bool PredicateVisitPointers(ObjectPtr* first, ObjectPtr* last) = 0;
+#if defined(DART_COMPRESSED_POINTERS)
+  virtual bool PredicateVisitCompressedPointers(uword heap_base,
+                                                CompressedObjectPtr* first,
+                                                CompressedObjectPtr* last) = 0;
+#else
+  bool PredicateVisitCompressedPointers(uword heap_base,
+                                        CompressedObjectPtr* first,
+                                        CompressedObjectPtr* last) {
+    return PredicateVisitPointers(first, last);
+  }
+#endif
+};
+
 // An object visitor interface.
 class ObjectVisitor {
  public: