Add size metric into Heap Stats.

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

Committed: https://code.google.com/p/v8/source/detail?r=11327

[mnaganov (inactive), 2012-04-13 14:36:41]

https://chromiumcodereview.appspot.com/10086004/diff/1/src/profile-generator.cc
File src/profile-generator.cc (right):

https://chromiumcodereview.appspot.com/10086004/diff/1/src/profile-generator....
src/profile-generator.cc:1435: stats_buffer.Add(time_interval.size =
entries_size);
It seems, you need to update the comment on PushHeapObjectsStats in
v8-profiler.h

https://chromiumcodereview.appspot.com/10086004/diff/1/test/cctest/test-heap-...
File test/cctest/test-heap-profiler.cc (right):

https://chromiumcodereview.appspot.com/10086004/diff/1/test/cctest/test-heap-...
test/cctest/test-heap-profiler.cc:732: entries_count_ += buffer[i+1];
Still not considering defining the stream structure somewhere in v8-profiler.h?
This looks pretty unreadable.

https://chromiumcodereview.appspot.com/10086004/diff/1/test/cctest/test-heap-...
test/cctest/test-heap-profiler.cc:827: CHECK_LT(0, stats_update.entries_size());
I think you can store the previous value of size and verify its relative
changes. Otherwise, you are not testing enough.

[mnaganov (inactive), 2012-04-13 14:42:31]

https://chromiumcodereview.appspot.com/10086004/diff/1/test/cctest/test-heap-...
File test/cctest/test-heap-profiler.cc (right):

https://chromiumcodereview.appspot.com/10086004/diff/1/test/cctest/test-heap-...
test/cctest/test-heap-profiler.cc:858: 
Also, it might be worth testing that you can really catch the leak scenario from
the change description:

"A constantly growing array of integers also can be a reason of a leak"

[yurys, 2012-04-13 15:01:19]

https://chromiumcodereview.appspot.com/10086004/diff/1/src/profile-generator.cc
File src/profile-generator.cc (left):

https://chromiumcodereview.appspot.com/10086004/diff/1/src/profile-generator....
src/profile-generator.cc:1315: : initial_fill_mode_(true),
Please remove the field from header.

[alexeif, 2012-04-13 15:01:27]

https://chromiumcodereview.appspot.com/10086004/diff/1/src/profile-generator.cc
File src/profile-generator.cc (right):

https://chromiumcodereview.appspot.com/10086004/diff/1/src/profile-generator....
src/profile-generator.cc:1382: entry_info.size = size;
why do you need to update the size? If the size is different then it's probably
a new object. If so we should get a new id for it.

https://chromiumcodereview.appspot.com/10086004/diff/1/src/profile-generator....
src/profile-generator.cc:1427: ++entries_count;
you still don't need to increase the count on each iteration.

https://chromiumcodereview.appspot.com/10086004/diff/1/src/profile-generator.h
File src/profile-generator.h (right):

https://chromiumcodereview.appspot.com/10086004/diff/1/src/profile-generator....
src/profile-generator.h:729: EntryInfo(SnapshotObjectId id, Address addr,
unsigned int size)
padding?

https://chromiumcodereview.appspot.com/10086004/diff/1/src/profile-generator....
src/profile-generator.h:731: EntryInfo(SnapshotObjectId id, Address addr,
unsigned int size, bool accessed)
why not a default value for accessed?

[loislo, 2012-04-13 16:32:44]

On 2012/04/13 15:01:27, alexeif wrote:
>
https://chromiumcodereview.appspot.com/10086004/diff/1/src/profile-generator.cc
> File src/profile-generator.cc (right):
> 
>
https://chromiumcodereview.appspot.com/10086004/diff/1/src/profile-generator....
> src/profile-generator.cc:1382: entry_info.size = size;
> why do you need to update the size? If the size is different then it's
probably
> a new object. If so we should get a new id for it.

The both count and size fields are useful.
Also v8 has optimizations for specific use cases and can grow arrays in place.

> 
>
https://chromiumcodereview.appspot.com/10086004/diff/1/src/profile-generator....
> src/profile-generator.cc:1427: ++entries_count;
> you still don't need to increase the count on each iteration.

done

[mnaganov (inactive), 2012-04-16 08:13:54]

https://chromiumcodereview.appspot.com/10086004/diff/1005/src/profile-generat...
File src/profile-generator.cc (right):

https://chromiumcodereview.appspot.com/10086004/diff/1005/src/profile-generat...
src/profile-generator.cc:1430: uint32_t entries_count = entry_info -
start_entry_info;
Are you sure this compiles on all platforms, for example, on 64-bit MSVC?

https://chromiumcodereview.appspot.com/10086004/diff/1005/test/cctest/test-he...
File test/cctest/test-heap-profiler.cc (right):

https://chromiumcodereview.appspot.com/10086004/diff/1005/test/cctest/test-he...
test/cctest/test-heap-profiler.cc:818: entries_size =
stats_update.entries_size();
I'd transpose these two lines to reuse entries_size in the check statement.

https://chromiumcodereview.appspot.com/10086004/diff/1005/test/cctest/test-he...
test/cctest/test-heap-profiler.cc:829: CHECK_LT(stats_update.entries_size(),
entries_size);
Transpose arguments, CHECK_LT -> CHECK_GT

https://chromiumcodereview.appspot.com/10086004/diff/1005/test/cctest/test-he...
test/cctest/test-heap-profiler.cc:870: entries_size =
stats_update.entries_size();
Also transpose lines

https://chromiumcodereview.appspot.com/10086004/diff/1005/test/cctest/test-he...
test/cctest/test-heap-profiler.cc:871: // It were the array and it's buffer.
nit: "It were" -> "There are", "it's" -> "its"

[loislo, 2012-04-16 08:49:08]

done

[mnaganov (inactive), 2012-04-16 09:35:04]

lgtm