[webkit-reviews] review denied: [Bug 41920] Avoid slow-path for put() in Array.splice() : [Attachment 61272] Proposed patch v2

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Jul 12 15:36:55 PDT 2010


Darin Adler <darin at apple.com> has denied Andreas Kling
<andreas.kling at nokia.com>'s request for review:
Bug 41920: Avoid slow-path for put() in Array.splice()
https://bugs.webkit.org/show_bug.cgi?id=41920

Attachment 61272: Proposed patch v2
https://bugs.webkit.org/attachment.cgi?id=61272&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
Great that you're tackling this!

> -	   JSArray(NonNullPassRefPtr<Structure>, unsigned initialLength);
> +	   JSArray(NonNullPassRefPtr<Structure>, unsigned initialLength, bool
compact = false);

In the WebKit project we strongly discourage using booleans for arguments like
this one where callers pass constants. Instead, we normally use enums.

Since there is only one place where we call this, I think the default for the
boolean argument isn't a good idea. No reason to have the default.

The patch otherwise looks pretty good. But setIndex is not fast enough. We want
to just set m_numValuesInVector, m_length, and the values in
m_storage->m_vector, without any branches and without looping. We should come
up with a new code path specifically designed for creating a new array
efficiently.

I’m going to say review- because of the boolean.


More information about the webkit-reviews mailing list