Count ArrayBuffer's backing_store memory in heap snapshot.

Count ArrayBuffer's backing_store memory in heap snapshot.

BUG=341741
LOG=N
R=dslomov@chromium.org, loislo@chromium.org

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

[alph, 2014-02-13 14:15:39]

ptal

[loislo, 2014-02-13 14:20:03]

On 2014/02/13 14:15:39, alph wrote:
> ptal

lgtm

[alph, 2014-02-13 14:21:53]

Dmitry, could you please take a look.

[Dmitry Lomov (no reviews), 2014-02-13 14:28:01]

A nit and a real comment.

https://codereview.chromium.org/163593002/diff/1/src/heap-snapshot-generator.cc
File src/heap-snapshot-generator.cc (right):

https://codereview.chromium.org/163593002/diff/1/src/heap-snapshot-generator....
src/heap-snapshot-generator.cc:1460: int entry, JSArrayBuffer* array) {
Nit: array -> buffer

https://codereview.chromium.org/163593002/diff/1/src/heap-snapshot-generator....
src/heap-snapshot-generator.cc:1467: int data_size =
NumberToInt32(array->byte_length());
ArrayBuffer's byte length should be size_t, not int. Use
NumberToSize/TryNumberToSize here.

[alph, 2014-02-13 14:49:13]

ptal

https://codereview.chromium.org/163593002/diff/1/src/heap-snapshot-generator.cc
File src/heap-snapshot-generator.cc (right):

https://codereview.chromium.org/163593002/diff/1/src/heap-snapshot-generator....
src/heap-snapshot-generator.cc:1460: int entry, JSArrayBuffer* array) {
On 2014/02/13 14:28:01, Dmitry Lomov (chromium) wrote:
> Nit: array -> buffer

Done.

https://codereview.chromium.org/163593002/diff/1/src/heap-snapshot-generator....
src/heap-snapshot-generator.cc:1467: int data_size =
NumberToInt32(array->byte_length());
On 2014/02/13 14:28:01, Dmitry Lomov (chromium) wrote:
> ArrayBuffer's byte length should be size_t, not int. Use
> NumberToSize/TryNumberToSize here.

I thought about it, but I found that [currently] ArrayBuffer in JS does not
support sizes larger than int32.

Anyway to be on the safe side, I added an ASSERT.

[Dmitry Lomov (no reviews), 2014-02-13 15:20:45]

lgtm with a nit

Also, I do recommend changing type size in SnapshotEntry to size_t - not
necessarily in this CL.
V8 code supports allocating >2GB ArrayBuffers, even though currently Blink
limits that to <1GB (just try it in ia32 d8: var x = new
ArrayBuffer(0x80000000))

https://codereview.chromium.org/163593002/diff/70001/src/heap-snapshot-genera...
File src/heap-snapshot-generator.cc (right):

https://codereview.chromium.org/163593002/diff/70001/src/heap-snapshot-genera...
src/heap-snapshot-generator.cc:1468: ASSERT(data_size <= kMaxInt);
Use CHECK instead of ASSERT to crash in Release mode as well.

[alph, 2014-02-13 15:27:24]

On 2014/02/13 15:20:45, Dmitry Lomov (chromium) wrote:
> lgtm with a nit
> 
> Also, I do recommend changing type size in SnapshotEntry to size_t - not
> necessarily in this CL.

That's among our plans. We have recently moved snapshot storage to the client
side, so we're not limited in snapshot memory in v8 anymore, and can afford
increasing heap entry size.

> V8 code supports allocating >2GB ArrayBuffers, even though currently Blink
> limits that to <1GB (just try it in ia32 d8: var x = new
> ArrayBuffer(0x80000000))
> 
>
https://codereview.chromium.org/163593002/diff/70001/src/heap-snapshot-genera...
> File src/heap-snapshot-generator.cc (right):
> 
>
https://codereview.chromium.org/163593002/diff/70001/src/heap-snapshot-genera...
> src/heap-snapshot-generator.cc:1468: ASSERT(data_size <= kMaxInt);
> Use CHECK instead of ASSERT to crash in Release mode as well.

[alph, 2014-02-13 15:27:37]

https://codereview.chromium.org/163593002/diff/70001/src/heap-snapshot-genera...
File src/heap-snapshot-generator.cc (right):

https://codereview.chromium.org/163593002/diff/70001/src/heap-snapshot-genera...
src/heap-snapshot-generator.cc:1468: ASSERT(data_size <= kMaxInt);
On 2014/02/13 15:20:46, Dmitry Lomov (chromium) wrote:
> Use CHECK instead of ASSERT to crash in Release mode as well.
ok

[alph, 2014-02-13 15:31:53]

Committed patchset #5 manually as r19356 (presubmit successful).