Heap-Snapshot-Generator only needs single pass

Now that V8 uses EphemeronHashTable as backing store for
JSWeakCollections one pass over the heap is enough. In the old
implementation a second pass was necessary to find the owner
of the FixedArray to determine if its entries are strong or weak
references.

Bug: chromium:844008
Change-Id: I04bdf7d480c9be301831698571be5b226d20ac20
Reviewed-on: https://chromium-review.googlesource.com/1090910Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Alexei Filippov <alph@chromium.org>
Commit-Queue: Dominik Inführ <dinfuehr@google.com>
Cr-Commit-Position: refs/heads/master@{#53643}

src/profiler/heap-snapshot-generator.cc

@@ -839,11 +839,7 @@ class IndexedReferencesExtractor : public ObjectVisitor {
   int parent_;
 };
 
-
-bool V8HeapExplorer::ExtractReferencesPass1(int entry, HeapObject* obj) {
-  if (obj->IsFixedArray() || obj->IsEphemeronHashTable())
-    return false;  // FixedArrays & EphemeronHashTables are processed on pass 2
-
+void V8HeapExplorer::ExtractReferences(int entry, HeapObject* obj) {
   if (obj->IsJSGlobalProxy()) {
     ExtractJSGlobalProxyReferences(entry, JSGlobalProxy::cast(obj));
   } else if (obj->IsJSArrayBuffer()) {
@@ -895,22 +891,13 @@ bool V8HeapExplorer::ExtractReferencesPass1(int entry, HeapObject* obj) {
   } else if (obj->IsWeakArrayList()) {
     ExtractWeakArrayReferences(WeakArrayList::kHeaderSize, entry,
                                WeakArrayList::cast(obj));
-  }
-  return true;
-}
-
-
-bool V8HeapExplorer::ExtractReferencesPass2(int entry, HeapObject* obj) {
-  if (!obj->IsFixedArray() && !obj->IsEphemeronHashTable()) return false;
-
-  if (obj->IsContext()) {
+  } else if (obj->IsContext()) {
     ExtractContextReferences(entry, Context::cast(obj));
   } else if (obj->IsEphemeronHashTable()) {
     ExtractEphemeronHashTableReferences(entry, EphemeronHashTable::cast(obj));
-  } else {
+  } else if (obj->IsFixedArray()) {
     ExtractFixedArrayReferences(entry, FixedArray::cast(obj));
   }
-  return true;
 }
 
 
@@ -1581,27 +1568,8 @@ bool V8HeapExplorer::IterateAndExtractReferences(
   extractor.SetVisitingWeakRoots();
   heap_->IterateWeakGlobalHandles(&extractor);
 
-  // We have to do two passes as sometimes FixedArrays are used
-  // to weakly hold their items, and it's impossible to distinguish
-  // between these cases without processing the array owner first.
-  bool interrupted =
-      IterateAndExtractSinglePass<&V8HeapExplorer::ExtractReferencesPass1>() ||
-      IterateAndExtractSinglePass<&V8HeapExplorer::ExtractReferencesPass2>();
-
-  if (interrupted) {
-    filler_ = nullptr;
-    return false;
-  }
-
-  filler_ = nullptr;
-  return progress_->ProgressReport(true);
-}
-
-
-template<V8HeapExplorer::ExtractReferencesMethod extractor>
-bool V8HeapExplorer::IterateAndExtractSinglePass() {
-  // Now iterate the whole heap.
   bool interrupted = false;
+
   HeapIterator iterator(heap_, HeapIterator::kFilterUnreachable);
   // Heap iteration with filtering must be finished in any case.
   for (HeapObject *obj = iterator.next(); obj != nullptr;
@@ -1618,14 +1586,13 @@ bool V8HeapExplorer::IterateAndExtractSinglePass() {
 
     HeapEntry* heap_entry = GetEntry(obj);
     int entry = heap_entry->index();
-    if ((this->*extractor)(entry, obj)) {
-      SetInternalReference(obj, entry,
-                           "map", obj->map(), HeapObject::kMapOffset);
-      // Extract unvisited fields as hidden references and restore tags
-      // of visited fields.
-      IndexedReferencesExtractor refs_extractor(this, obj, entry);
-      obj->Iterate(&refs_extractor);
-    }
+    ExtractReferences(entry, obj);
+    SetInternalReference(obj, entry, "map", obj->map(), HeapObject::kMapOffset);
+    // Extract unvisited fields as hidden references and restore tags
+    // of visited fields.
+    IndexedReferencesExtractor refs_extractor(this, obj, entry);
+    obj->Iterate(&refs_extractor);
+
     // Ensure visited_fields_ doesn't leak to the next object.
     for (size_t i = 0; i < max_pointer; ++i) {
       DCHECK(!visited_fields_[i]);
@@ -1633,7 +1600,14 @@ bool V8HeapExplorer::IterateAndExtractSinglePass() {
 
     if (!progress_->ProgressReport(false)) interrupted = true;
   }
-  return interrupted;
+
+  if (interrupted) {
+    filler_ = nullptr;
+    return false;
+  }
+
+  filler_ = nullptr;
+  return progress_->ProgressReport(true);
 }
 
 
@@ -1926,12 +1900,6 @@ void V8HeapExplorer::TagObject(Object* obj, const char* tag) {
   }
 }
 
-void V8HeapExplorer::TagFixedArraySubType(const FixedArray* array,
-                                          FixedArraySubInstanceType type) {
-  DCHECK(array_types_.find(array) == array_types_.end());
-  array_types_[array] = type;
-}
-
 class GlobalObjectsEnumerator : public RootVisitor {
  public:
   void VisitRootPointers(Root root, const char* description, Object** start,

src/profiler/heap-snapshot-generator.h

@@ -355,9 +355,6 @@ class V8HeapExplorer : public HeapEntriesAllocator {
   static String* GetConstructorName(JSObject* object);
 
  private:
-  typedef bool (V8HeapExplorer::*ExtractReferencesMethod)(int entry,
-                                                          HeapObject* object);
-
   void MarkVisitedField(int offset);
 
   HeapEntry* AddEntry(HeapObject* object);
@@ -367,11 +364,7 @@ class V8HeapExplorer : public HeapEntriesAllocator {
 
   const char* GetSystemEntryName(HeapObject* object);
 
-  template<V8HeapExplorer::ExtractReferencesMethod extractor>
-  bool IterateAndExtractSinglePass();
-
-  bool ExtractReferencesPass1(int entry, HeapObject* obj);
-  bool ExtractReferencesPass2(int entry, HeapObject* obj);
+  void ExtractReferences(int entry, HeapObject* obj);
   void ExtractJSGlobalProxyReferences(int entry, JSGlobalProxy* proxy);
   void ExtractJSObjectReferences(int entry, JSObject* js_obj);
   void ExtractStringReferences(int entry, String* obj);
@@ -461,8 +454,6 @@ class V8HeapExplorer : public HeapEntriesAllocator {
                              Object* child);
   const char* GetStrongGcSubrootName(Object* object);
   void TagObject(Object* obj, const char* tag);
-  void TagFixedArraySubType(const FixedArray* array,
-                            FixedArraySubInstanceType type);
 
   HeapEntry* GetEntry(Object* obj);
 
@@ -475,7 +466,6 @@ class V8HeapExplorer : public HeapEntriesAllocator {
   HeapObjectsSet objects_tags_;
   HeapObjectsSet strong_gc_subroot_names_;
   HeapObjectsSet user_roots_;
-  std::unordered_map<const FixedArray*, FixedArraySubInstanceType> array_types_;
   v8::HeapProfiler::ObjectNameResolver* global_object_name_resolver_;
 
   std::vector<bool> visited_fields_;