[heap-profiler] Simplify snapshotting of the roots.

This replaces three passes over the roots with a single pass.
This also removes root synchronization logic.

The GC subroot index is computed from the |root| parameter of the visit
method. The new |description| parameter is used as an edge name.

Bug: chromium:811842
Change-Id: I03a9215d56b54b3eb5f7bc8b32d5b22ad091c68b
Reviewed-on: https://chromium-review.googlesource.com/916781Reviewed-by: Alexei Filippov <alph@chromium.org>
Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#51350}

src/builtins/builtins.cc

@@ -86,8 +86,9 @@ Builtins::Name Builtins::GetBuiltinFromBailoutId(BailoutId id) {
 void Builtins::TearDown() { initialized_ = false; }
 
 void Builtins::IterateBuiltins(RootVisitor* v) {
-  v->VisitRootPointers(Root::kBuiltins, nullptr, &builtins_[0],
-                       &builtins_[0] + builtin_count);
+  for (int i = 0; i < builtin_count; i++) {
+    v->VisitRootPointer(Root::kBuiltins, name(i), &builtins_[i]);
+  }
 }
 
 const char* Builtins::Lookup(byte* pc) {

src/global-handles.cc

@@ -907,6 +907,13 @@ void GlobalHandles::IterateStrongRoots(RootVisitor* v) {
   }
 }
 
+void GlobalHandles::IterateWeakRoots(RootVisitor* v) {
+  for (NodeIterator it(this); !it.done(); it.Advance()) {
+    if (it.node()->IsWeak()) {
+      v->VisitRootPointer(Root::kGlobalHandles, nullptr, it.node()->location());
+    }
+  }
+}
 
 DISABLE_CFI_PERF
 void GlobalHandles::IterateAllRoots(RootVisitor* v) {

src/global-handles.h

@@ -108,10 +108,10 @@ class GlobalHandles {
   int PostGarbageCollectionProcessing(
       GarbageCollector collector, const v8::GCCallbackFlags gc_callback_flags);
 
-  // Iterates over all strong handles.
   void IterateStrongRoots(RootVisitor* v);
 
-  // Iterates over all handles.
+  void IterateWeakRoots(RootVisitor* v);
+
   void IterateAllRoots(RootVisitor* v);
 
   void IterateAllNewSpaceRoots(RootVisitor* v);

src/heap/heap.cc

@@ -462,30 +462,6 @@ bool Heap::IsRetainingPathTarget(HeapObject* object,
   return false;
 }
 
-namespace {
-const char* RootToString(Root root) {
-  switch (root) {
-#define ROOT_CASE(root_id, ignore, description) \
-  case Root::root_id:                           \
-    return description;
-    ROOT_ID_LIST(ROOT_CASE)
-#undef ROOT_CASE
-    case Root::kCodeFlusher:
-      return "(Code flusher)";
-    case Root::kPartialSnapshotCache:
-      return "(Partial snapshot cache)";
-    case Root::kWeakCollections:
-      return "(Weak collections)";
-    case Root::kWrapperTracing:
-      return "(Wrapper tracing)";
-    case Root::kUnknown:
-      return "(Unknown)";
-  }
-  UNREACHABLE();
-  return nullptr;
-}
-}  // namespace
-
 void Heap::PrintRetainingPath(HeapObject* target, RetainingPathOption option) {
   PrintF("\n\n\n");
   PrintF("#################################################\n");
@@ -528,7 +504,7 @@ void Heap::PrintRetainingPath(HeapObject* target, RetainingPathOption option) {
   }
   PrintF("\n");
   PrintF("^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n");
-  PrintF("Root: %s\n", RootToString(root));
+  PrintF("Root: %s\n", RootVisitor::RootName(root));
   PrintF("-------------------------------------------------\n");
 }
 
@@ -5146,6 +5122,9 @@ void Heap::IterateStrongRoots(RootVisitor* v, VisitMode mode) {
   }
 }
 
+void Heap::IterateWeakGlobalHandles(RootVisitor* v) {
+  isolate_->global_handles()->IterateWeakRoots(v);
+}
 
 // TODO(1236194): Since the heap size is configurable on the command line
 // and through the API, we should gracefully handle the case that the heap

src/heap/heap.h

@@ -1167,15 +1167,15 @@ class Heap {
   // Iterators. ================================================================
   // ===========================================================================
 
-  // Iterates over all roots in the heap.
   void IterateRoots(RootVisitor* v, VisitMode mode);
-  // Iterates over all strong roots in the heap.
   void IterateStrongRoots(RootVisitor* v, VisitMode mode);
   // 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 all the other roots in the heap.
+  // Iterates over weak string tables.
   void IterateWeakRoots(RootVisitor* v, VisitMode mode);
+  // Iterates over weak global handles.
+  void IterateWeakGlobalHandles(RootVisitor* v);
 
   // ===========================================================================
   // Store buffer API. =========================================================

src/profiler/heap-snapshot-generator.h

@@ -165,8 +165,8 @@ class HeapSnapshot {
   HeapProfiler* profiler() { return profiler_; }
   HeapEntry* root() { return &entries_[root_index_]; }
   HeapEntry* gc_roots() { return &entries_[gc_roots_index_]; }
-  HeapEntry* gc_subroot(int index) {
-    return &entries_[gc_subroot_indexes_[index]];
+  HeapEntry* gc_subroot(Root root) {
+    return &entries_[gc_subroot_indexes_[static_cast<int>(root)]];
   }
   std::vector<HeapEntry>& entries() { return entries_; }
   std::deque<HeapGraphEdge>& edges() { return edges_; }
@@ -191,12 +191,12 @@ class HeapSnapshot {
  private:
   HeapEntry* AddRootEntry();
   HeapEntry* AddGcRootsEntry();
-  HeapEntry* AddGcSubrootEntry(int tag, SnapshotObjectId id);
+  HeapEntry* AddGcSubrootEntry(Root root, SnapshotObjectId id);
 
   HeapProfiler* profiler_;
   int root_index_;
   int gc_roots_index_;
-  int gc_subroot_indexes_[VisitorSynchronization::kNumberOfSyncTags];
+  int gc_subroot_indexes_[static_cast<int>(Root::kNumberOfRoots)];
   std::vector<HeapEntry> entries_;
   std::deque<HeapGraphEdge> edges_;
   std::deque<HeapGraphEdge*> children_;
@@ -445,9 +445,9 @@ class V8HeapExplorer : public HeapEntriesAllocator {
 
   void SetUserGlobalReference(Object* user_global);
   void SetRootGcRootsReference();
-  void SetGcRootsReference(VisitorSynchronization::SyncTag tag);
-  void SetGcSubrootReference(
-      VisitorSynchronization::SyncTag tag, bool is_weak, Object* child);
+  void SetGcRootsReference(Root root);
+  void SetGcSubrootReference(Root root, const char* description, bool is_weak,
+                             Object* child);
   const char* GetStrongGcSubrootName(Object* object);
   void TagObject(Object* obj, const char* tag);
   void TagFixedArraySubType(const FixedArray* array,

src/visitors.cc

@@ -9,16 +9,19 @@
 namespace v8 {
 namespace internal {
 
-#define DECLARE_TAG(ignore1, name, ignore2) name,
-const char* const
-    VisitorSynchronization::kTags[VisitorSynchronization::kNumberOfSyncTags] = {
-        ROOT_ID_LIST(DECLARE_TAG)};
-#undef DECLARE_TAG
-
-#define DECLARE_TAG(ignore1, ignore2, name) name,
-const char* const VisitorSynchronization::kTagNames
-    [VisitorSynchronization::kNumberOfSyncTags] = {ROOT_ID_LIST(DECLARE_TAG)};
-#undef DECLARE_TAG
+const char* RootVisitor::RootName(Root root) {
+  switch (root) {
+#define ROOT_CASE(root_id, description) \
+  case Root::root_id:                   \
+    return description;
+    ROOT_ID_LIST(ROOT_CASE)
+#undef ROOT_CASE
+    case Root::kNumberOfRoots:
+      break;
+  }
+  UNREACHABLE();
+  return nullptr;
+}
 
 }  // namespace internal
 }  // namespace v8

src/visitors.h

@@ -13,45 +13,42 @@ namespace internal {
 class CodeDataContainer;
 class Object;
 
-#define ROOT_ID_LIST(V)                                                    \
-  V(kStringTable, "string_table", "(Internalized strings)")                \
-  V(kExternalStringsTable, "external_strings_table", "(External strings)") \
-  V(kStrongRootList, "strong_root_list", "(Strong roots)")                 \
-  V(kSmiRootList, "smi_root_list", "(Smi roots)")                          \
-  V(kBootstrapper, "bootstrapper", "(Bootstrapper)")                       \
-  V(kTop, "top", "(Isolate)")                                              \
-  V(kRelocatable, "relocatable", "(Relocatable)")                          \
-  V(kDebug, "debug", "(Debugger)")                                         \
-  V(kCompilationCache, "compilationcache", "(Compilation cache)")          \
-  V(kHandleScope, "handlescope", "(Handle scope)")                         \
-  V(kDispatchTable, "dispatchtable", "(Dispatch table)")                   \
-  V(kBuiltins, "builtins", "(Builtins)")                                   \
-  V(kGlobalHandles, "globalhandles", "(Global handles)")                   \
-  V(kEternalHandles, "eternalhandles", "(Eternal handles)")                \
-  V(kThreadManager, "threadmanager", "(Thread manager)")                   \
-  V(kStrongRoots, "strong roots", "(Strong roots)")                        \
-  V(kExtensions, "Extensions", "(Extensions)")
+#define ROOT_ID_LIST(V)                                \
+  V(kStringTable, "(Internalized strings)")            \
+  V(kExternalStringsTable, "(External strings)")       \
+  V(kStrongRootList, "(Strong roots)")                 \
+  V(kSmiRootList, "(Smi roots)")                       \
+  V(kBootstrapper, "(Bootstrapper)")                   \
+  V(kTop, "(Isolate)")                                 \
+  V(kRelocatable, "(Relocatable)")                     \
+  V(kDebug, "(Debugger)")                              \
+  V(kCompilationCache, "(Compilation cache)")          \
+  V(kHandleScope, "(Handle scope)")                    \
+  V(kDispatchTable, "(Dispatch table)")                \
+  V(kBuiltins, "(Builtins)")                           \
+  V(kGlobalHandles, "(Global handles)")                \
+  V(kEternalHandles, "(Eternal handles)")              \
+  V(kThreadManager, "(Thread manager)")                \
+  V(kStrongRoots, "(Strong roots)")                    \
+  V(kExtensions, "(Extensions)")                       \
+  V(kCodeFlusher, "(Code flusher)")                    \
+  V(kPartialSnapshotCache, "(Partial snapshot cache)") \
+  V(kWeakCollections, "(Weak collections)")            \
+  V(kWrapperTracing, "(Wrapper tracing)")              \
+  V(kUnknown, "(Unknown)")
 
 class VisitorSynchronization : public AllStatic {
  public:
-#define DECLARE_ENUM(enum_item, ignore1, ignore2) enum_item,
+#define DECLARE_ENUM(enum_item, ignore) enum_item,
   enum SyncTag { ROOT_ID_LIST(DECLARE_ENUM) kNumberOfSyncTags };
 #undef DECLARE_ENUM
-
-  static const char* const kTags[kNumberOfSyncTags];
-  static const char* const kTagNames[kNumberOfSyncTags];
 };
 
 enum class Root {
-#define DECLARE_ENUM(enum_item, ignore1, ignore2) enum_item,
+#define DECLARE_ENUM(enum_item, ignore) enum_item,
   ROOT_ID_LIST(DECLARE_ENUM)
 #undef DECLARE_ENUM
-  // TODO(ulan): Merge with the ROOT_ID_LIST.
-  kCodeFlusher,
-  kPartialSnapshotCache,
-  kWeakCollections,
-  kWrapperTracing,
-  kUnknown
+      kNumberOfRoots
 };
 
 // Abstract base class for visiting, and optionally modifying, the
@@ -76,6 +73,8 @@ class RootVisitor BASE_EMBEDDED {
   // Also used for marking up GC roots in heap snapshots.
   // TODO(ulan): Remove this.
   virtual void Synchronize(VisitorSynchronization::SyncTag tag) {}
+
+  static const char* RootName(Root root);
 };
 
 // Abstract base class for visiting, and optionally modifying, the