Refactor heap profiler's code to make possible including

src/profile-generator.h

 // Copyright 2010 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 663 matching lines @@
   HeapSnapshotsCollection* collection() { return collection_; }
   Type type() { return type_; }
   const char* title() { return title_; }
   unsigned uid() { return uid_; }
   HeapEntry* root() { return root_entry_; }
   HeapEntry* gc_roots() { return gc_roots_entry_; }
   List<HeapEntry*>* entries() { return &entries_; }
 
   void AllocateEntries(
       int entries_count, int children_count, int retainers_count);
-  HeapEntry* AddEntry(
-      HeapObject* object, int children_count, int retainers_count);
   HeapEntry* AddEntry(HeapEntry::Type type,
                       const char* name,
                       uint64_t id,
                       int size,
                       int children_count,
                       int retainers_count);
+  HeapEntry* AddRootEntry(int children_count);
+  HeapEntry* AddGcRootsEntry(int children_count, int retainers_count);
   void ClearPaint();
   HeapSnapshotsDiff* CompareWith(HeapSnapshot* snapshot);
   HeapEntry* GetEntryById(uint64_t id);
   List<HeapGraphPath*>* GetRetainingPaths(HeapEntry* entry);
   List<HeapEntry*>* GetSortedEntriesList();
   template<class Visitor>
   void IterateEntries(Visitor* visitor) { entries_.Iterate(visitor); }
   void SetDominatorsToSelf();
 
   void Print(int max_depth);
   void PrintEntriesSize();
 
-  static HeapObject* const kInternalRootObject;
-  static HeapObject* const kGcRootsObject;
-
  private:
-  HeapEntry* AddEntry(HeapObject* object,
-                      HeapEntry::Type type,
-                      const char* name,
-                      int children_count,
-                      int retainers_count);
   HeapEntry* GetNextEntryToInit();
 
   HeapSnapshotsCollection* collection_;
   Type type_;
   const char* title_;
   unsigned uid_;
   HeapEntry* root_entry_;
   HeapEntry* gc_roots_entry_;
   char* raw_entries_;
   List<HeapEntry*> entries_;
@@ skipping 143 matching lines @@
   StringsStorage names_;
   TokenEnumerator* token_enumerator_;
   // Mapping from HeapObject addresses to objects' uids.
   HeapObjectsMap ids_;
   HeapSnapshotsComparator comparator_;
 
   DISALLOW_COPY_AND_ASSIGN(HeapSnapshotsCollection);
 };
 
 
