Bug 21200 – Allow direct access to 'arguments' without using op_resolve

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Cameron Zwarich (cpst)]

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.

[Cameron Zwarich (cpst)]

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)]

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 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]

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]

Oops, that comment was meant for but 21203.

[Cameron Zwarich (cpst)]

Landed in r37050.