[Webkit-unassigned] [Bug 94810] Vector::shrinkToFit should use realloc when canCopyWithMemcpy is true

[bugzilla-daemon at webkit.org]

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


Benjamin Poulain <benjamin at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #161042|review?                     |review-
               Flag|                            |




--- Comment #23 from Benjamin Poulain <benjamin at webkit.org>  2012-08-28 22:55:31 PST ---
(From update of attachment 161042)
View in context: https://bugs.webkit.org/attachment.cgi?id=161042&action=review

I just r- for the minor issues, but everything looks good really. 

I'll have to double check the VectorTraits<T>::canMoveWithMemcpy, I do not remember by heart if it is the right one.

> Source/WTF/ChangeLog:8
> +        Only tested on BlackBerry. So it is wrapped with PLATFORM(BLACKBERRY) at the mean time.

"at the mean time" -> "in the meantime"/"for now"

> Source/WTF/ChangeLog:12
> +        Use realloc to shrink buffer when it is changing from a large malloc buffer
> +        to another large one (> 1k), and canMoveWithMemcpy is true. When a Vector<char> shrinks
> +        capacity from 2K to 1K, this gives more than 10x times performance boost on shrinkToFit()
> +        (Tested on BlackBerry).

This is outdated, your removed the 1K limit.
Please use a new paragraph for the performance detail, that'll make it easier to track.

10 times faster!!! I should really try your bench with fastMalloc!

> Source/WTF/wtf/Vector.h:288
> +            // FIXME: Return true when realloc() can give better performance.

The "when" is ambiguous IMHO. More like:
// FIXME: Return true on the platform where realloc() give better performance.
?

+ UNUSED_PARAM(newCapacity); (for the Mac break).

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