[vm] Finer labeling of roots in heap snapshots.

Add missing object id zone roots, which should have been part of ffbbdb5 when they switched from weak to strong.

TEST=examine snapshot after using inspect
Bug: #61036
Change-Id: I3bea765e4ae487babfd86eccbaa87bab80320dcf
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/439160
Reviewed-by: Alexander Aprelev <[email protected]>
Commit-Queue: Ryan Macnak <[email protected]>

Author: rmacnak-google

runtime/vm/heap/become.cc

@@ -365,15 +365,6 @@ void Become::FollowForwardingPointers(Thread* thread) {
   // C++ pointers.
   isolate_group->VisitObjectPointers(&pointer_visitor,
                                      ValidationPolicy::kValidateFrames);
-#ifndef PRODUCT
-  isolate_group->ForEachIsolate(
-      [&](Isolate* isolate) {
-        for (intptr_t i = 0; i < isolate->NumServiceIdZones(); ++i) {
-          isolate->GetServiceIdZone(i)->VisitPointers(pointer_visitor);
-        }
-      },
-      /*at_safepoint=*/true);
-#endif  // !PRODUCT
 
   // Weak persistent handles.
   ForwardHeapPointersHandleVisitor handle_visitor;

runtime/vm/heap/compactor.cc

@@ -469,19 +469,6 @@ void CompactorTask::RunEnteredIsolateGroup() {
           isolate_group_->VisitWeakPersistentHandles(compactor_);
           break;
         }
-#ifndef PRODUCT
-        case 4: {
-          TIMELINE_FUNCTION_GC_DURATION(thread, "ForwardObjectIdRing");
-          isolate_group_->ForEachIsolate(
-              [&](Isolate* isolate) {
-                for (intptr_t i = 0; i < isolate->NumServiceIdZones(); ++i) {
-                  isolate->GetServiceIdZone(i)->VisitPointers(*compactor_);
-                }
-              },
-              /*at_safepoint=*/true);
-          break;
-        }
-#endif  // !PRODUCT
         default:
           more_forwarding_tasks = false;
       }

runtime/vm/heap/incremental_compactor.cc

