Revision: 11327
Author: [email protected]
Date: Mon Apr 16 02:44:42 2012
Log: 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
http://code.google.com/p/v8/source/detail?r=11327
Modified:
/branches/bleeding_edge/include/v8-profiler.h
/branches/bleeding_edge/src/profile-generator.cc
/branches/bleeding_edge/src/profile-generator.h
/branches/bleeding_edge/test/cctest/test-heap-profiler.cc
=======================================
--- /branches/bleeding_edge/include/v8-profiler.h Fri Apr 13 05:50:48 2012
+++ /branches/bleeding_edge/include/v8-profiler.h Mon Apr 16 02:44:42 2012
@@ -433,8 +433,9 @@
* 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.
=======================================
--- /branches/bleeding_edge/src/profile-generator.cc Fri Apr 13 05:50:48
2012
+++ /branches/bleeding_edge/src/profile-generator.cc Mon Apr 16 02:44:42
2012
@@ -1314,8 +1314,7 @@
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 @@
// 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,25 +1334,8 @@
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) {
@@ -1377,38 +1359,21 @@
}
to_entry->value = reinterpret_cast<void*>(from_entry_info_index);
}
-
-
-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 @@
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::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());
- }
- initial_fill_mode_ = false;
+ FindOrAddEntry(obj->address(), obj->Size());
+ }
RemoveDeadEntries();
}
@@ -1458,14 +1423,19 @@
++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 @@
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 @@
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);
}
=======================================
--- /branches/bleeding_edge/src/profile-generator.h Fri Apr 13 05:50:48 2012
+++ /branches/bleeding_edge/src/profile-generator.h Mon Apr 16 02:44:42 2012
@@ -706,7 +706,8 @@
~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 @@
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 @@
v8::internal::kZeroHashSeed);
}
- bool initial_fill_mode_;
SnapshotObjectId next_id_;
HashMap entries_map_;
List<EntryInfo>* entries_;
@@ -789,7 +788,9 @@
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 {
=======================================
--- /branches/bleeding_edge/test/cctest/test-heap-profiler.cc Fri Apr 13
05:50:48 2012
+++ /branches/bleeding_edge/test/cctest/test-heap-profiler.cc Mon Apr 16
02:44:42 2012
@@ -700,6 +700,7 @@
: 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 @@
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 @@
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 @@
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 @@
// 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 @@
// 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 @@
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 @@
// 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 @@
// 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 @@
// 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,11 +843,44 @@
// 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();
}
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
