Bug 154897 - [[SetPrototypeOf]] should be a fully virtual method in ClassInfo::methodTable
Summary: [[SetPrototypeOf]] should be a fully virtual method in ClassInfo::methodTable
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Saam Barati
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-03-01 17:22 PST by Saam Barati
Modified: 2016-03-02 21:26 PST (History)
10 users (show)

See Also:


Attachments
perf results (64.04 KB, text/plain)
2016-03-02 12:32 PST, Saam Barati
no flags Details
patch (18.17 KB, patch)
2016-03-02 12:40 PST, Saam Barati
fpizlo: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Saam Barati 2016-03-01 17:22:10 PST
We can probably make this fast just by using the setPrototypeOfInline paradigm
where we will often be able to inline this call as JSObject::setPrototypeWithCycleCheck.

This is needed for Proxy
Comment 1 Saam Barati 2016-03-02 12:32:15 PST
Created attachment 272677 [details]
perf results

looks neutral
Comment 2 Saam Barati 2016-03-02 12:40:19 PST
Created attachment 272678 [details]
patch
Comment 3 Saam Barati 2016-03-02 14:39:24 PST
landed in:
http://trac.webkit.org/changeset/197467
Comment 4 Geoffrey Garen 2016-03-02 14:56:00 PST
Comment on attachment 272678 [details]
patch

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

> Source/JavaScriptCore/runtime/ClassInfo.h:113
> +    typedef bool (*SetPrototypeOfFunctionPtr)(JSObject*, ExecState*, JSValue);
> +    SetPrototypeOfFunctionPtr setPrototypeOf;

When referring to the object constructor, the spec says "setPrototypeOf" because the object constructor sets the prototype *of* another object.

But when we refer to an object setting its own prototype, that should be called "setPrototype". This applies to lots of places in this patch.

> Source/JavaScriptCore/runtime/JSObject.h:122
> +    ALWAYS_INLINE bool setPrototypeOfInline(VM& vm, ExecState* exec, JSValue prototype)

Why do we have this inline variant? I think you should remove it.
Comment 5 Saam Barati 2016-03-02 21:26:11 PST
(In reply to comment #4)
> Comment on attachment 272678 [details]
> patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=272678&action=review
> 
> > Source/JavaScriptCore/runtime/ClassInfo.h:113
> > +    typedef bool (*SetPrototypeOfFunctionPtr)(JSObject*, ExecState*, JSValue);
> > +    SetPrototypeOfFunctionPtr setPrototypeOf;
> 
> When referring to the object constructor, the spec says "setPrototypeOf"
> because the object constructor sets the prototype *of* another object.
> 
> But when we refer to an object setting its own prototype, that should be
> called "setPrototype". This applies to lots of places in this patch.
> 
> > Source/JavaScriptCore/runtime/JSObject.h:122
> > +    ALWAYS_INLINE bool setPrototypeOfInline(VM& vm, ExecState* exec, JSValue prototype)
> 
> Why do we have this inline variant? I think you should remove it.

Follow up to your comment in:
http://trac.webkit.org/changeset/197484