Add size metric into Heap Stats.

The stats data have only count field at the moment.
A constantly growing array of integers also can be a reason of a leak.
Ans we have to have a way to detect such kind of leaks.

Drive by fix:
FindObject and AddEntry were replaced with FindEntry/FindOrAddEntry pair.

BUG=none
TEST=HeapSnapshotObjectsStats

Review URL: https://chromiumcodereview.appspot.com/10086004

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

include/v8-profiler.h

@@ -433,8 +433,9 @@ class V8EXPORT HeapProfiler {
    * time interval entry contains information on the current heap objects
    * population size. The method also updates aggregated statistics and
    * reports updates for all previous time intervals via the OutputStream
-   * object. Updates on each time interval are provided as pairs of time
-   * interval index and updated heap objects count.
+   * object. Updates on each time interval are provided as a triplet. It has
+   * time interval index, updated heap objects count and updated heap objects
+   * size.
    *
    * StartHeapObjectsTracking must be called before the first call to this
    * method.

src/profile-generator.cc

@@ -1314,8 +1314,7 @@ const SnapshotObjectId HeapObjectsMap::kFirstAvailableObjectId =
     VisitorSynchronization::kNumberOfSyncTags * HeapObjectsMap::kObjectIdStep;
 
 HeapObjectsMap::HeapObjectsMap()
-    : initial_fill_mode_(true),
-      next_id_(kFirstAvailableObjectId),
+    : next_id_(kFirstAvailableObjectId),
       entries_map_(AddressesMatch),
       entries_(new List<EntryInfo>()) {
   // This dummy element solves a problem with entries_map_.
@@ -1325,7 +1324,7 @@ HeapObjectsMap::HeapObjectsMap()
   // With such dummy element we have a guaranty that all entries_map_ entries
   // will have the value field grater than 0.
   // This fact is using in MoveObject method.
-  entries_->Add(EntryInfo(0, NULL));
+  entries_->Add(EntryInfo(0, NULL, 0));
 }
 
 
@@ -1335,27 +1334,10 @@ HeapObjectsMap::~HeapObjectsMap() {
 
 
 void HeapObjectsMap::SnapshotGenerationFinished() {
-  initial_fill_mode_ = false;
   RemoveDeadEntries();
 }
 
 
-SnapshotObjectId HeapObjectsMap::FindObject(Address addr) {
-  if (!initial_fill_mode_) {
-    SnapshotObjectId existing = FindEntry(addr);
-    if (existing != 0) return existing;
-  }
-  SnapshotObjectId id = next_id_;
-  next_id_ += kObjectIdStep;
-  AddEntry(addr, id);
-  // Here and in other places the length of entries_ list has to be
-  // the same or greater than the length of entries_map_. But entries_ list
-  // has a dummy element at index 0.
-  ASSERT(static_cast<uint32_t>(entries_->length()) > entries_map_.occupancy());
-  return id;
-}
-
-
 void HeapObjectsMap::MoveObject(Address from, Address to) {
   ASSERT(to != NULL);
   ASSERT(from != NULL);
@@ -1379,36 +1361,19 @@ void HeapObjectsMap::MoveObject(Address from, Address to) {
 }
 
 
-void HeapObjectsMap::AddEntry(Address addr, SnapshotObjectId id) {
-  HashMap::Entry* entry = entries_map_.Lookup(addr, AddressHash(addr), true);
-  ASSERT(entry->value == NULL);
-  ASSERT(entries_->length() > 0 &&
-         entries_->at(0).id == 0 &&
-         entries_->at(0).addr == NULL);
-  ASSERT(entries_->at(entries_->length() - 1).id < id);
-  entry->value = reinterpret_cast<void*>(entries_->length());
-  entries_->Add(EntryInfo(id, addr));
-  ASSERT(static_cast<uint32_t>(entries_->length()) > entries_map_.occupancy());
-}
-
-
 SnapshotObjectId HeapObjectsMap::FindEntry(Address addr) {
   HashMap::Entry* entry = entries_map_.Lookup(addr, AddressHash(addr), false);
-  if (entry != NULL) {
-    int entry_index =
-        static_cast<int>(reinterpret_cast<intptr_t>(entry->value));
-    EntryInfo& entry_info = entries_->at(entry_index);
-    entry_info.accessed = true;
-    ASSERT(static_cast<uint32_t>(entries_->length()) >
-           entries_map_.occupancy());
-    return entry_info.id;
-  } else {
-    return 0;
-  }
+  if (entry == NULL) return 0;
+  int entry_index = static_cast<int>(reinterpret_cast<intptr_t>(entry->value));
+  EntryInfo& entry_info = entries_->at(entry_index);
+  entry_info.accessed = true;
+  ASSERT(static_cast<uint32_t>(entries_->length()) > entries_map_.occupancy());
+  return entry_info.id;
 }
 
 
-SnapshotObjectId HeapObjectsMap::FindOrAddEntry(Address addr) {
+SnapshotObjectId HeapObjectsMap::FindOrAddEntry(Address addr,
+                                                unsigned int size) {
   ASSERT(static_cast<uint32_t>(entries_->length()) > entries_map_.occupancy());
   HashMap::Entry* entry = entries_map_.Lookup(addr, AddressHash(addr), true);
   if (entry->value != NULL) {
@@ -1416,12 +1381,13 @@ SnapshotObjectId HeapObjectsMap::FindOrAddEntry(Address addr) {
         static_cast<int>(reinterpret_cast<intptr_t>(entry->value));
     EntryInfo& entry_info = entries_->at(entry_index);
     entry_info.accessed = true;
+    entry_info.size = size;
     return entry_info.id;
   }
   entry->value = reinterpret_cast<void*>(entries_->length());
   SnapshotObjectId id = next_id_;
   next_id_ += kObjectIdStep;
-  entries_->Add(EntryInfo(id, addr));
+  entries_->Add(EntryInfo(id, addr, size));
   ASSERT(static_cast<uint32_t>(entries_->length()) > entries_map_.occupancy());
   return id;
 }
@@ -1434,13 +1400,12 @@ void HeapObjectsMap::StopHeapObjectsTracking() {
 void HeapObjectsMap::UpdateHeapObjectsMap() {
   HEAP->CollectAllGarbage(Heap::kMakeHeapIterableMask,
                           "HeapSnapshotsCollection::UpdateHeapObjectsMap");
-  HeapIterator iterator(HeapIterator::kFilterUnreachable);
+  HeapIterator iterator;
   for (HeapObject* obj = iterator.next();
        obj != NULL;
        obj = iterator.next()) {
-    FindOrAddEntry(obj->address());
+    FindOrAddEntry(obj->address(), obj->Size());
   }
-  initial_fill_mode_ = false;
   RemoveDeadEntries();
 }
 
@@ -1458,14 +1423,19 @@ void HeapObjectsMap::PushHeapObjectsStats(OutputStream* stream) {
        ++time_interval_index) {
     TimeInterval& time_interval = time_intervals_[time_interval_index];
     SnapshotObjectId time_interval_id = time_interval.id;
-    uint32_t entries_count = 0;
+    uint32_t entries_size = 0;
+    EntryInfo* start_entry_info = entry_info;
     while (entry_info < end_entry_info && entry_info->id < time_interval_id) {
-      ++entries_count;
+      entries_size += entry_info->size;
       ++entry_info;
     }
-    if (time_interval.count != entries_count) {
+    uint32_t entries_count =
+        static_cast<uint32_t>(entry_info - start_entry_info);
+    if (time_interval.count != entries_count ||
+        time_interval.size != entries_size) {
       stats_buffer.Add(time_interval_index);
       stats_buffer.Add(time_interval.count = entries_count);
+      stats_buffer.Add(time_interval.size = entries_size);
       if (stats_buffer.length() >= prefered_chunk_size) {
         OutputStream::WriteResult result = stream->WriteUint32Chunk(
             &stats_buffer.first(), stats_buffer.length());
@@ -1596,7 +1566,7 @@ Handle<HeapObject> HeapSnapshotsCollection::FindHeapObjectById(
   for (HeapObject* obj = iterator.next();
        obj != NULL;
        obj = iterator.next()) {
-    if (ids_.FindObject(obj->address()) == id) {
+    if (ids_.FindEntry(obj->address()) == id) {
       ASSERT(object == NULL);
       object = obj;
       // Can't break -- kFilterUnreachable requires full heap traversal.
@@ -1897,10 +1867,13 @@ HeapEntry* V8HeapExplorer::AddEntry(HeapObject* object,
                                     const char* name,
                                     int children_count,
                                     int retainers_count) {
+  int object_size = object->Size();
+  SnapshotObjectId object_id =
+    collection_->GetObjectId(object->address(), object_size);
   return snapshot_->AddEntry(type,
                              name,
-                             collection_->GetObjectId(object->address()),
-                             object->Size(),
+                             object_id,
+                             object_size,
                              children_count,
                              retainers_count);
 }

src/profile-generator.h

@@ -706,7 +706,8 @@ class HeapObjectsMap {
   ~HeapObjectsMap();
 
   void SnapshotGenerationFinished();
-  SnapshotObjectId FindObject(Address addr);
+  SnapshotObjectId FindEntry(Address addr);
+  SnapshotObjectId FindOrAddEntry(Address addr, unsigned int size);
   void MoveObject(Address from, Address to);
   SnapshotObjectId last_assigned_id() const {
     return next_id_ - kObjectIdStep;
@@ -727,23 +728,22 @@ class HeapObjectsMap {
 
  private:
   struct EntryInfo {
-    EntryInfo(SnapshotObjectId id, Address addr)
-      : id(id), addr(addr), accessed(true) { }
-    EntryInfo(SnapshotObjectId id, Address addr, bool accessed)
-      : id(id), addr(addr), accessed(accessed) { }
+  EntryInfo(SnapshotObjectId id, Address addr, unsigned int size)
+      : id(id), addr(addr), size(size), accessed(true) { }
+  EntryInfo(SnapshotObjectId id, Address addr, unsigned int size, bool accessed)
+      : id(id), addr(addr), size(size), accessed(accessed) { }
     SnapshotObjectId id;
     Address addr;
+    unsigned int size;
     bool accessed;
   };
   struct TimeInterval {
-    explicit TimeInterval(SnapshotObjectId id) : id(id), count(0) { }
+    explicit TimeInterval(SnapshotObjectId id) : id(id), size(0), count(0) { }
     SnapshotObjectId id;
+    unsigned int size;
     uint32_t count;
   };
 
-  void AddEntry(Address addr, SnapshotObjectId id);
-  SnapshotObjectId FindEntry(Address addr);
-  SnapshotObjectId FindOrAddEntry(Address addr);
   void UpdateHeapObjectsMap();
   void RemoveDeadEntries();
 
@@ -757,7 +757,6 @@ class HeapObjectsMap {
         v8::internal::kZeroHashSeed);
   }
 
-  bool initial_fill_mode_;
   SnapshotObjectId next_id_;
   HashMap entries_map_;
   List<EntryInfo>* entries_;
@@ -789,7 +788,9 @@ class HeapSnapshotsCollection {
   StringsStorage* names() { return &names_; }
   TokenEnumerator* token_enumerator() { return token_enumerator_; }
 
-  SnapshotObjectId GetObjectId(Address addr) { return ids_.FindObject(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) { ids_.MoveObject(from, to); }
   SnapshotObjectId last_assigned_id() const {

test/cctest/test-heap-profiler.cc

@@ -700,6 +700,7 @@ class TestStatsStream : public v8::OutputStream {
     : eos_signaled_(0),
       numbers_written_(0),
       entries_count_(0),
+      entries_size_(0),
       intervals_count_(0),
       first_interval_index_(-1) { }
   TestStatsStream(const TestStatsStream& stream)
@@ -707,6 +708,7 @@ class TestStatsStream : public v8::OutputStream {
       eos_signaled_(stream.eos_signaled_),
       numbers_written_(stream.numbers_written_),
       entries_count_(stream.entries_count_),
+      entries_size_(stream.entries_size_),
       intervals_count_(stream.intervals_count_),
       first_interval_index_(stream.first_interval_index_) { }
   virtual ~TestStatsStream() {}
@@ -722,14 +724,17 @@ class TestStatsStream : public v8::OutputStream {
     entries_count_ = 0;
     if (first_interval_index_ == -1 && numbers_written != 0)
       first_interval_index_ = buffer[0];
-    for (int i = 1; i < numbers_written; i += 2)
-      entries_count_ += buffer[i];
+    for (int i = 0; i < numbers_written; i += 3) {
+      entries_count_ += buffer[i+1];
+      entries_size_ += buffer[i+2];
+    }
 
     return kContinue;
   }
   int eos_signaled() { return eos_signaled_; }
   int numbers_written() { return numbers_written_; }
   uint32_t entries_count() const { return entries_count_; }
+  uint32_t entries_size() const { return entries_size_; }
   int intervals_count() const { return intervals_count_; }
   int first_interval_index() const { return first_interval_index_; }
 
@@ -737,6 +742,7 @@ class TestStatsStream : public v8::OutputStream {
   int eos_signaled_;
   int numbers_written_;
   uint32_t entries_count_;
+  uint32_t entries_size_;
   int intervals_count_;
   int first_interval_index_;
 };
@@ -766,7 +772,8 @@ TEST(HeapSnapshotObjectsStats) {
     // Single chunk of data expected in update. Initial data.
     TestStatsStream stats_update = GetHeapStatsUpdate();
     CHECK_EQ(1, stats_update.intervals_count());
-    CHECK_EQ(2, stats_update.numbers_written());
+    CHECK_EQ(3, stats_update.numbers_written());
+    CHECK_LT(0, stats_update.entries_size());
     CHECK_EQ(0, stats_update.first_interval_index());
   }
 
@@ -779,7 +786,8 @@ TEST(HeapSnapshotObjectsStats) {
       // Single chunk of data with one new entry expected in update.
       TestStatsStream stats_update = GetHeapStatsUpdate();
       CHECK_EQ(1, stats_update.intervals_count());
-      CHECK_EQ(2, stats_update.numbers_written());
+      CHECK_EQ(3, stats_update.numbers_written());
+      CHECK_LT(0, stats_update.entries_size());
       CHECK_EQ(1, stats_update.entries_count());
       CHECK_EQ(2, stats_update.first_interval_index());
     }
@@ -791,6 +799,7 @@ TEST(HeapSnapshotObjectsStats) {
       v8::HandleScope inner_scope_2;
       v8::Local<v8::String> string2 = v8_str("string2");
 
+      uint32_t entries_size;
       {
         v8::HandleScope inner_scope_3;
         v8::Handle<v8::String> string3 = v8::String::New("string3");
@@ -800,7 +809,8 @@ TEST(HeapSnapshotObjectsStats) {
           // Single chunk of data with three new entries expected in update.
           TestStatsStream stats_update = GetHeapStatsUpdate();
           CHECK_EQ(1, stats_update.intervals_count());
-          CHECK_EQ(2, stats_update.numbers_written());
+          CHECK_EQ(3, stats_update.numbers_written());
+          CHECK_LT(0, entries_size = stats_update.entries_size());
           CHECK_EQ(3, stats_update.entries_count());
           CHECK_EQ(4, stats_update.first_interval_index());
         }
@@ -810,7 +820,8 @@ TEST(HeapSnapshotObjectsStats) {
         // Single chunk of data with two left entries expected in update.
         TestStatsStream stats_update = GetHeapStatsUpdate();
         CHECK_EQ(1, stats_update.intervals_count());
-        CHECK_EQ(2, stats_update.numbers_written());
+        CHECK_EQ(3, stats_update.numbers_written());
+        CHECK_GT(entries_size, stats_update.entries_size());
         CHECK_EQ(1, stats_update.entries_count());
         // Two strings from forth interval were released.
         CHECK_EQ(4, stats_update.first_interval_index());
@@ -821,7 +832,8 @@ TEST(HeapSnapshotObjectsStats) {
       // Single chunk of data with 0 left entries expected in update.
       TestStatsStream stats_update = GetHeapStatsUpdate();
       CHECK_EQ(1, stats_update.intervals_count());
-      CHECK_EQ(2, stats_update.numbers_written());
+      CHECK_EQ(3, stats_update.numbers_written());
+      CHECK_EQ(0, stats_update.entries_size());
       CHECK_EQ(0, stats_update.entries_count());
       // The last string from forth interval was released.
       CHECK_EQ(4, stats_update.first_interval_index());
@@ -831,12 +843,45 @@ TEST(HeapSnapshotObjectsStats) {
     // Single chunk of data with 0 left entries expected in update.
     TestStatsStream stats_update = GetHeapStatsUpdate();
     CHECK_EQ(1, stats_update.intervals_count());
-    CHECK_EQ(2, stats_update.numbers_written());
+    CHECK_EQ(3, stats_update.numbers_written());
+    CHECK_EQ(0, stats_update.entries_size());
     CHECK_EQ(0, stats_update.entries_count());
     // The only string from the second interval was released.
     CHECK_EQ(2, stats_update.first_interval_index());
   }
 
+  v8::Local<v8::Array> array = v8::Array::New();
+  CHECK_EQ(0, array->Length());
+  // Force array's buffer allocation.
+  array->Set(2, v8_num(7));
+
+  uint32_t entries_size;
+  {
+    // Single chunk of data with 2 entries expected in update.
+    TestStatsStream stats_update = GetHeapStatsUpdate();
+    CHECK_EQ(1, stats_update.intervals_count());
+    CHECK_EQ(3, stats_update.numbers_written());
+    CHECK_LT(0, entries_size = stats_update.entries_size());
+    // They are the array and its buffer.
+    CHECK_EQ(2, stats_update.entries_count());
+    CHECK_EQ(8, stats_update.first_interval_index());
+  }
+
+  for (int i = 0; i < 100; ++i)
+    array->Set(i, v8_num(i));
+
+  {
+    // Single chunk of data with 1 entry expected in update.
+    TestStatsStream stats_update = GetHeapStatsUpdate();
+    CHECK_EQ(1, stats_update.intervals_count());
+    // The first interval was changed because old buffer was collected.
+    // The second interval was changed because new buffer was allocated.
+    CHECK_EQ(6, stats_update.numbers_written());
+    CHECK_LT(entries_size, stats_update.entries_size());
+    CHECK_EQ(2, stats_update.entries_count());
+    CHECK_EQ(8, stats_update.first_interval_index());
+  }
+
   v8::HeapProfiler::StopHeapObjectsTracking();
 }