552230 - Profiler incorrectly reports memory taken by Array

Status: VERIFIED FIXED

(Tamarin Graveyard :: Virtual Machine, defect, P2)

Description

[Mike Morearty]

Note, this is very similar to bug 552229 in many ways, but that bug is about the memory taken by dynamic properties of an Object; this bug is about properties of an Array.

Arrays internally keep a dense array (AtomArray), which is used for members that were added sequentially, e.g. a[0], a[1], etc.  It periodically grows in size by 50%.  When the profiler asks the VM for the size of an Array (e.g. by calling flash.sampler.getSize()), the VM attempts to include the memory taken by the dense array, but it calculates the memory as "getLength() * sizeof(Atom)".  This is misleading because it does not include memory that has been reserved but not yet used.  The calculation should be the dense array's capacity * sizeof(Atom), not its length * sizeof(Atom).

Additionally, the current calculation only accounts for the dense array if array contains no non-dense members.

And finally, the fixed sizeof(AtomArray) should also be included in the size of the Array instance.

Steps to reproduce:
1. Compile and run this sample, which adds one property at a time to an Array and then reports its new size.  Also, at the end, it adds one member which will not be part of the dense array, and then it reports the size one more time:

    import flash.sampler.*;

    var a:Array = [];

    for (var i=0; i<=10000; ++i)
    {
        trace(i, getSize(a));
        a[i] = true;
    }

    a[100000] = true;

    trace('after adding one non-dense member:', getSize(a));

Actual result:
The array size is reported as growing by exactly 4 bytes each time a property is added:
    0 56
    1 60
    2 64
    3 68
    4 72
    5 76
    6 80
    7 84
    8 88
    9 92
    10 96
    ...

In addition, at the end, after adding a non-dense element, the array size is erroneously reported as having decreased dramatically:

    ...
    9997 40044
    9998 40048
    9999 40052
    10000 40056
    after adding one non-dense member: 56

Expected result:
The size should sometimes remain unchanged after adding a property, because the addition of the property did not cause any additional memory to be allocated.  Also, occasionally the size should jump by a large amount, because the addition of one property caused the dense array to be resized:
    0 64
    1 80
    2 80
    3 80
    4 80
    5 88
    6 88
    7 96
    8 96
    9 112
    10 112

In addition, at the end, after the non-dense element is added, the array size should go up or remain unchanged, not go down:

    ...
    9997 49192
    9998 49192
    9999 49192
    10000 49192
    after adding one non-dense member: 49192

Comment 1

[Mike Morearty]

Attachment: Proposed patch

Comment 2

[Mike Morearty]

> And finally, the fixed sizeof(AtomArray) should also be included in the size of
> the Array instance.

I may have been wrong about that part -- that size may already be included in the size of the Array instance.  Still trying to figure it out; may need to make a small change to my patch.  But this is a trivial part of the overall bug.

Comment 3

[Mike Morearty]

Attachment: Proposed patch

My previous comment was correct -- it turns out the sizeof(AtomArray) was already accounted for, so I should not have been adding that in.  This new patch fixes the more serious problems with the dense-array calculation, but does not add sizeof(AtomArray) to the total.

Comment 4

[Mike Morearty]

I would recommend the target milestone be 10.1.

Comment 5

[Edwin Smith]

Comment on attachment 432366 [details] [diff] [review]
Proposed patch

Rubber stamp:  looks more accurate, and doesn't appear to inject any bugs.

Comment 6

[Mike Morearty]

Fixed in changeset c7cfb8890f82

Comment 7

[Chris Peyer]

verified in argo changeset 3827	c7cfb8890f82