Bug 21364

Summary: Remove the branch in op_ret for OptionalCalleeActivation and OptionalCalleeArguments
Product: WebKit Reporter: Cameron Zwarich (cpst) <zwarich>
Component: JavaScriptCoreAssignee: Cameron Zwarich (cpst) <zwarich>
Status: RESOLVED FIXED    
Severity: Normal CC: mjs
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 20812    
Attachments:
Description Flags
Proposed patch to refactor some code in preparation for the speedup
none
Patch in progress
none
Proposed patch oliver: review+

Cameron Zwarich (cpst)
Reported 2008-10-04 00:36:13 PDT
Currently op_ret contains a branch for OptionalCalleeActivation and OptionalCalleeArguments, so it can tear either of the two off. Removing this branch is not that difficult, especially given recent changes to CodeFeatures in the parser, but it will require making f.arguments only live and shared in the case where 'arguments' appears lexically.
Attachments
Proposed patch to refactor some code in preparation for the speedup (5.06 KB, patch)
2008-10-05 15:34 PDT, Cameron Zwarich (cpst) 		no flags
DetailsFormatted DiffDiff
Patch in progress (18.30 KB, patch)
2008-10-05 18:39 PDT, Cameron Zwarich (cpst) 		no flags
DetailsFormatted DiffDiff
Proposed patch (25.26 KB, patch)
2008-10-05 22:31 PDT, Cameron Zwarich (cpst) 		oliver: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Cameron Zwarich (cpst)
Comment 1 2008-10-05 15:34:05 PDT
Created attachment 24104 [details] Proposed patch to refactor some code in preparation for the speedup
Maciej Stachowiak
Comment 2 2008-10-05 15:37:52 PDT
Comment on attachment 24104 [details] Proposed patch to refactor some code in preparation for the speedup r=me You may want to consider giving scopeChain a needsActivation method which checks for any of the needed conditions rather than having such a complex field initializer.
Cameron Zwarich (cpst)
Comment 3 2008-10-05 18:38:00 PDT
Comment on attachment 24104 [details] Proposed patch to refactor some code in preparation for the speedup Landed in r327320, so I am removing review flag.
Cameron Zwarich (cpst)
Comment 4 2008-10-05 18:39:02 PDT
Created attachment 24107 [details] Patch in progress This patch passes all tests, but it doesn't seem to be a speedup on my machine. I also need to add some tests for the changes in functionality.
Cameron Zwarich (cpst)
Comment 5 2008-10-05 22:31:23 PDT
Created attachment 24110 [details] Proposed patch This gives some odd test results on my machine, mostly due to a presumedly slowdown in Richards, but it is an overall progression on Maciej's machine, so he told me to put it up for review.
Oliver Hunt
Comment 6 2008-10-05 22:49:33 PDT
Comment on attachment 24110 [details] Proposed patch r=me assuming you've testing in cti and the interpreter
Cameron Zwarich (cpst)
Comment 7 2008-10-05 23:03:35 PDT
Landed in r37324.
Note You need to log in before you can comment on or make changes to this bug.