RESOLVED FIXED 154897
[[SetPrototypeOf]] should be a fully virtual method in ClassInfo::methodTable
https://bugs.webkit.org/show_bug.cgi?id=154897
Summary [[SetPrototypeOf]] should be a fully virtual method in ClassInfo::methodTable
Saam Barati
Reported 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
Attachments
perf results (64.04 KB, text/plain)
2016-03-02 12:32 PST, Saam Barati
no flags
patch (18.17 KB, patch)
2016-03-02 12:40 PST, Saam Barati
fpizlo: review+
Saam Barati
Comment 1 2016-03-02 12:32:15 PST
Created attachment 272677 [details] perf results looks neutral
Saam Barati
Comment 2 2016-03-02 12:40:19 PST
Saam Barati
Comment 3 2016-03-02 14:39:24 PST
Geoffrey Garen
Comment 4 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.
Saam Barati
Comment 5 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
Note You need to log in before you can comment on or make changes to this bug.