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

Adam Barth

Reported 2012-11-05 16:47:55 PST

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

Attachments
patch (V8 only at the moment) (1.26 KB, patch) 2012-11-05 16:49 PST, Adam Barth	no flags	Details Formatted Diff Diff
PerformanceTest results (first three are without patch; last three are with patch) (27.76 KB, text/html) 2012-11-05 16:51 PST, Adam Barth	no flags	Details
JSC PerformanceTest results (first three are without patch; last three are with) (28.21 KB, text/html) 2012-11-06 09:44 PST, Adam Barth	no flags	Details
Patch (3.10 KB, patch) 2012-11-06 11:08 PST, Adam Barth	no flags	Details Formatted Diff Diff
Patch (3.10 KB, patch) 2012-11-06 11:10 PST, Adam Barth	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Adam Barth

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

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

Adam Barth

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

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

Eric Seidel (no email)

Comment 3 2012-11-05 16:52:44 PST

Your patch seems sparse. :)

Adam Barth

Comment 4 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. :)

Erik Arvidsson

Comment 5 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?

Adam Barth

Comment 6 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.

Adam Barth

Comment 7 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.

Adam Barth

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

Created attachment 172620 [details] Patch

Erik Arvidsson

Comment 9 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.

Adam Barth

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

Created attachment 172622 [details] Patch

Adam Barth

Comment 11 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.

Eric Seidel (no email)

Comment 12 2012-11-06 11:37:51 PST

Comment on attachment 172622 [details] Patch Now the fun patches start. :)

WebKit Review Bot

Comment 13 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>

WebKit Review Bot

Comment 14 2012-11-06 12:26:22 PST

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.