Remove now unused CalculateExactRetainedSize function & co.
authormikhail.naganov@gmail.com <mikhail.naganov@gmail.com@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Mon, 27 Feb 2012 15:42:36 +0000 (15:42 +0000)
committermikhail.naganov@gmail.com <mikhail.naganov@gmail.com@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Mon, 27 Feb 2012 15:42:36 +0000 (15:42 +0000)
This patch changes the signature of the v8::HeapGraphNode::GetRetainedSize method, but it's not used in Chromium, and it should be easy for other clients (if any) to adjust to this change.

BUG=none
TEST=none

Review URL: https://chromiumcodereview.appspot.com/9466014
Patch from Alexei Filippov <alexeif@chromium.org>.

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

include/v8-profiler.h
src/api.cc
src/profile-generator.cc
src/profile-generator.h
test/cctest/test-heap-profiler.cc

index 5a3a40ff6f6d029cefcdda47205235828bcffb0a..fc3ee78837136a97bee4f212298835a9b00ce1e8 100644 (file)
@@ -284,14 +284,8 @@ class V8EXPORT HeapGraphNode {
    * the objects that are reachable only from this object. In other
    * words, the size of memory that will be reclaimed having this node
    * collected.
-   *
-   * Exact retained size calculation has O(N) (number of nodes)
-   * computational complexity, while approximate has O(1). It is
-   * assumed that initially heap profiling tools provide approximate
-   * sizes for all nodes, and then exact sizes are calculated for the
-   * most 'interesting' nodes.
    */
-  int GetRetainedSize(bool exact) const;
+  int GetRetainedSize() const;
 
   /** Returns child nodes count of the node. */
   int GetChildrenCount() const;
index 2b906950aac9821a69149043762d1b4b4fa49ab5..4498286916238f6ea009c1a3a0ce9124eb5e6cc4 100644 (file)
@@ -5873,10 +5873,10 @@ int HeapGraphNode::GetSelfSize() const {
 }
 
 
-int HeapGraphNode::GetRetainedSize(bool exact) const {
+int HeapGraphNode::GetRetainedSize() const {
   i::Isolate* isolate = i::Isolate::Current();
   IsDeadCheck(isolate, "v8::HeapSnapshot::GetRetainedSize");
-  return ToInternal(this)->RetainedSize(exact);
+  return ToInternal(this)->retained_size();
 }
 
 
index 156fbc7b63cb29782bbf85f1d8120554f7e53609..dd2f7014dbd4ae1a36ae8f74bdeb53c27a7b58fd 100644 (file)
@@ -971,7 +971,7 @@ void HeapEntry::Init(HeapSnapshot* snapshot,
                      int retainers_count) {
   snapshot_ = snapshot;
   type_ = type;
-  painted_ = kUnpainted;
+  painted_ = false;
   name_ = name;
   self_size_ = self_size;
   retained_size_ = 0;
@@ -1013,56 +1013,15 @@ void HeapEntry::SetUnidirElementReference(
 }
 
 
-int HeapEntry::RetainedSize(bool exact) {
-  if (exact && (retained_size_ & kExactRetainedSizeTag) == 0) {
-    CalculateExactRetainedSize();
-  }
-  return retained_size_ & (~kExactRetainedSizeTag);
-}
-
-
 Handle<HeapObject> HeapEntry::GetHeapObject() {
   return snapshot_->collection()->FindHeapObjectById(id());
 }
 
 
-template<class Visitor>
-void HeapEntry::ApplyAndPaintAllReachable(Visitor* visitor) {
-  List<HeapEntry*> list(10);
-  list.Add(this);
-  this->paint_reachable();
-  visitor->Apply(this);
-  while (!list.is_empty()) {
-    HeapEntry* entry = list.RemoveLast();
-    Vector<HeapGraphEdge> children = entry->children();
-    for (int i = 0; i < children.length(); ++i) {
-      if (children[i].type() == HeapGraphEdge::kShortcut) continue;
-      HeapEntry* child = children[i].to();
-      if (!child->painted_reachable()) {
-        list.Add(child);
-        child->paint_reachable();
-        visitor->Apply(child);
-      }
-    }
-  }
-}
-
-
-class NullClass {
- public:
-  void Apply(HeapEntry* entry) { }
-};
-
-void HeapEntry::PaintAllReachable() {
-  NullClass null;
-  ApplyAndPaintAllReachable(&null);
-}
-
-
 void HeapEntry::Print(
     const char* prefix, const char* edge_name, int max_depth, int indent) {
   OS::Print("%6d %7d @%6llu %*c %s%s: ",
-            self_size(), RetainedSize(false), id(),
+            self_size(), retained_size(), id(),
             indent, ' ', prefix, edge_name);
   if (type() != kString) {
     OS::Print("%s %.40s\n", TypeAsString(), name_);
@@ -1146,60 +1105,6 @@ int HeapEntry::EntriesSize(int entries_count,
 }
 
 
-class RetainedSizeCalculator {
- public:
-  RetainedSizeCalculator()
-      : retained_size_(0) {
-  }
-
-  int retained_size() const { return retained_size_; }
-
-  void Apply(HeapEntry** entry_ptr) {
-    if ((*entry_ptr)->painted_reachable()) {
-      retained_size_ += (*entry_ptr)->self_size();
-    }
-  }
-
- private:
-  int retained_size_;
-};
-
-
-void HeapEntry::CalculateExactRetainedSize() {
-  // To calculate retained size, first we paint all reachable nodes in
-  // one color, then we paint (or re-paint) all nodes reachable from
-  // other nodes with a different color. Then we sum up self sizes of
-  // nodes painted with the first color.
-  snapshot()->ClearPaint();
-  PaintAllReachable();
-
-  List<HeapEntry*> list(10);
-  HeapEntry* root = snapshot()->root();
-  if (this != root) {
-    list.Add(root);
-    root->paint_reachable_from_others();
-  }
-  while (!list.is_empty()) {
-    HeapEntry* curr = list.RemoveLast();
-    Vector<HeapGraphEdge> children = curr->children();
-    for (int i = 0; i < children.length(); ++i) {
-      if (children[i].type() == HeapGraphEdge::kShortcut) continue;
-      HeapEntry* child = children[i].to();
-      if (child != this && child->not_painted_reachable_from_others()) {
-        list.Add(child);
-        child->paint_reachable_from_others();
-      }
-    }
-  }
-
-  RetainedSizeCalculator ret_size_calc;
-  snapshot()->IterateEntries(&ret_size_calc);
-  retained_size_ = ret_size_calc.retained_size();
-  ASSERT((retained_size_ & kExactRetainedSizeTag) == 0);
-  retained_size_ |= kExactRetainedSizeTag;
-}
-
-
 // It is very important to keep objects that form a heap snapshot
 // as small as possible.
 namespace {  // Avoid littering the global namespace.
@@ -3189,7 +3094,7 @@ bool HeapSnapshotGenerator::GenerateSnapshot() {
   if (!FillReferences()) return false;
 
   if (!SetEntriesDominators()) return false;
-  if (!ApproximateRetainedSizes()) return false;
+  if (!CalculateRetainedSizes()) return false;
 
   progress_counter_ = progress_total_;
   if (!ProgressReport(true)) return false;
@@ -3247,7 +3152,7 @@ void HeapSnapshotGenerator::FillReversePostorderIndexes(
   int current_entry = 0;
   List<HeapEntry*> nodes_to_visit;
   nodes_to_visit.Add(snapshot_->root());
-  snapshot_->root()->paint_reachable();
+  snapshot_->root()->paint();
   while (!nodes_to_visit.is_empty()) {
     HeapEntry* entry = nodes_to_visit.last();
     Vector<HeapGraphEdge> children = entry->children();
@@ -3255,9 +3160,9 @@ void HeapSnapshotGenerator::FillReversePostorderIndexes(
     for (int i = 0; i < children.length(); ++i) {
       if (children[i].type() == HeapGraphEdge::kShortcut) continue;
       HeapEntry* child = children[i].to();
-      if (!child->painted_reachable()) {
+      if (!child->painted()) {
         nodes_to_visit.Add(child);
-        child->paint_reachable();
+        child->paint();
         has_new_edges = true;
       }
     }
@@ -3293,15 +3198,13 @@ bool HeapSnapshotGenerator::BuildDominatorTree(
   for (int i = 0; i < root_index; ++i) (*dominators)[i] = kNoDominator;
   (*dominators)[root_index] = root_index;
 
-  // The affected array is used to mark those entries that may
-  // be affected because of dominators change among their retainers.
-  ScopedVector<bool> affected(entries_length);
-  for (int i = 0; i < entries_length; ++i) affected[i] = false;
+  // The painted flag is used to mark entries that need to be recalculated
+  // because of dominators change among their retainers.
+  for (int i = 0; i < entries_length; ++i) entries[i]->clear_paint();
+
+  // Mark the root direct children as affected.
   Vector<HeapGraphEdge> children = entries[root_index]->children();
-  for (int i = 0; i < children.length(); ++i) {
-    // Mark the root direct children as affected.
-    affected[children[i].to()->ordered_index()] = true;
-  }
+  for (int i = 0; i < children.length(); ++i) children[i].to()->paint();
 
   bool changed = true;
   while (changed) {
@@ -3310,10 +3213,11 @@ bool HeapSnapshotGenerator::BuildDominatorTree(
       // If dominator of the entry has already been set to root,
       // then it can't propagate any further.
       if ((*dominators)[i] == root_index) continue;
-      if (!affected[i]) continue;
-      affected[i] = false;
+      HeapEntry* entry = entries[i];
+      if (!entry->painted()) continue;
+      entry->clear_paint();
       int new_idom_index = kNoDominator;
-      Vector<HeapGraphEdge*> rets = entries[i]->retainers();
+      Vector<HeapGraphEdge*> rets = entry->retainers();
       for (int j = 0; j < rets.length(); ++j) {
         if (rets[j]->type() == HeapGraphEdge::kShortcut) continue;
         int ret_index = rets[j]->From()->ordered_index();
@@ -3331,9 +3235,7 @@ bool HeapSnapshotGenerator::BuildDominatorTree(
         (*dominators)[i] = new_idom_index;
         changed = true;
         Vector<HeapGraphEdge> children = entries[i]->children();
-        for (int j = 0; j < children.length(); ++j) {
-          affected[children[j].to()->ordered_index()] = true;
-        }
+        for (int j = 0; j < children.length(); ++j) children[j].to()->paint();
       }
     }
   }
@@ -3355,7 +3257,7 @@ bool HeapSnapshotGenerator::SetEntriesDominators() {
 }
 
 
-bool HeapSnapshotGenerator::ApproximateRetainedSizes() {
+bool HeapSnapshotGenerator::CalculateRetainedSizes() {
   // As for the dominators tree we only know parent nodes, not
   // children, to sum up total sizes we "bubble" node's self size
   // adding it to all of its parents.
@@ -3607,7 +3509,7 @@ void HeapSnapshotJSONSerializer::SerializeNode(HeapEntry* entry) {
       GetStringId(entry->name()),
       entry->id(),
       entry->self_size(),
-      entry->RetainedSize(false),
+      entry->retained_size(),
       GetNodeId(entry->dominator()),
       children.length());
   USE(result);
index 13c6b2db0b667c6b82821150d0ae8235ae07acf5..22a0473eeaa1bf66ce88536d00ee796474c7088a 100644 (file)
@@ -558,22 +558,9 @@ class HeapEntry BASE_EMBEDDED {
     ASSERT(entry != NULL);
     dominator_ = entry;
   }
-
-  void clear_paint() { painted_ = kUnpainted; }
-  bool painted_reachable() { return painted_ == kPainted; }
-  void paint_reachable() {
-    ASSERT(painted_ == kUnpainted);
-    painted_ = kPainted;
-  }
-  bool not_painted_reachable_from_others() {
-    return painted_ != kPaintedReachableFromOthers;
-  }
-  void paint_reachable_from_others() {
-    painted_ = kPaintedReachableFromOthers;
-  }
-  template<class Visitor>
-  void ApplyAndPaintAllReachable(Visitor* visitor);
-  void PaintAllReachable();
+  void clear_paint() { painted_ = false; }
+  bool painted() { return painted_; }
+  void paint() { painted_ = true; }
 
   void SetIndexedReference(HeapGraphEdge::Type type,
                            int child_index,
@@ -588,7 +575,6 @@ class HeapEntry BASE_EMBEDDED {
   void SetUnidirElementReference(int child_index, int index, HeapEntry* entry);
 
   int EntrySize() { return EntriesSize(1, children_count_, retainers_count_); }
-  int RetainedSize(bool exact);
 
   void Print(
       const char* prefix, const char* edge_name, int max_depth, int indent);
@@ -606,12 +592,11 @@ class HeapEntry BASE_EMBEDDED {
   HeapGraphEdge** retainers_arr() {
     return reinterpret_cast<HeapGraphEdge**>(children_arr() + children_count_);
   }
-  void CalculateExactRetainedSize();
   const char* TypeAsString();
 
-  unsigned painted_: 2;
+  unsigned painted_: 1;
   unsigned type_: 4;
-  int children_count_: 26;
+  int children_count_: 27;
   int retainers_count_;
   int self_size_;
   union {
@@ -626,13 +611,6 @@ class HeapEntry BASE_EMBEDDED {
   } id_;  // This is to avoid extra padding of 64-bit value.
   const char* name_;
 
-  // Paints used for exact retained sizes calculation.
-  static const unsigned kUnpainted = 0;
-  static const unsigned kPainted = 1;
-  static const unsigned kPaintedReachableFromOthers = 2;
-
-  static const int kExactRetainedSizeTag = 1;
-
   DISALLOW_COPY_AND_ASSIGN(HeapEntry);
 };
 
@@ -1099,9 +1077,9 @@ class HeapSnapshotGenerator : public SnapshottingProgressReportingInterface {
   bool GenerateSnapshot();
 
  private:
-  bool ApproximateRetainedSizes();
   bool BuildDominatorTree(const Vector<HeapEntry*>& entries,
                           Vector<int>* dominators);
+  bool CalculateRetainedSizes();
   bool CountEntriesAndReferences();
   bool FillReferences();
   void FillReversePostorderIndexes(Vector<HeapEntry*>* entries);
index a6f04b308d03d42c0dc511fe19c0519fcf383be1..f7078509408dd7fb6b21205ab6b322109b04485e 100644 (file)
@@ -18,14 +18,30 @@ class NamedEntriesDetector {
       : has_A2(false), has_B2(false), has_C2(false) {
   }
 
-  void Apply(i::HeapEntry** entry_ptr) {
-    if (IsReachableNodeWithName(*entry_ptr, "A2")) has_A2 = true;
-    if (IsReachableNodeWithName(*entry_ptr, "B2")) has_B2 = true;
-    if (IsReachableNodeWithName(*entry_ptr, "C2")) has_C2 = true;
+  void CheckEntry(i::HeapEntry* entry) {
+    if (strcmp(entry->name(), "A2") == 0) has_A2 = true;
+    if (strcmp(entry->name(), "B2") == 0) has_B2 = true;
+    if (strcmp(entry->name(), "C2") == 0) has_C2 = true;
   }
 
-  static bool IsReachableNodeWithName(i::HeapEntry* entry, const char* name) {
-    return strcmp(name, entry->name()) == 0 && entry->painted_reachable();
+  void CheckAllReachables(i::HeapEntry* root) {
+    i::List<i::HeapEntry*> list(10);
+    list.Add(root);
+    root->paint();
+    CheckEntry(root);
+    while (!list.is_empty()) {
+      i::HeapEntry* entry = list.RemoveLast();
+      i::Vector<i::HeapGraphEdge> children = entry->children();
+      for (int i = 0; i < children.length(); ++i) {
+        if (children[i].type() == i::HeapGraphEdge::kShortcut) continue;
+        i::HeapEntry* child = children[i].to();
+        if (!child->painted()) {
+          list.Add(child);
+          child->paint();
+          CheckEntry(child);
+        }
+      }
+    }
   }
 
   bool has_A2;
@@ -90,10 +106,6 @@ TEST(HeapSnapshot) {
       const_cast<i::HeapSnapshot*>(
           reinterpret_cast<const i::HeapSnapshot*>(snapshot_env2));
   const v8::HeapGraphNode* global_env2 = GetGlobalObject(snapshot_env2);
-  // Paint all nodes reachable from global object.
-  i_snapshot_env2->ClearPaint();
-  const_cast<i::HeapEntry*>(
-      reinterpret_cast<const i::HeapEntry*>(global_env2))->PaintAllReachable();
 
   // Verify, that JS global object of env2 has '..2' properties.
   const v8::HeapGraphNode* a2_node =
@@ -105,8 +117,11 @@ TEST(HeapSnapshot) {
       NULL, GetProperty(global_env2, v8::HeapGraphEdge::kShortcut, "b2_2"));
   CHECK_NE(NULL, GetProperty(global_env2, v8::HeapGraphEdge::kShortcut, "c2"));
 
+  // Paint all nodes reachable from global object.
   NamedEntriesDetector det;
-  i_snapshot_env2->IterateEntries(&det);
+  i_snapshot_env2->ClearPaint();
+  det.CheckAllReachables(const_cast<i::HeapEntry*>(
+      reinterpret_cast<const i::HeapEntry*>(global_env2)));
   CHECK(det.has_A2);
   CHECK(det.has_B2);
   CHECK(det.has_C2);
@@ -136,14 +151,10 @@ TEST(HeapSnapshotObjectSizes) {
       GetProperty(x, v8::HeapGraphEdge::kProperty, "b");
   CHECK_NE(NULL, x2);
 
-  // Test approximate sizes.
-  CHECK_EQ(x->GetSelfSize() * 3, x->GetRetainedSize(false));
-  CHECK_EQ(x1->GetSelfSize(), x1->GetRetainedSize(false));
-  CHECK_EQ(x2->GetSelfSize(), x2->GetRetainedSize(false));
-  // Test exact sizes.
-  CHECK_EQ(x->GetSelfSize() * 3, x->GetRetainedSize(true));
-  CHECK_EQ(x1->GetSelfSize(), x1->GetRetainedSize(true));
-  CHECK_EQ(x2->GetSelfSize(), x2->GetRetainedSize(true));
+  // Test sizes.
+  CHECK_EQ(x->GetSelfSize() * 3, x->GetRetainedSize());
+  CHECK_EQ(x1->GetSelfSize(), x1->GetRetainedSize());
+  CHECK_EQ(x2->GetSelfSize(), x2->GetRetainedSize());
 }