Bug 79129

Summary: [V8] Bindings for accessors never use the name param
Product: WebKit Reporter: Erik Arvidsson <arv>
Component: DOMAssignee: Nobody <webkit-unassigned>
Status: RESOLVED WONTFIX    
Severity: Normal CC: antonm, haraken, japhet
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   

Description Erik Arvidsson 2012-02-21 11:06:19 PST
The V8 bindings use v8::AccessorGetter and v8::AccessorSetter which takes the name of the property (as a v8::String) as the first argument. This is never[*] used in the binding code so we should get rid of it.

[*] There are two instances of these.

V8HTMLDocument::allAccessorSetter

This one can be fixed by only having a custom getter

V8MessageEvent::dataAccessorGetter:

This one can be replaced with a new v8::String
Comment 1 Erik Arvidsson 2012-02-21 11:48:04 PST
(In reply to comment #0)
> V8HTMLDocument::allAccessorSetter
> 
> This one can be fixed by only having a custom getter

Actually this one needs to be replaced with the hard coded string "all".
Comment 2 anton muhin 2012-02-24 04:03:24 PST
(In reply to comment #1)
> (In reply to comment #0)
> > V8HTMLDocument::allAccessorSetter
> > 
> > This one can be fixed by only having a custom getter
> 
> Actually this one needs to be replaced with the hard coded string "all".

Erik, do I understand you right that you want to remove it from public v8 API?

If yes, it might be questionable---sometimes it's convenient to have generic accessors.
Comment 3 Erik Arvidsson 2012-02-24 09:12:00 PST
The V8 bindings are very slow and even though this is not a big deal it is extra work that we clearly do not need.

Would it make sense to have V8 support both? Maybe supporting both would make things slower?
Comment 4 anton muhin 2012-02-24 10:07:44 PST
(In reply to comment #3)
> The V8 bindings are very slow and even though this is not a big deal it is extra work that we clearly do not need.
> 
> Would it make sense to have V8 support both? Maybe supporting both would make things slower?

I don't think v8 bindings are very slow :)

Name support on ia32 is three additional instructions (push reg, mov reg/reg, mov reg/mem) and two additional slots on the stack.  Not sure if it will save us a lot, but it's always worth trying.
Comment 5 Erik Arvidsson 2012-02-24 11:05:54 PST
V8 is generally twice as slow as JSC bindings.
Comment 6 anton muhin 2012-02-24 11:09:16 PST
A year ago we were faster :)  Looks like several huge regressions slipped in.

(In reply to comment #5)
> V8 is generally twice as slow as JSC bindings.
Comment 7 Kentaro Hara 2012-02-24 15:21:26 PST
(In reply to comment #4)
> (In reply to comment #3)
> > Would it make sense to have V8 support both? Maybe supporting both would make things slower?
> 
> I don't think v8 bindings are very slow :)

I am not yet sure how much the name support has impacted on V8 performance, but V8 is seriously slow. You can find the data in this SpreadSheet (https://docs.google.com/a/google.com/spreadsheet/ccc?key=0ArGPzKNdEGeQdEttZTAxWXJwMVdWeGxjbDgxMExYSEE#gid=0) or the perf-o-matic results (http://webkit-perf.appspot.com/index.html).
Comment 8 Erik Arvidsson 2012-02-24 15:28:24 PST
I don't think this would do much but it is clearly doing more work than we need to do.