Bug 44037 - JSC: Move the static_cast into to(U)Int32 fast case
Summary: JSC: Move the static_cast into to(U)Int32 fast case
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords: Performance
Depends on:
Blocks:
 
Reported: 2010-08-15 13:48 PDT by Andreas Kling
Modified: 2010-08-19 14:04 PDT (History)
3 users (show)

See Also:


Attachments
Proposed patch (3.64 KB, patch)
2010-08-15 13:53 PDT, Andreas Kling
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andreas Kling 2010-08-15 13:48:00 PDT
Converting a double JSValue to an int32 (or uint32) is fairly expensive since it includes the overhead of calling to(U)Int32SlowCase.

We could move the ideal case (static_cast<(u)int32_t> of double value within range) out of the slow case for a performance boost on conversion-heavy pages such as http://www.bel.fi/~alankila/plasma.html
Comment 1 Andreas Kling 2010-08-15 13:53:37 PDT
Created attachment 64453 [details]
Proposed patch
Comment 2 Geoffrey Garen 2010-08-18 10:07:36 PDT
Comment on attachment 64453 [details]
Proposed patch

For changes that are likely to affect performance, we usually require a performance measurement.

Please at least post SunSpider results with and without this patch, though I'd also be interested to see measurements of the website you mentioned.
Comment 3 Andreas Kling 2010-08-18 14:25:01 PDT
(In reply to comment #2)
> (From update of attachment 64453 [details])
> Please at least post SunSpider results with and without this patch, though I'd also be interested to see measurements of the website you mentioned.

Callgrind says: SunSpider is an amazing 0.0000072283% faster with this change.

Results for the linked page are pretty much uninteresting until something is done about bug 43742.
Comment 4 Andreas Kling 2010-08-19 12:01:34 PDT
Comment on attachment 64453 [details]
Proposed patch

With the fix for 43742 landed, this patch yields a 1.6% speedup for the plasma demo - http://www.bel.fi/~alankila/plasma.html
Comment 5 Geoffrey Garen 2010-08-19 12:08:04 PDT
Comment on attachment 64453 [details]
Proposed patch

r=me
Comment 6 WebKit Commit Bot 2010-08-19 14:04:44 PDT
Comment on attachment 64453 [details]
Proposed patch

Clearing flags on attachment: 64453

Committed r65698: <http://trac.webkit.org/changeset/65698>
Comment 7 WebKit Commit Bot 2010-08-19 14:04:49 PDT
All reviewed patches have been landed.  Closing bug.