Bug 101279

Summary:

DOMImplementation should use ScriptWrappable

Product:

WebKit

Reporter:

Adam Barth <abarth>

Component:

WebCore JavaScript

Assignee:

Adam Barth <abarth>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

arv, eric, haraken, ktf.kim, ojan, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

101110, 101319

Bug Blocks:

Attachments:

Description	Flags
patch (V8 only at the moment)	none
PerformanceTest results (first three are without patch; last three are with patch)	none
JSC PerformanceTest results (first three are without patch; last three are with)	none
Patch	none
Patch	none

Description Adam Barth 2012-11-05 16:47:55 PST

Using ScriptWrappable for DOMImplementation makes document.implementation about 23% faster.

Comment 1 Adam Barth 2012-11-05 16:49:52 PST

Created attachment 172439 [details]
patch (V8 only at the moment)

Comment 2 Adam Barth 2012-11-05 16:51:37 PST

Created attachment 172440 [details]
PerformanceTest results (first three are without patch; last three are with patch)

Comment 3 Eric Seidel (no email) 2012-11-05 16:52:44 PST

Your patch seems sparse. :)

Comment 4 Adam Barth 2012-11-05 17:11:15 PST

> Your patch seems sparse. :)

Do you mean no ChangeLog?  I still need to get the JSC side working.  :)

Comment 5 Erik Arvidsson 2012-11-05 19:08:15 PST

It does look a little bit thin. Didn't you have to do anything to the code gen?

Comment 6 Adam Barth 2012-11-05 19:39:16 PST

(In reply to comment #5)
> It does look a little bit thin. Didn't you have to do anything to the code gen?

Nope.

Comment 7 Adam Barth 2012-11-06 09:44:33 PST

Created attachment 172605 [details]
JSC PerformanceTest results (first three are without patch; last three are with)

In the JSC bindings, this patch is a 69% improvement on Bindings/document-implementation.

Comment 8 Adam Barth 2012-11-06 11:08:16 PST

Created attachment 172620 [details]
Patch

Comment 9 Erik Arvidsson 2012-11-06 11:09:20 PST

Comment on attachment 172620 [details]
Patch

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

> PerformanceTests/Bindings/document-implementation.html:7
> +    description: "This benchmark repeatedly accesses properties of range.",

Please update description.

Comment 10 Adam Barth 2012-11-06 11:10:51 PST

Created attachment 172622 [details]
Patch

Comment 11 Adam Barth 2012-11-06 11:11:02 PST

(In reply to comment #9)
> (From update of attachment 172620 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=172620&action=review
> 
> > PerformanceTests/Bindings/document-implementation.html:7
> > +    description: "This benchmark repeatedly accesses properties of range.",
> 
> Please update description.

Done.

Comment 12 Eric Seidel (no email) 2012-11-06 11:37:51 PST

Comment on attachment 172622 [details]
Patch

Now the fun patches start. :)

Comment 13 WebKit Review Bot 2012-11-06 12:26:18 PST

Comment on attachment 172622 [details]
Patch

Clearing flags on attachment: 172622

Committed r133657: <http://trac.webkit.org/changeset/133657>

Comment 14 WebKit Review Bot 2012-11-06 12:26:22 PST

All reviewed patches have been landed.  Closing bug.