Closed Bug 779689 Opened 12 years ago Closed 7 years ago

IonMonkey: Improve powi() performance.

Categories

(Core :: JavaScript Engine, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED INVALID

People

(Reporter: sstangl, Unassigned)

References

Details

(Whiteboard: [ion:t])

Attachments

(2 files)

Attached patch patchSplinter Review
The modified partial-sums benchmark posted in Bug 777564 spends about 40% of its time in powi(). The attached patch re-expresses the main loop in a clearer way, with a terminal case moved far away from the loop body, and with a single multiplication occasionally saved. Runtime on the benchmark drops from ~727ms to ~702ms on x86_64.
Attachment #648152 - Flags: review?(dvander)
Although the performance results above were consistently reproducible about 20 minutes ago, testing now with the exact same binaries as before shows no perf difference. I don't know, but I wouldn't fritz with things unless the perf boost is guaranteed. Resolving invalid.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → INVALID
Attachment #648152 - Flags: review?(dvander)
After looking at this a bit, I decided that the branch in the middle of the loop was up to no good!
getting rid of it (and replacing it with an array access of all things!) caused this microbenchmark
    double d =0;
    for (int i = 0; i < 100000000; i++) {
        d+=pow(1.0000001, i);
    }
    fprintf(stderr, "%f", d); 
to go from 11 seconds (with sstangl's patched code)
to 3.9 seconds.  I'll try with the actual js shell in a bit
Evidently attaching a patch removes the status changes
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
Oh yeah, I tried this out on ARM, and as expected, Sean's version is much faster than mine (memory accesses are painful on ARM).  I'll get a version that uses some preprocessor magic for the two variants (maybe also uses the non-array variant on older x86 chips), and get it up for review.
Assignee: general → nobody
Closing old bug.
Status: REOPENED → RESOLVED
Closed: 12 years ago7 years ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: