[heap] Refactor root iteration

This replaces VisitMode with a set of option flags that allow skipping
specific roots like unserializable, weak, global handles, etc.
The advantage is that it is no longer coupled with the callers and does
not know about different types of GCs and their phases.

The CL is pure refactoring without behavior changes except for the
heap verification where more roots are verified that before.

Change-Id: I350b2ed14826e0efb75770111c6b28bb8d4d9845
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2190420Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Dominik Inführ <dinfuehr@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#67704}

src/common/globals.h

@@ -811,17 +811,6 @@ enum class LocalSpaceKind {
 
 enum Executability { NOT_EXECUTABLE, EXECUTABLE };
 
-enum VisitMode {
-  VISIT_ALL,
-  VISIT_ALL_IN_MINOR_MC_MARK,
-  VISIT_ALL_IN_MINOR_MC_UPDATE,
-  VISIT_ALL_IN_SCAVENGE,
-  VISIT_ALL_IN_SWEEP_NEWSPACE,
-  VISIT_ONLY_STRONG,
-  VISIT_ONLY_STRONG_IGNORE_STACK,
-  VISIT_FOR_SERIALIZATION,
-};
-
 enum class BytecodeFlushMode {
   kDoNotFlushBytecode,
   kFlushBytecode,

src/heap/heap.cc

@@ -3247,7 +3247,7 @@ FixedArrayBase Heap::LeftTrimFixedArray(FixedArrayBase object,
     // to the original FixedArray (which is now the filler object).
     LeftTrimmerVerifierRootVisitor root_visitor(object);
     ReadOnlyRoots(this).Iterate(&root_visitor);
-    IterateRoots(&root_visitor, VISIT_ALL);
+    IterateRoots(&root_visitor, {});
   }
 #endif  // ENABLE_SLOW_DCHECKS
 
@@ -4218,7 +4218,7 @@ void Heap::Verify() {
   array_buffer_sweeper()->EnsureFinished();
 
   VerifyPointersVisitor visitor(this);
-  IterateRoots(&visitor, VISIT_ONLY_STRONG);
+  IterateRoots(&visitor, {});
 
   if (!isolate()->context().is_null() &&
       !isolate()->normalized_map_cache()->IsUndefined(isolate())) {
@@ -4469,20 +4469,13 @@ void Heap::set_builtin(int index, Code builtin) {
   isolate()->builtins_table()[index] = builtin.ptr();
 }
 
-void Heap::IterateRoots(RootVisitor* v, VisitMode mode) {
-  IterateStrongRoots(v, mode);
-  IterateWeakRoots(v, mode);
-}
-
-void Heap::IterateWeakRoots(RootVisitor* v, VisitMode mode) {
-  const bool isMinorGC = mode == VISIT_ALL_IN_SCAVENGE ||
-                         mode == VISIT_ALL_IN_MINOR_MC_MARK ||
-                         mode == VISIT_ALL_IN_MINOR_MC_UPDATE;
+void Heap::IterateWeakRoots(RootVisitor* v, base::EnumSet<SkipRoot> options) {
+  DCHECK(!options.contains(SkipRoot::kWeak));
   v->VisitRootPointer(Root::kStringTable, nullptr,
                       FullObjectSlot(&roots_table()[RootIndex::kStringTable]));
   v->Synchronize(VisitorSynchronization::kStringTable);
-  if (!isMinorGC && mode != VISIT_ALL_IN_SWEEP_NEWSPACE &&
-      mode != VISIT_FOR_SERIALIZATION) {
+  if (!options.contains(SkipRoot::kExternalStringTable) &&
+      !options.contains(SkipRoot::kUnserializable)) {
     // Scavenge collections have special processing for this.
     // Do not visit for serialization, since the external string table will
     // be populated from scratch upon deserialization.
@@ -4549,10 +4542,7 @@ class FixStaleLeftTrimmedHandlesVisitor : public RootVisitor {
   Heap* heap_;
 };
 
-void Heap::IterateStrongRoots(RootVisitor* v, VisitMode mode) {
-  const bool isMinorGC = mode == VISIT_ALL_IN_SCAVENGE ||
-                         mode == VISIT_ALL_IN_MINOR_MC_MARK ||
-                         mode == VISIT_ALL_IN_MINOR_MC_UPDATE;
+void Heap::IterateRoots(RootVisitor* v, base::EnumSet<SkipRoot> options) {
   v->VisitRootPointers(Root::kStrongRootList, nullptr,
                        roots_table().strong_roots_begin(),
                        roots_table().strong_roots_end());
@@ -4568,37 +4558,11 @@ void Heap::IterateStrongRoots(RootVisitor* v, VisitMode mode) {
   isolate_->compilation_cache()->Iterate(v);
   v->Synchronize(VisitorSynchronization::kCompilationCache);
 
-  // Iterate over the builtin code objects in the heap. Note that it is not
-  // necessary to iterate over code objects on scavenge collections.
-  if (!isMinorGC) {
+  if (!options.contains(SkipRoot::kOldGeneration)) {
     IterateBuiltins(v);
     v->Synchronize(VisitorSynchronization::kBuiltins);
   }
 
-  // Iterate over global handles.
-  switch (mode) {
-    case VISIT_FOR_SERIALIZATION:
-      // Global handles are not iterated by the serializer. Values referenced by
-      // global handles need to be added manually.
-      break;
-    case VISIT_ONLY_STRONG:
-    case VISIT_ONLY_STRONG_IGNORE_STACK:
-      isolate_->global_handles()->IterateStrongRoots(v);
-      break;
-    case VISIT_ALL_IN_SCAVENGE:
-    case VISIT_ALL_IN_MINOR_MC_MARK:
-      isolate_->global_handles()->IterateYoungStrongAndDependentRoots(v);
-      break;
-    case VISIT_ALL_IN_MINOR_MC_UPDATE:
-      isolate_->global_handles()->IterateAllYoungRoots(v);
-      break;
-    case VISIT_ALL_IN_SWEEP_NEWSPACE:
-    case VISIT_ALL:
-      isolate_->global_handles()->IterateAllRoots(v);
-      break;
-  }
-  v->Synchronize(VisitorSynchronization::kGlobalHandles);
-
   // Iterate over pointers being held by inactive threads.
   isolate_->thread_manager()->Iterate(v);
   v->Synchronize(VisitorSynchronization::kThreadManager);
@@ -4618,8 +4582,30 @@ void Heap::IterateStrongRoots(RootVisitor* v, VisitMode mode) {
   // The general guideline for adding visitors to this section vs. adding them
   // above is that non-transient heap state is always visited, transient heap
   // state is visited only when not serializing.
-  if (mode != VISIT_FOR_SERIALIZATION) {
-    if (mode != VISIT_ONLY_STRONG_IGNORE_STACK) {
+  if (!options.contains(SkipRoot::kUnserializable)) {
+    if (!options.contains(SkipRoot::kGlobalHandles)) {
+      if (options.contains(SkipRoot::kWeak)) {
+        if (options.contains(SkipRoot::kOldGeneration)) {
+          // Skip handles that are either weak or old.
+          isolate_->global_handles()->IterateYoungStrongAndDependentRoots(v);
+        } else {
+          // Skip handles that are weak.
+          isolate_->global_handles()->IterateStrongRoots(v);
+        }
+      } else {
+        // Do not skip weak handles.
+        if (options.contains(SkipRoot::kOldGeneration)) {
+          // Skip handles that are old.
+          isolate_->global_handles()->IterateAllYoungRoots(v);
+        } else {
+          // Do not skip any handles.
+          isolate_->global_handles()->IterateAllRoots(v);
+        }
+      }
+    }
+    v->Synchronize(VisitorSynchronization::kGlobalHandles);
+
+    if (!options.contains(SkipRoot::kStack)) {
       isolate_->Iterate(v);
       isolate_->global_handles()->IterateStrongStackRoots(v);
       v->Synchronize(VisitorSynchronization::kTop);
@@ -4641,10 +4627,7 @@ void Heap::IterateStrongRoots(RootVisitor* v, VisitMode mode) {
     isolate_->IterateDeferredHandles(v);
     v->Synchronize(VisitorSynchronization::kHandleScope);
 
-    // Iterate over eternal handles. Eternal handles are not iterated by the
-    // serializer. Values referenced by eternal handles need to be added
-    // manually.
-    if (isMinorGC) {
+    if (options.contains(SkipRoot::kOldGeneration)) {
       isolate_->eternal_handles()->IterateYoungRoots(v);
     } else {
       isolate_->eternal_handles()->IterateAllRoots(v);
@@ -4674,6 +4657,10 @@ void Heap::IterateStrongRoots(RootVisitor* v, VisitMode mode) {
     SerializerDeserializer::Iterate(isolate_, v);
     v->Synchronize(VisitorSynchronization::kStartupObjectCache);
   }
+
+  if (!options.contains(SkipRoot::kWeak)) {
+    IterateWeakRoots(v, options);
+  }
 }
 
 void Heap::IterateWeakGlobalHandles(RootVisitor* v) {
@@ -6030,7 +6017,7 @@ class UnreachableObjectsFilter : public HeapObjectsFilter {
 
   void MarkReachableObjects() {
     MarkingVisitor visitor(this);
-    heap_->IterateRoots(&visitor, VISIT_ALL);
+    heap_->IterateRoots(&visitor, {});
     visitor.TransitiveClosure();
   }
 

src/heap/heap.h

@@ -17,6 +17,7 @@
 #include "include/v8-internal.h"
 #include "include/v8.h"
 #include "src/base/atomic-utils.h"
+#include "src/base/enum-set.h"
 #include "src/base/platform/condition-variable.h"
 #include "src/builtins/accessors.h"
 #include "src/common/assert-scope.h"
@@ -164,6 +165,15 @@ enum class YoungGenerationHandling {
 
 enum class GCIdleTimeAction : uint8_t;
 
+enum class SkipRoot {
+  kExternalStringTable,
+  kGlobalHandles,
+  kOldGeneration,
+  kStack,
+  kUnserializable,
+  kWeak
+};
+
 class AllocationResult {
  public:
   static inline AllocationResult Retry(AllocationSpace space = NEW_SPACE) {
@@ -922,14 +932,12 @@ class Heap {
   // (de)serialization or heap verification.
 
   // Iterates over the strong roots and the weak roots.
-  void IterateRoots(RootVisitor* v, VisitMode mode);
-  // Iterates over the strong roots.
-  void IterateStrongRoots(RootVisitor* v, VisitMode mode);
+  void IterateRoots(RootVisitor* v, base::EnumSet<SkipRoot> options);
   // Iterates over entries in the smi roots list.  Only interesting to the
   // serializer/deserializer, since GC does not care about smis.
   void IterateSmiRoots(RootVisitor* v);
   // Iterates over weak string tables.
-  void IterateWeakRoots(RootVisitor* v, VisitMode mode);
+  void IterateWeakRoots(RootVisitor* v, base::EnumSet<SkipRoot> options);
   // Iterates over weak global handles.
   void IterateWeakGlobalHandles(RootVisitor* v);
   // Iterates over builtins.

src/heap/incremental-marking.cc

@@ -414,7 +414,8 @@ void IncrementalMarking::MarkRoots() {
   DCHECK(IsMarking());
 
   IncrementalMarkingRootMarkingVisitor visitor(this);
-  heap_->IterateStrongRoots(&visitor, VISIT_ONLY_STRONG_IGNORE_STACK);
+  heap_->IterateRoots(
+      &visitor, base::EnumSet<SkipRoot>{SkipRoot::kStack, SkipRoot::kWeak});
 }
 
 bool IncrementalMarking::ShouldRetainMap(Map map, int age) {

src/heap/mark-compact.cc

@@ -98,7 +98,7 @@ class MarkingVerifier : public ObjectVisitor, public RootVisitor {
     VerifyRootPointers(start, end);
   }
 
-  void VerifyRoots(VisitMode mode);
+  void VerifyRoots();
   void VerifyMarkingOnPage(const Page* page, Address start, Address end);
   void VerifyMarking(NewSpace* new_space);
   void VerifyMarking(PagedSpace* paged_space);
@@ -107,8 +107,8 @@ class MarkingVerifier : public ObjectVisitor, public RootVisitor {
   Heap* heap_;
 };
 
-void MarkingVerifier::VerifyRoots(VisitMode mode) {
-  heap_->IterateStrongRoots(this, mode);
+void MarkingVerifier::VerifyRoots() {
+  heap_->IterateRoots(this, base::EnumSet<SkipRoot>{SkipRoot::kWeak});
 }
 
 void MarkingVerifier::VerifyMarkingOnPage(const Page* page, Address start,
@@ -179,7 +179,7 @@ class FullMarkingVerifier : public MarkingVerifier {
             heap->mark_compact_collector()->non_atomic_marking_state()) {}
 
   void Run() override {
-    VerifyRoots(VISIT_ONLY_STRONG);
+    VerifyRoots();
     VerifyMarking(heap_->new_space());
     VerifyMarking(heap_->new_lo_space());
     VerifyMarking(heap_->old_space());
@@ -275,7 +275,7 @@ class EvacuationVerifier : public ObjectVisitor, public RootVisitor {
   virtual void VerifyPointers(MaybeObjectSlot start, MaybeObjectSlot end) = 0;
   virtual void VerifyRootPointers(FullObjectSlot start, FullObjectSlot end) = 0;
 
-  void VerifyRoots(VisitMode mode);
+  void VerifyRoots();
   void VerifyEvacuationOnPage(Address start, Address end);
   void VerifyEvacuation(NewSpace* new_space);
   void VerifyEvacuation(PagedSpace* paged_space);
@@ -283,8 +283,8 @@ class EvacuationVerifier : public ObjectVisitor, public RootVisitor {
   Heap* heap_;
 };
 
-void EvacuationVerifier::VerifyRoots(VisitMode mode) {
-  heap_->IterateStrongRoots(this, mode);
+void EvacuationVerifier::VerifyRoots() {
+  heap_->IterateRoots(this, base::EnumSet<SkipRoot>{SkipRoot::kWeak});
 }
 
 void EvacuationVerifier::VerifyEvacuationOnPage(Address start, Address end) {
@@ -325,7 +325,7 @@ class FullEvacuationVerifier : public EvacuationVerifier {
   explicit FullEvacuationVerifier(Heap* heap) : EvacuationVerifier(heap) {}
 
   void Run() override {
-    VerifyRoots(VISIT_ALL);
+    VerifyRoots();
     VerifyEvacuation(heap_->new_space());
     VerifyEvacuation(heap_->old_space());
     VerifyEvacuation(heap_->code_space());
@@ -1604,7 +1604,7 @@ void MarkCompactCollector::MarkRoots(RootVisitor* root_visitor,
                                      ObjectVisitor* custom_root_body_visitor) {
   // Mark the heap roots including global variables, stack variables,
   // etc., and all objects reachable from them.
-  heap()->IterateStrongRoots(root_visitor, VISIT_ONLY_STRONG);
+  heap()->IterateRoots(root_visitor, base::EnumSet<SkipRoot>{SkipRoot::kWeak});
 
   // Custom marking for string table and top optimized frame.
   MarkStringTable(custom_root_body_visitor);
@@ -3877,7 +3877,9 @@ void MarkCompactCollector::UpdatePointersAfterEvacuation() {
   {
     TRACE_GC(heap()->tracer(),
              GCTracer::Scope::MC_EVACUATE_UPDATE_POINTERS_TO_NEW_ROOTS);
-    heap_->IterateRoots(&updating_visitor, VISIT_ALL_IN_SWEEP_NEWSPACE);
+    // The external string table is updated at the end.
+    heap_->IterateRoots(&updating_visitor, base::EnumSet<SkipRoot>{
+                                               SkipRoot::kExternalStringTable});
   }
 
   {
@@ -4131,7 +4133,7 @@ class YoungGenerationMarkingVerifier : public MarkingVerifier {
   }
 
   void Run() override {
-    VerifyRoots(VISIT_ALL_IN_SCAVENGE);
+    VerifyRoots();
     VerifyMarking(heap_->new_space());
   }
 
@@ -4181,7 +4183,7 @@ class YoungGenerationEvacuationVerifier : public EvacuationVerifier {
       : EvacuationVerifier(heap) {}
 
   void Run() override {
-    VerifyRoots(VISIT_ALL_IN_SCAVENGE);
+    VerifyRoots();
     VerifyEvacuation(heap_->new_space());
     VerifyEvacuation(heap_->old_space());
     VerifyEvacuation(heap_->code_space());
@@ -4461,7 +4463,9 @@ void MinorMarkCompactCollector::UpdatePointersAfterEvacuation() {
   {
     TRACE_GC(heap()->tracer(),
              GCTracer::Scope::MINOR_MC_EVACUATE_UPDATE_POINTERS_TO_NEW_ROOTS);
-    heap_->IterateRoots(&updating_visitor, VISIT_ALL_IN_MINOR_MC_UPDATE);
+    heap()->IterateRoots(&updating_visitor,
+                         base::EnumSet<SkipRoot>{SkipRoot::kExternalStringTable,
+                                                 SkipRoot::kOldGeneration});
   }
   {
     TRACE_GC(heap()->tracer(),
@@ -4915,7 +4919,15 @@ void MinorMarkCompactCollector::MarkRootSetInParallel(
       TRACE_GC(heap()->tracer(), GCTracer::Scope::MINOR_MC_MARK_SEED);
       isolate()->global_handles()->IdentifyWeakUnmodifiedObjects(
           &JSObject::IsUnmodifiedApiObject);
-      heap()->IterateRoots(root_visitor, VISIT_ALL_IN_MINOR_MC_MARK);
+      // MinorMC treats all weak roots except for global handles as strong.
+      // That is why we don't set skip_weak = true here and instead visit
+      // global handles separately.
+      heap()->IterateRoots(
+          root_visitor, base::EnumSet<SkipRoot>{SkipRoot::kExternalStringTable,
+                                                SkipRoot::kGlobalHandles,
+                                                SkipRoot::kOldGeneration});
+      isolate()->global_handles()->IterateYoungStrongAndDependentRoots(
+          root_visitor);
       // Create items for each page.
       RememberedSet<OLD_TO_NEW>::IterateMemoryChunks(
           heap(), [&job, &slots](MemoryChunk* chunk) {

src/heap/scavenger.cc

@@ -296,7 +296,16 @@ void ScavengerCollector::CollectGarbage() {
     {
       // Copy roots.
       TRACE_GC(heap_->tracer(), GCTracer::Scope::SCAVENGER_SCAVENGE_ROOTS);
-      heap_->IterateRoots(&root_scavenge_visitor, VISIT_ALL_IN_SCAVENGE);
+      // Scavenger treats all weak roots except for global handles as strong.
+      // That is why we don't set skip_weak = true here and instead visit
+      // global handles separately.
+      heap_->IterateRoots(
+          &root_scavenge_visitor,
+          base::EnumSet<SkipRoot>{SkipRoot::kExternalStringTable,
+                                  SkipRoot::kGlobalHandles,
+                                  SkipRoot::kOldGeneration});
+      isolate_->global_handles()->IterateYoungStrongAndDependentRoots(
+          &root_scavenge_visitor);
     }
     {
       // Parallel phase scavenging all copied and promoted objects.

src/profiler/heap-snapshot-generator.cc

@@ -1487,7 +1487,11 @@ bool V8HeapExplorer::IterateAndExtractReferences(
   // its custom name to a generic builtin.
   RootsReferencesExtractor extractor(this);
   ReadOnlyRoots(heap_).Iterate(&extractor);
-  heap_->IterateRoots(&extractor, VISIT_ONLY_STRONG);
+  heap_->IterateRoots(&extractor, base::EnumSet<SkipRoot>{SkipRoot::kWeak});
+  // TODO(ulan): The heap snapshot generator incorrectly considers the weak
+  // string tables as strong retainers. Move IterateWeakRoots after
+  // SetVisitingWeakRoots.
+  heap_->IterateWeakRoots(&extractor, {});
   extractor.SetVisitingWeakRoots();
   heap_->IterateWeakGlobalHandles(&extractor);
 

src/snapshot/startup-deserializer.cc

@@ -33,9 +33,12 @@ void StartupDeserializer::DeserializeInto(Isolate* isolate) {
   {
     DisallowHeapAllocation no_gc;
     isolate->heap()->IterateSmiRoots(this);
-    isolate->heap()->IterateStrongRoots(this, VISIT_FOR_SERIALIZATION);
+    isolate->heap()->IterateRoots(
+        this,
+        base::EnumSet<SkipRoot>{SkipRoot::kUnserializable, SkipRoot::kWeak});
     Iterate(isolate, this);
-    isolate->heap()->IterateWeakRoots(this, VISIT_FOR_SERIALIZATION);
+    isolate->heap()->IterateWeakRoots(
+        this, base::EnumSet<SkipRoot>{SkipRoot::kUnserializable});
     DeserializeDeferredObjects();
     RestoreExternalReferenceRedirectors(isolate, accessor_infos());
     RestoreExternalReferenceRedirectors(isolate, call_handler_infos());

src/snapshot/startup-serializer.cc

@@ -179,7 +179,8 @@ void StartupSerializer::SerializeWeakReferencesAndDeferred() {
   Object undefined = ReadOnlyRoots(isolate()).undefined_value();
   VisitRootPointer(Root::kStartupObjectCache, nullptr,
                    FullObjectSlot(&undefined));
-  isolate()->heap()->IterateWeakRoots(this, VISIT_FOR_SERIALIZATION);
+  isolate()->heap()->IterateWeakRoots(
+      this, base::EnumSet<SkipRoot>{SkipRoot::kUnserializable});
   SerializeDeferredObjects();
   Pad();
 }
@@ -199,7 +200,9 @@ void StartupSerializer::SerializeStrongReferences(
   // Visit smi roots and immortal immovables first to make sure they end up in
   // the first page.
   isolate->heap()->IterateSmiRoots(this);
-  isolate->heap()->IterateStrongRoots(this, VISIT_FOR_SERIALIZATION);
+  isolate->heap()->IterateRoots(
+      this,
+      base::EnumSet<SkipRoot>{SkipRoot::kUnserializable, SkipRoot::kWeak});
 }
 
 SerializedHandleChecker::SerializedHandleChecker(Isolate* isolate,