Bug 58994 - JSString::resolveRope inefficient for common 2 fiber case
Summary: JSString::resolveRope inefficient for common 2 fiber case
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Michael Saboff
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-04-20 08:38 PDT by Michael Saboff
Modified: 2011-04-25 21:36 PDT (History)
0 users

See Also:


Attachments
Patch that splits resolveRope into common and slow cases (5.53 KB, patch)
2011-04-20 09:07 PDT, Michael Saboff
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Saboff 2011-04-20 08:38:05 PDT
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.
Comment 1 Michael Saboff 2011-04-20 09:07:53 PDT
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.
Comment 2 Geoffrey Garen 2011-04-20 13:24:01 PDT
Comment on attachment 90346 [details]
Patch that splits resolveRope into common and slow cases

What does SunSpider say about this change?
Comment 3 Michael Saboff 2011-04-20 22:49:34 PDT
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%
Comment 4 Michael Saboff 2011-04-21 10:23:20 PDT
Committed r84515: <http://trac.webkit.org/changeset/84515>
Comment 5 Eric Seidel (no email) 2011-04-25 21:36:19 PDT
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).