Bug 101279 - DOMImplementation should use ScriptWrappable
Summary: DOMImplementation should use ScriptWrappable
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore JavaScript (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Adam Barth
URL:
Keywords:
Depends on: 101110 101319
Blocks:
  Show dependency tree / graph
 
Reported: 2012-11-05 16:47 PST by Adam Barth
Modified: 2014-01-28 02:26 PST (History)
6 users (show)

See Also:

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 an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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.