Remove HeapSnapshotsCollection class

The class added unecessary level of indirection to the heap profiler implementation. I merged some of its implementation into HeapProfiler and deleted the rest. This refactoring is also a prerequisite for keeping HeapObjectsMap alive even when all snapshots are deleted.

BUG=None
LOG=N
R=alph@chromium.org, mstarzinger@chromium.org

Review URL: https://codereview.chromium.org/101393002

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@18221 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/heap-profiler.cc

@@ -28,28 +28,41 @@
 #include "v8.h"
 
 #include "heap-profiler.h"
+
+#include "allocation-tracker.h"
 #include "heap-snapshot-generator-inl.h"
 
 namespace v8 {
 namespace internal {
 
 HeapProfiler::HeapProfiler(Heap* heap)
-    : snapshots_(new HeapSnapshotsCollection(heap)),
+    : ids_(new HeapObjectsMap(heap)),
+      names_(new StringsStorage(heap)),
       next_snapshot_uid_(1),
-      is_tracking_allocations_(false),
       is_tracking_object_moves_(false) {
 }
 
 
+static void DeleteHeapSnapshot(HeapSnapshot** snapshot_ptr) {
+  delete *snapshot_ptr;
+}
+
+
 HeapProfiler::~HeapProfiler() {
-  delete snapshots_;
+  snapshots_.Iterate(DeleteHeapSnapshot);
+  snapshots_.Clear();
 }
 
 
 void HeapProfiler::DeleteAllSnapshots() {
-  Heap* the_heap = heap();
-  delete snapshots_;
-  snapshots_ = new HeapSnapshotsCollection(the_heap);
+  snapshots_.Iterate(DeleteHeapSnapshot);
+  snapshots_.Clear();
+  names_.Reset(new StringsStorage(heap()));
+}
+
+
+void HeapProfiler::RemoveSnapshot(HeapSnapshot* snapshot) {
+  snapshots_.RemoveElement(snapshot);
 }
 
 
@@ -76,15 +89,17 @@ HeapSnapshot* HeapProfiler::TakeSnapshot(
     const char* name,
     v8::ActivityControl* control,
     v8::HeapProfiler::ObjectNameResolver* resolver) {
-  HeapSnapshot* result = snapshots_->NewSnapshot(name, next_snapshot_uid_++);
+  HeapSnapshot* result = new HeapSnapshot(this, name, next_snapshot_uid_++);
   {
     HeapSnapshotGenerator generator(result, control, resolver, heap());
     if (!generator.GenerateSnapshot()) {
       delete result;
       result = NULL;
+    } else {
+      snapshots_.Add(result);
     }
   }
-  snapshots_->SnapshotGenerationFinished(result);
+  ids_->RemoveDeadEntries();
   is_tracking_object_moves_ = true;
   return result;
 }
@@ -94,69 +109,79 @@ HeapSnapshot* HeapProfiler::TakeSnapshot(
     String* name,
     v8::ActivityControl* control,
     v8::HeapProfiler::ObjectNameResolver* resolver) {
-  return TakeSnapshot(snapshots_->names()->GetName(name), control, resolver);
+  return TakeSnapshot(names_->GetName(name), control, resolver);
 }
 
 
 void HeapProfiler::StartHeapObjectsTracking(bool track_allocations) {
-  snapshots_->StartHeapObjectsTracking(track_allocations);
+  ids_->UpdateHeapObjectsMap();
   is_tracking_object_moves_ = true;
-  ASSERT(!is_tracking_allocations_);
+  ASSERT(!is_tracking_allocations());
   if (track_allocations) {
+    allocation_tracker_.Reset(new AllocationTracker(*ids_, *names_));
     heap()->DisableInlineAllocation();
-    is_tracking_allocations_ = true;
   }
 }
 
 
 SnapshotObjectId HeapProfiler::PushHeapObjectsStats(OutputStream* stream) {
-  return snapshots_->PushHeapObjectsStats(stream);
+  return ids_->PushHeapObjectsStats(stream);
 }
 
 
 void HeapProfiler::StopHeapObjectsTracking() {
-  snapshots_->StopHeapObjectsTracking();
-  if (is_tracking_allocations_) {
+  ids_->StopHeapObjectsTracking();
+  if (is_tracking_allocations()) {
+    allocation_tracker_.Reset(NULL);
     heap()->EnableInlineAllocation();
-    is_tracking_allocations_ = false;
   }
 }
 
 
 size_t HeapProfiler::GetMemorySizeUsedByProfiler() {
-  return snapshots_->GetUsedMemorySize();
+  size_t size = sizeof(*this);
+  size += names_->GetUsedMemorySize();
+  size += ids_->GetUsedMemorySize();
+  size += GetMemoryUsedByList(snapshots_);
+  for (int i = 0; i < snapshots_.length(); ++i) {
+    size += snapshots_[i]->RawSnapshotSize();
+  }
+  return size;
 }
 
 
 int HeapProfiler::GetSnapshotsCount() {
-  return snapshots_->snapshots()->length();
+  return snapshots_.length();
 }
 
 
 HeapSnapshot* HeapProfiler::GetSnapshot(int index) {
-  return snapshots_->snapshots()->at(index);
+  return snapshots_.at(index);
 }
 
 
 SnapshotObjectId HeapProfiler::GetSnapshotObjectId(Handle<Object> obj) {
   if (!obj->IsHeapObject())
     return v8::HeapProfiler::kUnknownObjectId;
-  return snapshots_->FindObjectId(HeapObject::cast(*obj)->address());
+  return ids_->FindEntry(HeapObject::cast(*obj)->address());
 }
 
 
 void HeapProfiler::ObjectMoveEvent(Address from, Address to, int size) {
-  snapshots_->ObjectMoveEvent(from, to, size);
+  ids_->MoveObject(from, to, size);
 }
 
 
 void HeapProfiler::AllocationEvent(Address addr, int size) {
-  snapshots_->AllocationEvent(addr, size);
+  DisallowHeapAllocation no_allocation;
+  if (!allocation_tracker_.is_empty()) {
+    allocation_tracker_->AllocationEvent(addr, size);
+  }
 }
 
 
 void HeapProfiler::UpdateObjectSizeEvent(Address addr, int size) {
-  snapshots_->UpdateObjectSizeEvent(addr, size);
+  ids_->UpdateObjectSize(addr, size);
 }
 
 
@@ -167,4 +192,24 @@ void HeapProfiler::SetRetainedObjectInfo(UniqueId id,
 }
 
 
+Handle<HeapObject> HeapProfiler::FindHeapObjectById(SnapshotObjectId id) {
+  heap()->CollectAllGarbage(Heap::kMakeHeapIterableMask,
+                            "HeapProfiler::FindHeapObjectById");
+  DisallowHeapAllocation no_allocation;
+  HeapObject* object = NULL;
+  HeapIterator iterator(heap(), HeapIterator::kFilterUnreachable);
+  // Make sure that object with the given id is still reachable.
+  for (HeapObject* obj = iterator.next();
+       obj != NULL;
+       obj = iterator.next()) {
+    if (ids_->FindEntry(obj->address()) == id) {
+      ASSERT(object == NULL);
+      object = obj;
+      // Can't break -- kFilterUnreachable requires full heap traversal.
+    }
+  }
+  return object != NULL ? Handle<HeapObject>(object) : Handle<HeapObject>();
+}
+
+
 } }  // namespace v8::internal

src/heap-profiler.h

@@ -30,12 +30,12 @@
 
 #include "heap-snapshot-generator-inl.h"
 #include "isolate.h"
+#include "smart-pointers.h"
 
 namespace v8 {
 namespace internal {
 
 class HeapSnapshot;
-class HeapSnapshotsCollection;
 
 class HeapProfiler {
  public:
@@ -55,12 +55,16 @@ class HeapProfiler {
 
   void StartHeapObjectsTracking(bool track_allocations);
   void StopHeapObjectsTracking();
+  AllocationTracker* allocation_tracker() { return *allocation_tracker_; }
+  HeapObjectsMap* heap_object_map() { return *ids_; }
+  StringsStorage* names() { return *names_; }
 
   SnapshotObjectId PushHeapObjectsStats(OutputStream* stream);
   int GetSnapshotsCount();
   HeapSnapshot* GetSnapshot(int index);
   SnapshotObjectId GetSnapshotObjectId(Handle<Object> obj);
   void DeleteAllSnapshots();
+  void RemoveSnapshot(HeapSnapshot* snapshot);
 
   void ObjectMoveEvent(Address from, Address to, int size);
 
@@ -76,19 +80,20 @@ class HeapProfiler {
   void SetRetainedObjectInfo(UniqueId id, RetainedObjectInfo* info);
 
   bool is_tracking_object_moves() const { return is_tracking_object_moves_; }
-  bool is_tracking_allocations() const { return is_tracking_allocations_; }
+  bool is_tracking_allocations() { return !allocation_tracker_.is_empty(); }
 
-  int FindUntrackedObjects() {
-    return snapshots_->FindUntrackedObjects();
-  }
+  Handle<HeapObject> FindHeapObjectById(SnapshotObjectId id);
 
  private:
-  Heap* heap() const { return snapshots_->heap(); }
+  Heap* heap() const { return ids_->heap(); }
 
-  HeapSnapshotsCollection* snapshots_;
+  // Mapping from HeapObject addresses to objects' uids.
+  SmartPointer<HeapObjectsMap> ids_;
+  List<HeapSnapshot*> snapshots_;
+  SmartPointer<StringsStorage> names_;
   unsigned next_snapshot_uid_;
   List<v8::HeapProfiler::WrapperInfoCallback> wrapper_callbacks_;
-  bool is_tracking_allocations_;
+  SmartPointer<AllocationTracker> allocation_tracker_;
   bool is_tracking_object_moves_;
 };
 

src/heap-snapshot-generator.h

@@ -154,21 +154,19 @@ class HeapEntry BASE_EMBEDDED {
 };
 
 
-class HeapSnapshotsCollection;
-
 // HeapSnapshot represents a single heap snapshot. It is stored in
-// HeapSnapshotsCollection, which is also a factory for
+// HeapProfiler, which is also a factory for
 // HeapSnapshots. All HeapSnapshots share strings copied from JS heap
 // to be able to return them even if they were collected.
 // HeapSnapshotGenerator fills in a HeapSnapshot.
 class HeapSnapshot {
  public:
-  HeapSnapshot(HeapSnapshotsCollection* collection,
+  HeapSnapshot(HeapProfiler* profiler,
                const char* title,
                unsigned uid);
   void Delete();
 
-  HeapSnapshotsCollection* collection() { return collection_; }
+  HeapProfiler* profiler() { return profiler_; }
   const char* title() { return title_; }
   unsigned uid() { return uid_; }
   size_t RawSnapshotSize() const;
@@ -202,7 +200,7 @@ class HeapSnapshot {
   void PrintEntriesSize();
 
  private:
-  HeapSnapshotsCollection* collection_;
+  HeapProfiler* profiler_;
   const char* title_;
   unsigned uid_;
   int root_index_;
@@ -227,7 +225,6 @@ class HeapObjectsMap {
 
   Heap* heap() const { return heap_; }
 
-  void SnapshotGenerationFinished();
   SnapshotObjectId FindEntry(Address addr);
   SnapshotObjectId FindOrAddEntry(Address addr,
                                   unsigned int size,
@@ -242,7 +239,7 @@ class HeapObjectsMap {
   SnapshotObjectId PushHeapObjectsStats(OutputStream* stream);
   size_t GetUsedMemorySize() const;
 
-  static SnapshotObjectId GenerateId(Heap* heap, v8::RetainedObjectInfo* info);
+  SnapshotObjectId GenerateId(v8::RetainedObjectInfo* info);
   static inline SnapshotObjectId GetNthGcSubrootId(int delta);
 
   static const int kObjectIdStep = 2;
@@ -255,6 +252,7 @@ class HeapObjectsMap {
   int FindUntrackedObjects();
 
   void UpdateHeapObjectsMap();
+  void RemoveDeadEntries();
 
  private:
   struct EntryInfo {
@@ -274,8 +272,6 @@ class HeapObjectsMap {
     uint32_t count;
   };
 
-  void RemoveDeadEntries();
-
   SnapshotObjectId next_id_;
   HashMap entries_map_;
   List<EntryInfo> entries_;
@@ -286,59 +282,6 @@ class HeapObjectsMap {
 };
 
 
-class HeapSnapshotsCollection {
- public:
-  explicit HeapSnapshotsCollection(Heap* heap);
-  ~HeapSnapshotsCollection();
-
-  Heap* heap() const { return ids_.heap(); }
-
-  SnapshotObjectId PushHeapObjectsStats(OutputStream* stream) {
-    return ids_.PushHeapObjectsStats(stream);
-  }
-  void StartHeapObjectsTracking(bool track_allocations);
-  void StopHeapObjectsTracking();
-
-  HeapSnapshot* NewSnapshot(const char* name, unsigned uid);
-  void SnapshotGenerationFinished(HeapSnapshot* snapshot);
-  List<HeapSnapshot*>* snapshots() { return &snapshots_; }
-  void RemoveSnapshot(HeapSnapshot* snapshot);
-
-  StringsStorage* names() { return &names_; }
-  AllocationTracker* allocation_tracker() { return allocation_tracker_; }
-
-  SnapshotObjectId FindObjectId(Address object_addr) {
-    return ids_.FindEntry(object_addr);
-  }
-  SnapshotObjectId GetObjectId(Address object_addr, int object_size) {
-    return ids_.FindOrAddEntry(object_addr, object_size);
-  }
-  Handle<HeapObject> FindHeapObjectById(SnapshotObjectId id);
-  void ObjectMoveEvent(Address from, Address to, int size) {
-    ids_.MoveObject(from, to, size);
-  }
-  void AllocationEvent(Address addr, int size);
-  void UpdateObjectSizeEvent(Address addr, int size) {
-    ids_.UpdateObjectSize(addr, size);
-  }
-  SnapshotObjectId last_assigned_id() const {
-    return ids_.last_assigned_id();
-  }
-  size_t GetUsedMemorySize() const;
-
-  int FindUntrackedObjects() { return ids_.FindUntrackedObjects(); }
-
- private:
-  List<HeapSnapshot*> snapshots_;
-  StringsStorage names_;
-  // Mapping from HeapObject addresses to objects' uids.
-  HeapObjectsMap ids_;
-  AllocationTracker* allocation_tracker_;
-
-  DISALLOW_COPY_AND_ASSIGN(HeapSnapshotsCollection);
-};
-
-
 // A typedef for referencing anything that can be snapshotted living
 // in any kind of heap memory.
 typedef void* HeapThing;
@@ -531,7 +474,8 @@ class V8HeapExplorer : public HeapEntriesAllocator {
 
   Heap* heap_;
   HeapSnapshot* snapshot_;
-  HeapSnapshotsCollection* collection_;
+  StringsStorage* names_;
+  HeapObjectsMap* heap_object_map_;
   SnapshottingProgressReportingInterface* progress_;
   SnapshotFillerInterface* filler_;
   HeapObjectsSet objects_tags_;
@@ -592,7 +536,7 @@ class NativeObjectsExplorer {
 
   Isolate* isolate_;
   HeapSnapshot* snapshot_;
-  HeapSnapshotsCollection* collection_;
+  StringsStorage* names_;
   SnapshottingProgressReportingInterface* progress_;
   bool embedder_queried_;
   HeapObjectsSet in_groups_;

src/list.h

@@ -196,6 +196,13 @@ class List {
   DISALLOW_COPY_AND_ASSIGN(List);
 };
 
+
+template<typename T, class P>
+size_t GetMemoryUsedByList(const List<T, P>& list) {
+  return list.length() * sizeof(T) + sizeof(list);
+}
+
+
 class Map;
 class Type;
 class Code;

test/cctest/cctest.h

@@ -363,7 +363,7 @@ class HeapObjectsTracker {
 
   ~HeapObjectsTracker() {
     i::Isolate::Current()->heap()->CollectAllAvailableGarbage();
-    CHECK_EQ(0, heap_profiler_->FindUntrackedObjects());
+    CHECK_EQ(0, heap_profiler_->heap_object_map()->FindUntrackedObjects());
     heap_profiler_->StopHeapObjectsTracking();
   }
 

test/cctest/test-heap-profiler.cc

@@ -2055,7 +2055,8 @@ void HeapProfilerExtension::FindUntrackedObjects(
     const v8::FunctionCallbackInfo<v8::Value>& args) {
   i::HeapProfiler* heap_profiler =
       reinterpret_cast<i::HeapProfiler*>(args.GetIsolate()->GetHeapProfiler());
-  int untracked_objects = heap_profiler->FindUntrackedObjects();
+  int untracked_objects =
+      heap_profiler->heap_object_map()->FindUntrackedObjects();
   args.GetReturnValue().Set(untracked_objects);
   CHECK_EQ(0, untracked_objects);
 }
@@ -2161,12 +2162,6 @@ static const char* record_trace_tree_source =
 "for (var i = 0; i < 100; i++) start();\n";
 
 
-static i::HeapSnapshot* ToInternal(const v8::HeapSnapshot* snapshot) {
-  return const_cast<i::HeapSnapshot*>(
-      reinterpret_cast<const i::HeapSnapshot*>(snapshot));
-}
-
-
 static AllocationTraceNode* FindNode(
     AllocationTracker* tracker, const Vector<const char*>& names) {
   AllocationTraceNode* node = tracker->trace_tree()->root();
@@ -2201,10 +2196,8 @@ TEST(ArrayGrowLeftTrim) {
     "    a.shift();\n");
 
   const char* names[] = { "(anonymous function)" };
-  const v8::HeapSnapshot* snapshot = heap_profiler->TakeHeapSnapshot(
-      v8::String::NewFromUtf8(env->GetIsolate(), "Test1"));
-  i::HeapSnapshotsCollection* collection = ToInternal(snapshot)->collection();
-  AllocationTracker* tracker = collection->allocation_tracker();
+  AllocationTracker* tracker =
+      reinterpret_cast<i::HeapProfiler*>(heap_profiler)->allocation_tracker();
   CHECK_NE(NULL, tracker);
   // Resolve all function locations.
   tracker->PrepareForSerialization();
@@ -2229,10 +2222,8 @@ TEST(TrackHeapAllocations) {
 
   CompileRun(record_trace_tree_source);
 
-  const v8::HeapSnapshot* snapshot = heap_profiler->TakeHeapSnapshot(
-      v8::String::NewFromUtf8(env->GetIsolate(), "Test"));
-  i::HeapSnapshotsCollection* collection = ToInternal(snapshot)->collection();
-  AllocationTracker* tracker = collection->allocation_tracker();
+  AllocationTracker* tracker =
+      reinterpret_cast<i::HeapProfiler*>(heap_profiler)->allocation_tracker();
   CHECK_NE(NULL, tracker);
   // Resolve all function locations.
   tracker->PrepareForSerialization();
@@ -2282,10 +2273,8 @@ TEST(TrackBumpPointerAllocations) {
 
     CompileRun(inline_heap_allocation_source);
 
-    const v8::HeapSnapshot* snapshot = heap_profiler->TakeHeapSnapshot(
-        v8::String::NewFromUtf8(env->GetIsolate(), "Test2"));
-    i::HeapSnapshotsCollection* collection = ToInternal(snapshot)->collection();
-    AllocationTracker* tracker = collection->allocation_tracker();
+    AllocationTracker* tracker =
+        reinterpret_cast<i::HeapProfiler*>(heap_profiler)->allocation_tracker();
     CHECK_NE(NULL, tracker);
     // Resolve all function locations.
     tracker->PrepareForSerialization();
@@ -2310,10 +2299,8 @@ TEST(TrackBumpPointerAllocations) {
 
     CompileRun(inline_heap_allocation_source);
 
-    const v8::HeapSnapshot* snapshot = heap_profiler->TakeHeapSnapshot(
-        v8::String::NewFromUtf8(env->GetIsolate(), "Test1"));
-    i::HeapSnapshotsCollection* collection = ToInternal(snapshot)->collection();
-    AllocationTracker* tracker = collection->allocation_tracker();
+    AllocationTracker* tracker =
+        reinterpret_cast<i::HeapProfiler*>(heap_profiler)->allocation_tracker();
     CHECK_NE(NULL, tracker);
     // Resolve all function locations.
     tracker->PrepareForSerialization();