[Webkit-unassigned] [Bug 94712] High memory usage spike in AssemblerBuffer

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=94712





--- Comment #20 from Benjamin Poulain <benjamin at webkit.org>  2012-08-24 16:06:22 PST ---
(From update of attachment 160468)
View in context: https://bugs.webkit.org/attachment.cgi?id=160468&action=review

This generally looks like it could fly, SegmentedVector can likely be efficient enough for the few copy operations.

I am _really_ not a fan of having this only on ARM_THUMB2. Please ask about that to the JSC experts.

No review from me, especially without hard numbers, but here are a few comments:

> Source/WTF/wtf/SegmentedVector.h:173
> +            m_size += length;

ASSERT for overflow would be nice.

> Source/WTF/wtf/SegmentedVector.h:177
> +                if (segmentIndex == m_segments.size())
> +                    m_segments.append(new Segment);

I would do this upfront before while (length) to avoid executing the branch for every iteration when it is not needed.

>> Source/WTF/wtf/SegmentedVector.h:235
>> +            ASSERT(start < m_size);
> 
> will change it to ASSERT(start <= m_size); so it won't assert when getElements(buffer, 0, 0) is called on an empty SegmentedVector

> will change it to ASSERT(start <= m_size); so it won't assert when getElements(buffer, 0, 0) is called on an empty SegmentedVector

Maybe:
ASSERT(start < m_size || (!start && !m_size));
Because "start < m_size" seems better if m_size > 0.

> Source/WTF/wtf/SegmentedVector.h:243
> +            size_t segment = start / SegmentSize;

segment -> segmentIndex

Similarily to subscriptFor().,I am tempted to have a 
size_t segmentIndexFor(absoluteIndex) {
    return absoluteIndex / SegmentSize;
}

> Source/WTF/wtf/SegmentedVector.h:275
> +        {

Add ASSERT(index < m_size); ?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.