Bug 131579 – Make NodeList.length inline-cacheable by JSC.

• Status: RESOLVED
• Resolution: FIXED
• Component: Bindings

[Andreas Kling]

NodeList.length currently isn't inline-cacheable because CodeGeneratorJS.pm makes everything that has and index or named getter uncacheable.
I don't think we need to be doing that.

[Andreas Kling]

Created attachment 229193 [details]
Patch

[Andreas Kling]

Created attachment 229194 [details]
Patch

[Geoffrey Garen]

Comment on attachment 229194 [details]
Patch

r=me

[Build Bot]

Comment on attachment 229194 [details]
Patch

Attachment 229194 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/5534621794041856

New failing tests:
fast/dom/SelectorAPI/resig-SelectorsAPI-test.xhtml

[Build Bot]

Created attachment 229195 [details]
Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-15  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 229194 [details]
Patch

Attachment 229194 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/6509598697586688

New failing tests:
fast/dom/SelectorAPI/resig-SelectorsAPI-test.xhtml

[Build Bot]

Created attachment 229197 [details]
Archive of layout-test-results from webkit-ews-03 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-03  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Andreas Kling]

Comment on attachment 229194 [details]
Patch

I have a cleaner fix.

[Andreas Kling]

Created attachment 229198 [details]
Patch

Better patch: just mark the slot cacheable when we find a builtin that takes precedence over a named/indexed item.

[Andreas Kling]

Created attachment 229199 [details]
Patch

(With updated bindings test results this time.)

[Oliver Hunt]

Comment on attachment 229199 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=229199&action=review

> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:407
>              push(@getOwnPropertySlotImpl, "    const ${namespaceMaybe}HashTableValue* entry = getStaticValueSlotEntryWithoutCaching<$className>(exec, propertyName);\n");

Do  we still want getStaticValueSlotEntryWithoutCaching?

[Andreas Kling]

Comment on attachment 229199 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=229199&action=review

>> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:407
>>              push(@getOwnPropertySlotImpl, "    const ${namespaceMaybe}HashTableValue* entry = getStaticValueSlotEntryWithoutCaching<$className>(exec, propertyName);\n");
> 
> Do  we still want getStaticValueSlotEntryWithoutCaching?

I think so. Plain getStaticValueSlot will fall back to the parent's getOwnPropertySlot, that's not what we want right?

[Oliver Hunt]

(In reply to comment #12)
> (From update of attachment 229199 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=229199&action=review
> 
> >> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:407
> >>              push(@getOwnPropertySlotImpl, "    const ${namespaceMaybe}HashTableValue* entry = getStaticValueSlotEntryWithoutCaching<$className>(exec, propertyName);\n");
> > 
> > Do  we still want getStaticValueSlotEntryWithoutCaching?
> 
> I think so. Plain getStaticValueSlot will fall back to the parent's getOwnPropertySlot, that's not what we want right?

Haha, the sole point of getStaticValueSlotEntryWithoutCaching is to get the property getter (and setter?) without taking a property slot and doing setCacheableCustom

[Andreas Kling]

Committed r167181: <http://trac.webkit.org/changeset/167181>