@@ -574,7 +574,6 @@ class EpilogueState {
   bool TakeOOM() { return oom_slice_.exchange(false); }
   bool TakeWeakHandles() { return weak_handles_slice_.exchange(false); }
   bool TakeWeakTables() { return weak_tables_slice_.exchange(false); }
-  bool TakeIdRing() { return id_ring_slice_.exchange(false); }
   bool TakeRoots() { return roots_slice_.exchange(false); }
   bool TakeResetProgressBars() {
     return reset_progress_bars_slice_.exchange(false);
@@ -634,18 +633,6 @@ class EpilogueTask : public SafepointTask {
       TIMELINE_FUNCTION_GC_DURATION(thread, "WeakTables");
       isolate_group_->heap()->ForwardWeakTables(&visitor);
     }
-#ifndef PRODUCT
-    if (state_->TakeIdRing()) {
-      TIMELINE_FUNCTION_GC_DURATION(thread, "IdRing");
-      isolate_group_->ForEachIsolate(
-          [&](Isolate* isolate) {
-            for (intptr_t i = 0; i < isolate->NumServiceIdZones(); ++i) {
-              isolate->GetServiceIdZone(i)->VisitPointers(visitor);
-            }
-          },
-          /*at_safepoint=*/true);
-    }
-#endif  // !PRODUCT
 
     barrier_->Sync();
 

runtime/vm/heap/marker.cc

@@ -751,8 +751,7 @@ void GCMarker::Epilogue() {}
 
 enum RootSlices {
   kIsolate = 0,
-  kObjectIdRing = 1,
-  kNumFixedRootSlices = 2,
+  kNumFixedRootSlices = 1,
 };
 
 void GCMarker::ResetSlices() {
@@ -774,18 +773,13 @@ void GCMarker::IterateRoots(ObjectPointerVisitor* visitor) {
 
     switch (slice) {
       case kIsolate: {
+        // TODO(gc): Split this by isolate?
         TIMELINE_FUNCTION_GC_DURATION(Thread::Current(),
                                       "ProcessIsolateGroupRoots");
         isolate_group_->VisitObjectPointers(
             visitor, ValidationPolicy::kDontValidateFrames);
         break;
       }
-      case kObjectIdRing: {
-        TIMELINE_FUNCTION_GC_DURATION(Thread::Current(),
-                                      "ProcessObjectIdTable");
-        isolate_group_->VisitPointersInAllServiceIdZones(*visitor);
-        break;
-      }
     }
 
     MonitorLocker ml(&root_slices_monitor_);

runtime/vm/heap/scavenger.cc

@@ -1192,33 +1192,22 @@ void Scavenger::IterateRememberedCards(ScavengerVisitor* visitor) {
   heap_->old_space()->VisitRememberedCards(visitor);
 }
 
-void Scavenger::IterateObjectIdTable(ObjectPointerVisitor* visitor) {
-#ifndef PRODUCT
-  TIMELINE_FUNCTION_GC_DURATION(Thread::Current(), "IterateObjectIdTable");
-  heap_->isolate_group()->VisitPointersInAllServiceIdZones(*visitor);
-#endif  // !PRODUCT
-}
-
 enum RootSlices {
   kIsolate = 0,
-  kObjectIdRing,
-  kNumRootSlices,
+  kNumFixedRootSlices = 1,
 };
 
 void Scavenger::IterateRoots(ScavengerVisitor* visitor) {
   for (;;) {
     intptr_t slice = root_slices_started_.fetch_add(1);
-    if (slice >= kNumRootSlices) {
+    if (slice >= kNumFixedRootSlices) {
       break;  // No more slices.
     }
-
     switch (slice) {
       case kIsolate:
+        // TODO(gc): Split this by isolate?
         IterateIsolateRoots(visitor);
         break;
-      case kObjectIdRing:
-        IterateObjectIdTable(visitor);
-        break;
       default:
         UNREACHABLE();
     }

runtime/vm/heap/scavenger.h

@@ -281,7 +281,6 @@ class Scavenger {
   void IterateIsolateRoots(ObjectPointerVisitor* visitor);
   void IterateStoreBuffers(ScavengerVisitor* visitor);
   void IterateRememberedCards(ScavengerVisitor* visitor);
-  void IterateObjectIdTable(ObjectPointerVisitor* visitor);
   void IterateRoots(ScavengerVisitor* visitor);
   void IterateWeak();
   void MournWeakHandles();

runtime/vm/isolate.cc

@@ -2935,53 +2935,95 @@ void IsolateGroup::VisitObjectPointers(ObjectPointerVisitor* visitor,
   VisitStackPointers(visitor, validate_frames);
 }
 
-void IsolateGroup::VisitSharedPointers(ObjectPointerVisitor* visitor) {
-  // Visit objects in the class table.
-  class_table()->VisitObjectPointers(visitor);
-  if (heap_walk_class_table() != class_table()) {
-    heap_walk_class_table()->VisitObjectPointers(visitor);
-  }
-  api_state()->VisitObjectPointersUnlocked(visitor);
-  // Visit objects in the object store.
-  if (object_store() != nullptr) {
-    object_store()->VisitObjectPointers(visitor);
-  }
-  visitor->VisitPointer(reinterpret_cast<ObjectPtr*>(&saved_unlinked_calls_));
-  initial_field_table()->VisitObjectPointers(visitor);
-  sentinel_field_table()->VisitObjectPointers(visitor);
-  shared_initial_field_table()->VisitObjectPointers(visitor);
-  shared_field_table()->VisitObjectPointers(visitor);
-
-  // Visit the boxed_field_list_.
-  // 'boxed_field_list_' access via mutator and background compilation threads
-  // is guarded with a monitor. This means that we can visit it only
-  // when at safepoint or the field_list_mutex_ lock has been taken.
-  visitor->VisitPointer(reinterpret_cast<ObjectPtr*>(&boxed_field_list_));
-
-  NOT_IN_PRECOMPILED(background_compiler()->VisitPointers(visitor));
-
+void IsolateGroup::VisitSharedPointers(ObjectPointerVisitor* visitor,
+                                       intptr_t slice) {
+  switch (slice) {
+    case kClassTable:
+      class_table()->VisitObjectPointers(visitor);
+      if (heap_walk_class_table() != class_table()) {
+        heap_walk_class_table()->VisitObjectPointers(visitor);
+      }
+      break;
+    case kApiState:
+      api_state()->VisitObjectPointersUnlocked(visitor);
+      break;
+    case kObjectStore:
+      if (object_store() != nullptr) {
+        object_store()->VisitObjectPointers(visitor);
+      }
+      break;
+    case kSavedUnlinkedCalls:
+      visitor->VisitPointer(
+          reinterpret_cast<ObjectPtr*>(&saved_unlinked_calls_));
+      break;
+    case kInitialFieldTable:
+      initial_field_table()->VisitObjectPointers(visitor);
+      break;
+    case kSentinelFieldTable:
+      sentinel_field_table()->VisitObjectPointers(visitor);
+      break;
+    case kSharedInitialFieldTable:
+      shared_initial_field_table()->VisitObjectPointers(visitor);
+      break;
+    case kSharedFieldTable:
+      shared_field_table()->VisitObjectPointers(visitor);
+      break;
+    case kBoxedFieldList:
+      // Visit the boxed_field_list_.
+      // 'boxed_field_list_' access via mutator and background compilation
+      // threads is guarded with a monitor. This means that we can visit it only
+      // when at safepoint or the field_list_mutex_ lock has been taken.
+      visitor->VisitPointer(reinterpret_cast<ObjectPtr*>(&boxed_field_list_));
+      break;
+    case kBackgroundCompiler:
+      NOT_IN_PRECOMPILED(background_compiler()->VisitPointers(visitor));
+      break;
+    case kDebugger:
 #if !defined(PRODUCT)
-  if (debugger() != nullptr) {
-    debugger()->VisitObjectPointers(visitor);
-  }
+      if (debugger() != nullptr) {
+        debugger()->VisitObjectPointers(visitor);
+      }
 #endif
-
+      break;
+    case kReloadContext:
 #if !defined(PRODUCT) && !defined(DART_PRECOMPILED_RUNTIME)
-  // Visit objects that are being used for isolate reload.
-  if (program_reload_context() != nullptr) {
-    program_reload_context()->VisitObjectPointers(visitor);
-    program_reload_context()->group_reload_context()->VisitObjectPointers(
-        visitor);
-  }
+      if (program_reload_context() != nullptr) {
+        program_reload_context()->VisitObjectPointers(visitor);
+        program_reload_context()->group_reload_context()->VisitObjectPointers(
+            visitor);
+      }
 #endif  // !defined(PRODUCT) && !defined(DART_PRECOMPILED_RUNTIME)
-
-  if (source()->loaded_blobs_ != nullptr) {
-    visitor->VisitPointer(
-        reinterpret_cast<ObjectPtr*>(&(source()->loaded_blobs_)));
+      break;
+    case kLoadedBlobs:
+      if (source()->loaded_blobs_ != nullptr) {
+        visitor->VisitPointer(
+            reinterpret_cast<ObjectPtr*>(&(source()->loaded_blobs_)));
+      }
+      break;
+    case kBecome:
+      if (become() != nullptr) {
+        become()->VisitObjectPointers(visitor);
+      }
+      break;
+    case kObjectIdZones:
+#if !defined(PRODUCT)
+      if (visitor->trace_object_id_rings()) {
+        for (Isolate* isolate : isolates_) {
+          for (intptr_t i = 0; i < isolate->NumServiceIdZones(); ++i) {
+            isolate->GetServiceIdZone(i)->VisitPointers(visitor);
+          }
+        }
+      }
+#endif  // !defined(PRODUCT)
+      break;
+    default:
+      UNREACHABLE();
   }
+}
 
-  if (become() != nullptr) {
-    become()->VisitObjectPointers(visitor);
+void IsolateGroup::VisitSharedPointers(ObjectPointerVisitor* visitor) {
+  for (intptr_t i = 0; i < kNumRootSlices; i++) {
+    VisitSharedPointers(visitor, static_cast<RootSlice>(i));
   }
 }
 
@@ -3002,17 +3044,6 @@ void IsolateGroup::VisitStackPointers(ObjectPointerVisitor* visitor,
   visitor->clear_gc_root_type();
 }
 
-void IsolateGroup::VisitPointersInAllServiceIdZones(
-    ObjectPointerVisitor& visitor) {
-#if !defined(PRODUCT)
-  for (Isolate* isolate : isolates_) {
-    for (intptr_t i = 0; i < isolate->NumServiceIdZones(); ++i) {
-      isolate->GetServiceIdZone(i)->VisitPointers(visitor);
-    }
-  }
-#endif  // !defined(PRODUCT)
-}
-
 void IsolateGroup::VisitWeakPersistentHandles(HandleVisitor* visitor) {
   api_state()->VisitWeakHandlesUnlocked(visitor);
 }

runtime/vm/isolate.h

@@ -262,6 +262,63 @@ class MutatorThreadPool : public ThreadPool {
   IsolateGroup* isolate_group_ = nullptr;
 };
 
+enum RootSlice : intptr_t {
+  kClassTable,
+  kApiState,
+  kObjectStore,
+  kSavedUnlinkedCalls,
+  kInitialFieldTable,
+  kSentinelFieldTable,
+  kSharedInitialFieldTable,
+  kSharedFieldTable,
+  kBoxedFieldList,
+  kBackgroundCompiler,
+  kDebugger,
+  kReloadContext,
+  kLoadedBlobs,
+  kBecome,
+  kObjectIdZones,
+
+  kNumRootSlices,
+};
+
+inline const char* RootSliceToCString(intptr_t slice) {
+  switch (slice) {
+    case kClassTable:
+      return "class table";
+    case kApiState:
+      return "api state";
+    case kObjectStore:
+      return "group object store";
+    case kSavedUnlinkedCalls:
+      return "saved unlinked calls";
+    case kInitialFieldTable:
+      return "initial field table";
+    case kSentinelFieldTable:
+      return "sentinel field table";
+    case kSharedInitialFieldTable:
+      return "shared initial field table";
+    case kSharedFieldTable:
+      return "shared field table";
+    case kBoxedFieldList:
+      return "boxed field list";
+    case kBackgroundCompiler:
+      return "background compiler";
+    case kDebugger:
+      return "debugger";
+    case kReloadContext:
+      return "reload context";
+    case kLoadedBlobs:
+      return "loaded blobs";
+    case kBecome:
+      return "become";
+    case kObjectIdZones:
+      return "object id zones";
+    default:
+      return "?";
+  }
+}
+
 // Represents an isolate group and is shared among all isolates within a group.
 class IsolateGroup : public IntrusiveDListEntry<IsolateGroup> {
  public:
@@ -692,9 +749,9 @@ class IsolateGroup : public IntrusiveDListEntry<IsolateGroup> {
   void VisitObjectPointers(ObjectPointerVisitor* visitor,
                            ValidationPolicy validate_frames);
   void VisitSharedPointers(ObjectPointerVisitor* visitor);
+  void VisitSharedPointers(ObjectPointerVisitor* visitor, intptr_t slice);
   void VisitStackPointers(ObjectPointerVisitor* visitor,
                           ValidationPolicy validate_frames);
-  void VisitPointersInAllServiceIdZones(ObjectPointerVisitor& visitor);
   void VisitWeakPersistentHandles(HandleVisitor* visitor);
 
   // In precompilation we finalize all regular classes before compiling.