[Webkit-unassigned] [Bug 119417] Use op_get_by_id_custom_stub to fast access to JSCallbackObject's static value

[bugzilla-daemon at webkit.org]

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


Geoffrey Garen <ggaren at apple.com> changed:

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




--- Comment #8 from Geoffrey Garen <ggaren at apple.com>  2013-08-28 17:52:10 PST ---
(From update of attachment 208682)
View in context: https://bugs.webkit.org/attachment.cgi?id=208682&action=review

I think this approach will work, but I'd like to see some refinement.

> Source/JavaScriptCore/API/JSCallbackObjectFunctions.h:172
> +                    JSValue value = getStaticValue(exec, thisObject, propertyName);
>                      if (value) {
> +                        if (thisObject->classRef()->isCacheable())
> +                            entry->cacheable = true;

I'd prefer to see the cacheable field initialized when the StaticValueEntry was created. You can do this in OpaqueJSClassContextData::OpaqueJSClassContextData().

> Source/JavaScriptCore/API/JSCallbackObjectFunctions.h:189
> +    slot.setUnCacheable(); // Don't cache Parent's property

I don't think this comment adds anything that wasn't there in code.

Also, we should only do this if our class is uncacheable, right? We don't want to make all parent class properties unconditionally uncacheable.

> Source/JavaScriptCore/API/JSCallbackObjectFunctions.h:301
> +    Parent::put(thisObject, exec, propertyName, value, slot);
> +    slot.setUnCacheable(); // Don't cache Parent's property
> +    return;

Ditto.

> Source/JavaScriptCore/API/JSClassRef.h:48
> +    bool cacheable;

Let's call this "isCacheable".

> Source/JavaScriptCore/API/JSClassRef.h:127
> +    bool m_cacheable;

Let's call this "m_isCacheable".

> Source/JavaScriptCore/runtime/PropertySlot.h:64
> +    void setUnCacheable() { m_offset = invalidOffset; }

"uncacheable" is one word, so let's call this "setUncacheable".

> Source/JavaScriptCore/runtime/PutPropertySlot.h:68
> +        void setUnCacheable() { m_type = Uncachable; }

Ditto.

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