+// A typedef for referencing anything that can be snapshotted living
+// in any kind of heap memory.
+typedef void* HeapThing;
+
+
+// An interface that creates HeapEntries by HeapThings.
+class HeapEntriesAllocator {
+ public:
+  virtual ~HeapEntriesAllocator() { }
+  virtual HeapEntry* AllocateEntry(
+      HeapThing ptr, int children_count, int retainers_count) = 0;
+};
+
+
 // The HeapEntriesMap instance is used to track a mapping between
 // real heap objects and their representations in heap snapshots.
 class HeapEntriesMap {

[Vitaly Repeshko, 2011/03/01 16:55:51]

Consider refactoring it into a simple HeapThing->EntryInfo map and a wrapper
that does things like Pair on top of it.

[mnaganov (inactive), 2011/03/01 17:35:41]

Thanks, but let me postpone it.

  public:
   HeapEntriesMap();
   ~HeapEntriesMap();
 
-  HeapEntry* Map(HeapObject* object);
-  void Pair(HeapObject* object, HeapEntry* entry);
-  void CountReference(HeapObject* from, HeapObject* to,
+  HeapEntry* Map(HeapThing thing);
+  void Pair(HeapThing thing, HeapEntriesAllocator* allocator, HeapEntry* entry);
+  void CountReference(HeapThing from, HeapThing to,
                       int* prev_children_count = NULL,
                       int* prev_retainers_count = NULL);
-  template<class Visitor>
-  void UpdateEntries(Visitor* visitor);
+  void UpdateEntries();

[Vitaly Repeshko, 2011/03/01 16:55:51]

The name is confusing. It's supposed to be called only once (if I got it right)
to actually create the entries.

[mnaganov (inactive), 2011/03/01 17:35:41]

Renamed to AllocateEntries -- this should meet your expectation.

 
   int entries_count() { return entries_count_; }
   int total_children_count() { return total_children_count_; }
   int total_retainers_count() { return total_retainers_count_; }
 
   static HeapEntry *const kHeapEntryPlaceholder;
 
  private:
   struct EntryInfo {
-    explicit EntryInfo(HeapEntry* entry)
-        : entry(entry), children_count(0), retainers_count(0) { }
+    EntryInfo(HeapEntry* entry, HeapEntriesAllocator* allocator)
+        : entry(entry),
+          allocator(allocator),
+          children_count(0),
+          retainers_count(0) {
+    }
     HeapEntry* entry;
+    HeapEntriesAllocator* allocator;
     int children_count;
     int retainers_count;
   };
 
-  static uint32_t Hash(HeapObject* object) {
+  static uint32_t Hash(HeapThing thing) {
     return ComputeIntegerHash(
-        static_cast<uint32_t>(reinterpret_cast<uintptr_t>(object)));
+        static_cast<uint32_t>(reinterpret_cast<uintptr_t>(thing)));
   }
-  static bool HeapObjectsMatch(void* key1, void* key2) { return key1 == key2; }
+  static bool HeapThingsMatch(HeapThing key1, HeapThing key2) {
+    return key1 == key2;
+  }
 
   HashMap entries_;
   int entries_count_;
   int total_children_count_;
   int total_retainers_count_;
 
   friend class HeapObjectsSet;
 
   DISALLOW_COPY_AND_ASSIGN(HeapEntriesMap);
 };
 
 
 class HeapObjectsSet {
  public:
   HeapObjectsSet();
   void Clear();
   bool Contains(Object* object);
   void Insert(Object* obj);
 
  private:
   HashMap entries_;
 
   DISALLOW_COPY_AND_ASSIGN(HeapObjectsSet);
 };
 
 
-class HeapSnapshotGenerator {
+// An interface used to populate a snapshot with nodes and edges.
+class SnapshotFillerInterface {
  public:
-  class SnapshotFillerInterface {
-   public:
-    virtual ~SnapshotFillerInterface() { }
-    virtual HeapEntry* AddEntry(HeapObject* obj) = 0;
-    virtual void SetIndexedReference(HeapGraphEdge::Type type,
-                                     HeapObject* parent_obj,
-                                     HeapEntry* parent_entry,
-                                     int index,
-                                     Object* child_obj,
-                                     HeapEntry* child_entry) = 0;
-    virtual void SetNamedReference(HeapGraphEdge::Type type,
-                                   HeapObject* parent_obj,
+  virtual ~SnapshotFillerInterface() { }
+  virtual HeapEntry* AddEntry(HeapThing ptr) = 0;
+  virtual HeapEntry* FindOrAddEntry(HeapThing ptr) = 0;
+  virtual void SetIndexedReference(HeapGraphEdge::Type type,
+                                   HeapThing parent_ptr,
                                    HeapEntry* parent_entry,
-                                   const char* reference_name,
-                                   Object* child_obj,
+                                   int index,
+                                   HeapThing child_ptr,
                                    HeapEntry* child_entry) = 0;
-    virtual void SetRootGcRootsReference() = 0;
-    virtual void SetRootShortcutReference(Object* child_obj,
+  virtual void SetIndexedAutoIndexReference(HeapGraphEdge::Type type,
+                                            HeapThing parent_ptr,
+                                            HeapEntry* parent_entry,
+                                            HeapThing child_ptr,
+                                            HeapEntry* child_entry) = 0;
+  virtual void SetNamedReference(HeapGraphEdge::Type type,
+                                 HeapThing parent_ptr,
+                                 HeapEntry* parent_entry,
+                                 const char* reference_name,
+                                 HeapThing child_ptr,
+                                 HeapEntry* child_entry) = 0;
+  virtual void SetNamedAutoIndexReference(HeapGraphEdge::Type type,
+                                          HeapThing parent_ptr,
+                                          HeapEntry* parent_entry,
+                                          HeapThing child_ptr,
                                           HeapEntry* child_entry) = 0;
-    virtual void SetStrongRootReference(Object* child_obj,
-                                        HeapEntry* child_entry) = 0;
-  };
+};
 
-  HeapSnapshotGenerator(HeapSnapshot* snapshot,
-                        v8::ActivityControl* control);
-  bool GenerateSnapshot();
 
- private:
-  bool ApproximateRetainedSizes();
-  bool BuildDominatorTree(const Vector<HeapEntry*>& entries,
-                          Vector<HeapEntry*>* dominators);
-  bool CountEntriesAndReferences();
-  HeapEntry* GetEntry(Object* obj);
-  void IncProgressCounter() { ++progress_counter_; }
+class SnapshottingProgressReportingInterface {
+ public:
+  virtual ~SnapshottingProgressReportingInterface() { }
+  virtual void ProgressStep() = 0;
+  virtual bool ProgressReport(bool force) = 0;
+};
+
+
+// An implementation of V8 heap graph extractor.
+class V8HeapExplorer : public HeapEntriesAllocator {
+ public:
+  V8HeapExplorer(HeapSnapshot* snapshot,
+                 SnapshottingProgressReportingInterface* progress);
+  ~V8HeapExplorer();
+  HeapEntry* AllocateEntry(

[Vitaly Repeshko, 2011/03/01 16:55:51]

It seems weird to have both AllocateEntry and AddEntry in the public interface.
The latter should be hidden. Also please mark the function as virtual to make it
clearer it's an interface implementation.

[mnaganov (inactive), 2011/03/01 17:35:41]

Marked AllocateEntry as virtual.
Also, I made private as many functions of V8HeapExplorer as possible.

+      HeapThing ptr, int children_count, int retainers_count);
+
+  HeapEntry* AddEntry(
+      HeapObject* object, int children_count, int retainers_count);
+  HeapEntry* AddEntry(HeapObject* object,
+                      HeapEntry::Type type,
+                      const char* name,
+                      int children_count,
+                      int retainers_count);
+  void AddRootEntries(SnapshotFillerInterface* filler);
   void ExtractReferences(HeapObject* obj);
   void ExtractClosureReferences(JSObject* js_obj, HeapEntry* entry);
   void ExtractPropertyReferences(JSObject* js_obj, HeapEntry* entry);
   void ExtractElementReferences(JSObject* js_obj, HeapEntry* entry);
   void ExtractInternalReferences(JSObject* js_obj, HeapEntry* entry);
-  bool FillReferences();
-  void FillReversePostorderIndexes(Vector<HeapEntry*>* entries);
-  bool IterateAndExtractReferences();
-  inline bool ReportProgress(bool force = false);
-  bool SetEntriesDominators();
+  int EstimateObjectsCount();
+  bool IterateAndExtractReferences(SnapshotFillerInterface* filler);
   void SetClosureReference(HeapObject* parent_obj,
                            HeapEntry* parent,
                            String* reference_name,
                            Object* child);
   void SetElementReference(HeapObject* parent_obj,
                            HeapEntry* parent,
                            int index,
                            Object* child);
   void SetInternalReference(HeapObject* parent_obj,
                             HeapEntry* parent,
@@ skipping 11 matching lines @@
                             HeapEntry* parent,
                             String* reference_name,
                             Object* child);
   void SetPropertyShortcutReference(HeapObject* parent_obj,
                                     HeapEntry* parent,
                                     String* reference_name,
                                     Object* child);
   void SetRootShortcutReference(Object* child);
   void SetRootGcRootsReference();
   void SetGcRootsReference(Object* child);
+
+ private:
+  HeapEntry* GetEntry(Object* obj);
+
+  HeapSnapshot* snapshot_;
+  HeapSnapshotsCollection* collection_;
+  SnapshottingProgressReportingInterface* progress_;
+  // Used during references extraction to mark heap objects that
+  // are references via non-hidden properties.
+  HeapObjectsSet known_references_;
+  SnapshotFillerInterface* filler_;
+
+  static HeapObject* const kInternalRootObject;
+  static HeapObject* const kGcRootsObject;
+
+  friend class IndexedReferencesExtractor;
+  friend class RootsReferencesExtractor;
+
+  DISALLOW_COPY_AND_ASSIGN(V8HeapExplorer);
+};
+
+
+class HeapSnapshotGenerator : public SnapshottingProgressReportingInterface {
+ public:
+  HeapSnapshotGenerator(HeapSnapshot* snapshot,
+                        v8::ActivityControl* control);
+  bool GenerateSnapshot();
+
+ private:
+  bool ApproximateRetainedSizes();
+  bool BuildDominatorTree(const Vector<HeapEntry*>& entries,
+                          Vector<HeapEntry*>* dominators);
+  bool CountEntriesAndReferences();
+  bool FillReferences();
+  void FillReversePostorderIndexes(Vector<HeapEntry*>* entries);
+  void ProgressStep();
+  bool ProgressReport(bool force = false);
+  bool SetEntriesDominators();
   void SetProgressTotal(int iterations_count);
 
   HeapSnapshot* snapshot_;
   v8::ActivityControl* control_;
-  HeapSnapshotsCollection* collection_;
-  // Mapping from HeapObject* pointers to HeapEntry* pointers.
+  V8HeapExplorer v8_heap_explorer_;
+  // Mapping from HeapThing pointers to HeapEntry* pointers.
   HeapEntriesMap entries_;
-  SnapshotFillerInterface* filler_;
-  // Used during references extraction to mark heap objects that
-  // are references via non-hidden properties.
-  HeapObjectsSet known_references_;
   // Used during snapshot generation.
   int progress_counter_;
   int progress_total_;
 
-  friend class IndexedReferencesExtractor;
-  friend class RootsReferencesExtractor;
-
   DISALLOW_COPY_AND_ASSIGN(HeapSnapshotGenerator);
 };
 
 class OutputStreamWriter;
 
 class HeapSnapshotJSONSerializer {
  public:
   explicit HeapSnapshotJSONSerializer(HeapSnapshot* snapshot)
       : snapshot_(snapshot),
         nodes_(ObjectsMatch),
@@ skipping 40 matching lines @@
 };
 
 
 String* GetConstructorNameForHeapProfile(JSObject* object);
 
 } }  // namespace v8::internal
 
 #endif  // ENABLE_LOGGING_AND_PROFILING
 
 #endif  // V8_PROFILE_GENERATOR_H_