Bug 28664 – Array index miss case creates a string every time

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Darin Adler]

The object prototype is creating a string and searching for it in the Identifier table every time there's a miss. This is bad on the crypto tests in SunSpider, and in other real-world code too I'm sure.

[Darin Adler]

Created attachment 38446 [details]
patch

[Darin Adler]

Comment on attachment 38446 [details]
patch

Maybe it should be called m_mayHaveIntegralNamedProperty because I don't have any code to set it to false if the offending properties are all removed.

[Darin Adler]

Comment on attachment 38446 [details]
patch

Or I could reverse its sense and call this m_hasNoIntegralNamedProperties.

[David Kilzer (:ddkilzer)]

Comment on attachment 38446 [details]
patch

>+    if (isUInt32)
>+        m_hasIntegralNamedProperty = true;

You could save a branch here by always assigning m_hasIntegralNamedProperty:

    m_hasIntegralNamedProperty = isUInt32;

Not sure if that's helpful or not, though.

[Darin Adler]

(In reply to comment #4)
> (From update of attachment 38446 [details])
> >+    if (isUInt32)
> >+        m_hasIntegralNamedProperty = true;
> 
> You could save a branch here by always assigning m_hasIntegralNamedProperty:
> 
>     m_hasIntegralNamedProperty = isUInt32;
> 
> Not sure if that's helpful or not, though.

That would break the case where isUInt32 is false and m_hasIntegralNamedProperty is already true, but I could do:

    m_hasIntegralNamedProperty |= isUInt32;

Or I could skip the entire check if m_hasIntegralNamedProperty is already true. No need to call toStrictUInt32 at all in that case.

But none of that seems important. We don't need to optimize adding properties to the object prototypes. We should optimize for clarity. My existing patch seems OK to me, but we could make changes.

[Geoffrey Garen]

Comment on attachment 38446 [details]
patch

> +    return m_hasIntegralNamedProperty && JSObject::getOwnPropertySlot(exec, propertyName, slot);

I tend to think that code like this is more readable if the m_hasIntegralNamedProperty case is an explicit early return. Up to you.

r=me

[Darin Adler]

http://trac.webkit.org/changeset/47727

[Darin Adler]

http://trac.webkit.org/changeset/47730