Closed
Bug 779689
Opened 12 years ago
Closed 7 years ago
IonMonkey: Improve powi() performance.
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
INVALID
People
(Reporter: sstangl, Unassigned)
References
Details
(Whiteboard: [ion:t])
Attachments
(2 files)
1.89 KB,
patch
|
Details | Diff | Splinter Review | |
2.06 KB,
patch
|
Details | Diff | Splinter 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)
Reporter | ||
Comment 1•12 years ago
|
||
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
Reporter | ||
Updated•12 years ago
|
Attachment #648152 -
Flags: review?(dvander)
Comment 2•12 years ago
|
||
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
Comment 3•12 years ago
|
||
Evidently attaching a patch removes the status changes
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
Updated•12 years ago
|
Whiteboard: [ion:t]
Comment 4•12 years ago
|
||
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 | ||
Updated•10 years ago
|
Assignee: general → nobody
Reporter | ||
Comment 5•7 years ago
|
||
Closing old bug.
Status: REOPENED → RESOLVED
Closed: 12 years ago → 7 years ago
Resolution: --- → INVALID
You need to log in
before you can comment on or make changes to this bug.
Description
•