Bug 58994 – JSString::resolveRope inefficient for common 2 fiber case

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Michael Saboff]

JSString::resolveRope is a generic routine to convert ropes into simple strings.  It doesn't have an optimization for the common 2 fiber case.

A restructuring of common and slow cases is in order.  This would improve performance.  This was found while tuning for the dromaeo DOM benchmark.

[Michael Saboff]

Created attachment 90346 [details]
Patch that splits resolveRope into common and slow cases

Improves performance on dromaeo DOM-query benchmark.

Suspect this will generally improve performance.

Considered inlining the fast case, but felt the method was a little long for that.  Although there are only two call sites for resolveRope, both are in JSString.h in common methods that will be inlined themselves.  This could cause code bloat.

[Geoffrey Garen]

Comment on attachment 90346 [details]
Patch that splits resolveRope into common and slow cases

What does SunSpider say about this change?

[Michael Saboff]

SunSpider likes the change ;-)  It is ~.7% improvement overall

Noticeable improvements marked with <===

Test                     Without patch        With patch
Total                  201.4ms +/- 0.1%     199.9ms +/- 0.1%   
  3d                    28.2ms +/- 0.6%      28.1ms +/- 0.5%
    cube                10.0ms +/- 0.0%      10.0ms +/- 0.0%
    morph                8.1ms +/- 1.5%       8.1ms +/- 1.7%
    raytrace            10.1ms +/- 0.7%      10.0ms +/- 0.0%

  access                26.6ms +/- 0.5%      26.6ms +/- 0.5%
    binary-trees         2.5ms +/- 5.6%       2.6ms +/- 5.6%
    fannkuch            13.0ms +/- 0.0%      13.0ms +/- 0.3%
    nbody                7.0ms +/- 0.0%       7.0ms +/- 0.0%
    nsieve               4.0ms +/- 1.0%       4.0ms +/- 0.0%

  bitops                15.0ms +/- 0.0%      15.0ms +/- 0.0%
    3bit-bits-in-byte    2.0ms +/- 0.0%       2.0ms +/- 0.0%
    bits-in-byte         5.0ms +/- 0.0%       5.0ms +/- 0.0%
    bitwise-and          3.0ms +/- 0.0%       3.0ms +/- 0.0%
    nsieve-bits          5.0ms +/- 0.0%       5.0ms +/- 0.0%

  controlflow            2.0ms +/- 0.0%       2.0ms +/- 0.0%
    recursive            2.0ms +/- 0.0%       2.0ms +/- 0.0%

  crypto                12.5ms +/- 1.1%      12.1ms +/- 0.9%
    aes                  7.5ms +/- 1.9%       7.1ms +/- 1.5%  <===
    md5                  3.0ms +/- 0.0%       3.0ms +/- 0.0%
    sha1                 2.0ms +/- 0.0%       2.0ms +/- 0.0%

  date                  23.3ms +/- 0.6%      23.0ms +/- 0.0%
    format-tofte        13.3ms +/- 1.0%      13.0ms +/- 0.0%  <===
    format-xparb        18.2ms +/- 0.6%      18.1ms +/- 0.4%
    cordic               5.0ms +/- 0.8%       5.0ms +/- 0.8%
    partial-sums         9.2ms +/- 1.2%       9.0ms +/- 0.6%  <===
    spectral-norm        4.0ms +/- 0.0%       4.0ms +/- 0.0%

  regexp                12.0ms +/- 0.5%      11.8ms +/- 1.0%
    dna                 12.0ms +/- 0.5%      11.8ms +/- 1.0%  <===

  string                63.6ms +/- 0.3%      63.3ms +/- 0.2%
    base64               6.3ms +/- 2.1%       6.1ms +/- 1.3%  <===
    fasta                8.0ms +/- 0.0%       8.0ms +/- 0.0%
    tagcloud            16.2ms +/- 0.8%      16.2ms +/- 0.7%
    unpack-code         25.1ms +/- 0.3%      25.0ms +/- 0.2%
    validate-input       8.0ms +/- 0.0%       8.0ms +/- 0.0%

[Michael Saboff]

Committed r84515: <http://trac.webkit.org/changeset/84515>

[Eric Seidel (no email)]

Comment on attachment 90346 [details]
Patch that splits resolveRope into common and slow cases

Cleared review? from attachment 90346 [details] so that this bug does not appear in http://webkit.org/pending-review.  If you would like this patch reviewed, please attach it to a new bug (or re-open this bug before marking it for review again).