[heap] Remove revisiting logic in the main thread marker

Factory::CopyCode was using ProcessBlackAllocatedObject and
WriteBarrierForCode(Code) to handle write barriers for that newly
created code object. But even when used in tandem with each other they
would miss OLD_TO_NEW references in the code object header.

This CL simplifies Factory::CopyCode by letting
WriteBarrierForCode(Code) handle all outgoing pointers of that code
object (not just a subset of RelocInfos) by implementing an
ObjectVisitor. This removes the need for ProcessBlackAllocatedObject.

Since Factory::CopyCode was the only user of
ProcessBlackAllocatedObject, we can also remove all the object
revisiting logic in the main thread marker.

Bug: v8:11708
Change-Id: I7d9b12eb0a76ba41a38efc147f44556ddc941a96
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3810186Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Dominik Inführ <dinfuehr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#82212}

src/heap/factory.cc

@@ -2563,9 +2563,6 @@ Handle<Code> Factory::CopyCode(Handle<Code> code) {
     new_code->set_code_data_container(*data_container, kReleaseStore);
 
     new_code->Relocate(new_addr - old_addr);
-    // We have to iterate over the object and process its pointers when black
-    // allocation is on.
-    heap->incremental_marking()->ProcessBlackAllocatedObject(*new_code);
     // Record all references to embedded objects in the new code object.
 #ifndef V8_DISABLE_WRITE_BARRIERS
     WriteBarrierForCode(*new_code);

src/heap/heap.cc

@@ -92,9 +92,11 @@
 #include "src/objects/hash-table.h"
 #include "src/objects/instance-type.h"
 #include "src/objects/maybe-object.h"
+#include "src/objects/objects.h"
 #include "src/objects/shared-function-info.h"
 #include "src/objects/slots-atomic-inl.h"
 #include "src/objects/slots-inl.h"
+#include "src/objects/visitors.h"
 #include "src/regexp/regexp.h"
 #include "src/snapshot/embedded/embedded-data.h"
 #include "src/snapshot/serializer-deserializer.h"
@@ -7251,13 +7253,46 @@ CodeLookupResult Heap::GcSafeFindCodeForInnerPointerForPrinting(
   return {};
 }
 
-void Heap::WriteBarrierForCodeSlow(Code code) {
-  PtrComprCageBase cage_base = code.main_cage_base();
-  for (RelocIterator it(code, RelocInfo::EmbeddedObjectModeMask()); !it.done();
-       it.next()) {
-    HeapObject target_object = it.rinfo()->target_object(cage_base);
-    WriteBarrierForCode(code, it.rinfo(), target_object);
+class CodeObjectVisitor : public ObjectVisitorWithCageBases {
+ public:
+  explicit CodeObjectVisitor(Isolate* isolate)
+      : ObjectVisitorWithCageBases(isolate) {}
+
+  void VisitCodePointer(HeapObject host, CodeObjectSlot slot) override {
+    UNREACHABLE();
+  }
+  void VisitCodeTarget(Code host, RelocInfo* rinfo) override {
+#ifdef ENABLE_SLOW_DCHECKS
+    DCHECK(RelocInfo::IsCodeTargetMode(rinfo->rmode()));
+    Code target_object =
+        Code::GetCodeFromTargetAddress(rinfo->target_address());
+    DCHECK(!WriteBarrier::IsRequired(host, target_object));
+#endif
+  }
+
+  void VisitEmbeddedPointer(Code host, RelocInfo* rinfo) override {
+    HeapObject target_object = rinfo->target_object(cage_base());
+    WriteBarrierForCode(host, rinfo, target_object);
   }
+  void VisitPointers(HeapObject host, MaybeObjectSlot start,
+                     MaybeObjectSlot end) override {
+    UNREACHABLE();
+  }
+  void VisitPointers(HeapObject host, ObjectSlot start,
+                     ObjectSlot end) override {
+    for (ObjectSlot slot = start; slot < end; ++slot) {
+      Object object = slot.Relaxed_Load(cage_base());
+      CombinedWriteBarrier(host, slot, object, UPDATE_WRITE_BARRIER);
+    }
+  }
+};
+
+void Heap::WriteBarrierForCodeSlow(Code code) {
+  Isolate* isolate = GetIsolateFromWritableObject(code);
+  CombinedWriteBarrier(code, code.map_slot(), code.map(isolate),
+                       UPDATE_WRITE_BARRIER);
+  CodeObjectVisitor visitor(isolate);
+  Code::BodyDescriptor::IterateBody(code.map(isolate), code, &visitor);
 }
 
 void Heap::CombinedGenerationalAndSharedBarrierSlow(HeapObject object,

src/heap/incremental-marking.cc

@@ -456,12 +456,6 @@ void IncrementalMarking::UpdateMarkedBytesAfterScavenge(
   bytes_marked_ -= std::min(bytes_marked_, dead_bytes_in_new_space);
 }
 
-void IncrementalMarking::ProcessBlackAllocatedObject(HeapObject obj) {
-  if (IsMarking() && marking_state()->IsBlack(obj)) {
-    collector_->RevisitObject(obj);
-  }
-}
-
 StepResult IncrementalMarking::EmbedderStep(double expected_duration_ms,
                                             double* duration_ms) {
   if (!ShouldDoEmbedderStep()) {

src/heap/incremental-marking.h

@@ -130,8 +130,6 @@ class V8_EXPORT_PRIVATE IncrementalMarking final {
 
   bool IsCompacting() { return IsMarking() && is_compacting_; }
 
-  void ProcessBlackAllocatedObject(HeapObject obj);
-
   Heap* heap() const { return heap_; }
 
   IncrementalMarkingJob* incremental_marking_job() {

src/heap/mark-compact.cc

@@ -2240,14 +2240,6 @@ void MarkCompactCollector::VisitObject(HeapObject obj) {
   marking_visitor_->Visit(obj.map(), obj);
 }
 
-void MarkCompactCollector::RevisitObject(HeapObject obj) {
-  DCHECK(marking_state()->IsBlack(obj));
-  DCHECK_IMPLIES(MemoryChunk::FromHeapObject(obj)->ProgressBar().IsEnabled(),
-                 0u == MemoryChunk::FromHeapObject(obj)->ProgressBar().Value());
-  MarkingVisitor::RevisitScope revisit(marking_visitor_.get());
-  marking_visitor_->Visit(obj.map(marking_visitor_->cage_base()), obj);
-}
-
 bool MarkCompactCollector::MarkTransitiveClosureUntilFixpoint() {
   int iterations = 0;
   int max_iterations = FLAG_ephemeron_fixpoint_iterations;
@@ -5473,11 +5465,6 @@ void MinorMarkCompactCollector::VisitObject(HeapObject obj) {
   main_marking_visitor_->Visit(obj.map(), obj);
 }
 
-void MinorMarkCompactCollector::RevisitObject(HeapObject obj) {
-  // TODO(v8:13012): Implement.
-  UNREACHABLE();
-}
-
 void MinorMarkCompactCollector::SweepArrayBufferExtensions() {
   TRACE_GC(heap()->tracer(),
            GCTracer::Scope::MINOR_MC_FINISH_SWEEP_ARRAY_BUFFERS);

src/heap/mark-compact.h

@@ -259,22 +259,6 @@ class MainMarkingVisitor final
     : public MarkingVisitorBase<MainMarkingVisitor<MarkingState>,
                                 MarkingState> {
  public:
-  // This is used for revisiting objects that were black allocated.
-  class V8_NODISCARD RevisitScope {
-   public:
-    explicit RevisitScope(MainMarkingVisitor* visitor) : visitor_(visitor) {
-      DCHECK(!visitor->revisiting_object_);
-      visitor->revisiting_object_ = true;
-    }
-    ~RevisitScope() {
-      DCHECK(visitor_->revisiting_object_);
-      visitor_->revisiting_object_ = false;
-    }
-
-   private:
-    MainMarkingVisitor<MarkingState>* visitor_;
-  };
-
   MainMarkingVisitor(MarkingState* marking_state,
                      MarkingWorklists::Local* local_marking_worklists,
                      WeakObjects::Local* local_weak_objects, Heap* heap,
@@ -286,13 +270,11 @@ class MainMarkingVisitor final
             local_marking_worklists, local_weak_objects, heap,
             mark_compact_epoch, code_flush_mode, embedder_tracing_enabled,
             should_keep_ages_unchanged),
-        marking_state_(marking_state),
-        revisiting_object_(false) {}
+        marking_state_(marking_state) {}
 
-  // HeapVisitor override to allow revisiting of black objects.
+  // HeapVisitor override.
   bool ShouldVisit(HeapObject object) {
-    return marking_state_->GreyToBlack(object) ||
-           V8_UNLIKELY(revisiting_object_);
+    return marking_state_->GreyToBlack(object);
   }
 
  private:
@@ -321,7 +303,6 @@ class MainMarkingVisitor final
 
   friend class MarkingVisitorBase<MainMarkingVisitor<MarkingState>,
                                   MarkingState>;
-  bool revisiting_object_;
 };
 
 class YoungGenerationMarkingVisitor final
@@ -372,8 +353,6 @@ class CollectorBase {
 
   // Used by incremental marking for object that change their layout.
   virtual void VisitObject(HeapObject obj) = 0;
-  // Used by incremental marking for black-allocated objects.
-  virtual void RevisitObject(HeapObject obj) = 0;
 
   virtual bool sweeping_in_progress() const = 0;
 
@@ -536,7 +515,6 @@ class MarkCompactCollector final : public CollectorBase {
   WeakObjects::Local* local_weak_objects() { return local_weak_objects_.get(); }
 
   void VisitObject(HeapObject obj) final;
-  void RevisitObject(HeapObject obj) final;
 
   void AddNewlyDiscovered(HeapObject object) {
     if (ephemeron_marking_.newly_discovered_overflowed) return;
@@ -835,7 +813,6 @@ class MinorMarkCompactCollector final : public CollectorBase {
   }
 
   void VisitObject(HeapObject obj) final;
-  void RevisitObject(HeapObject obj) final;
 
  private:
   class RootMarkingVisitor;