RESOLVED FIXED Bug 21200
Allow direct access to 'arguments' without using op_resolve
https://bugs.webkit.org/show_bug.cgi?id=21200
Summary Allow direct access to 'arguments' without using op_resolve
Cameron Zwarich (cpst)
Reported 2008-09-28 11:56:38 PDT
Currently, we access 'arguments' via op_resolve on the JSActivation. This is a decent performance hit on the V8 Raytrace benchmark. I have a patch to put the arguments object in the callframe and access it directly. This does mean using an extra callframe slot, but a slot will be freed up after fixing bug 21123. Hopefully it won't be a performance regression in the meantime. I have a few more little things to fix up with my patch and then I'll post it.
Attachments
Patch in progress (33.43 KB, patch)
2008-09-28 14:30 PDT, Cameron Zwarich (cpst)
no flags
Proposed patch (33.68 KB, patch)
2008-09-28 19:14 PDT, Cameron Zwarich (cpst)
mjs: review+
Cameron Zwarich (cpst)
Comment 1 2008-09-28 14:30:31 PDT
Created attachment 23897 [details] Patch in progress Here is a patch in progress. I only perf tested non-CTI, and while SunSpider is fine, the V8 results are bizarre: TEST COMPARISON FROM TO DETAILS ============================================================================= ** TOTAL **: *1.029x as slow* 5983.8ms +/- 0.2% 6156.1ms +/- 0.1% significant ============================================================================= v8: *1.029x as slow* 5983.8ms +/- 0.2% 6156.1ms +/- 0.1% significant crypto: *1.114x as slow* 999.4ms +/- 0.1% 1113.1ms +/- 0.1% significant deltablue: *1.023x as slow* 1766.6ms +/- 0.1% 1807.7ms +/- 0.1% significant earley-boyer: *1.013x as slow* 678.5ms +/- 0.1% 687.2ms +/- 0.1% significant raytrace: 1.030x as fast 657.6ms +/- 0.1% 638.8ms +/- 0.1% significant richards: *1.015x as slow* 1881.7ms +/- 0.6% 1909.4ms +/- 0.1% significant
Cameron Zwarich (cpst)
Comment 2 2008-09-28 19:14:45 PDT
Created attachment 23899 [details] Proposed patch Maciej says he does not see any of the bizarre regressions that I do, so he is ready to review this.
Maciej Stachowiak
Comment 3 2008-09-28 19:45:35 PDT
Comment on attachment 23899 [details] Proposed patch r=me Some optional comments: - consider renaming create_arguments to init_arguments (perhaps init should be renamed to enter) - in the parser, instead of comparing to "arguments" consider comparing to GOBAL_DATA->propertyNames.arguments()
Maciej Stachowiak
Comment 4 2008-09-28 19:59:07 PDT
I sadly duplicated the logic for UString::from(int) and UString::from(double) though fortunately not the actual append logic. Perhaps I should try harder to refactor to make it ot duplicat, though it was not 100% clear how to do this except maybe using a template. UString::from(int) is not much code but the double case is nontrivial.
Maciej Stachowiak
Comment 5 2008-09-28 20:00:56 PDT
Oops, that comment was meant for but 21203.
Cameron Zwarich (cpst)
Comment 6 2008-09-28 20:04:34 PDT
Landed in r37050.
Note You need to log in before you can comment on or make changes to this bug.