Bug 78195

Summary: [JSC] Cache the number of non-custom constructor arguments
Product: WebKit Reporter: Kentaro Hara <haraken>
Component: WebCore JavaScriptAssignee: Kentaro Hara <haraken>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, darin, japhet, sam, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 77393    
Attachments:
Description Flags
Patch
none
Patch
none
patch for commit none

Kentaro Hara
Reported 2012-02-08 18:56:19 PST
If [ConstructorParameters=] is specified, JSC caches the number of constructor arguments for performance. However, at present, [ConstructorParameters=] is specified on a small part of custom constructors (It appears that people have forgotten to add [ConstructorParameters=]). Thus, we can improve JSC by the two changes: [1] For non-custom constructors, CodeGeneratorJS.pm should cache the number of constructor arguments automatically without [ConstructorParameters=] (CodeGeneratorJS.pm can know the number of arguments by the [Constructor=...] signature). [2] For custom constructors, we should add [ConstructorParameters=] to their IDL files. (Or, maybe we can completely remove [ConstructorParameters=] if the caching is not so important for practical performance. I am afraid that people will forget to add [ConstructorParameters=] in the future. Anyway let's discuss the topic in another bug.) In this bug, we make a change for [1].
Attachments
Patch (3.51 KB, patch)
2012-02-08 18:59 PST, Kentaro Hara 		no flags
DetailsFormatted DiffDiff
Patch (11.99 KB, patch)
2012-02-14 18:26 PST, Kentaro Hara 		no flags
DetailsFormatted DiffDiff
patch for commit (11.98 KB, patch)
2012-02-14 18:40 PST, Kentaro Hara 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Kentaro Hara
Comment 1 2012-02-08 18:59:52 PST
Created attachment 126221 [details] Patch
Darin Adler
Comment 2 2012-02-13 16:07:28 PST
Comment on attachment 126221 [details] Patch Looks good. I’d like to see some test covering this, though. It’s not great to land a fix like this without a test.
Kentaro Hara
Comment 3 2012-02-14 18:26:20 PST
Created attachment 127096 [details] Patch
Kentaro Hara
Comment 4 2012-02-14 18:26:57 PST
(In reply to comment #2) > (From update of attachment 126221 [details]) > Looks good. I’d like to see some test covering this, though. It’s not great to land a fix like this without a test. Darin: I added the tests. Would you take another look? Thanks!
Darin Adler
Comment 5 2012-02-14 18:28:00 PST
Comment on attachment 127096 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=127096&action=review > LayoutTests/fast/js/constructor-length.html:8 > +description("This tests the length property of constructors."); Could we sort these alphabetically?
Kentaro Hara
Comment 6 2012-02-14 18:40:09 PST
Created attachment 127098 [details] patch for commit sorted tests alphabetically
WebKit Review Bot
Comment 7 2012-02-14 19:22:31 PST
Comment on attachment 127098 [details] patch for commit Clearing flags on attachment: 127098 Committed r107772: <http://trac.webkit.org/changeset/107772>
Note You need to log in before you can comment on or make changes to this bug.