Currently JavaScriptCore caches results for Math.sin, as introduced by the patch for bug #38714. I think it makes sense to do the same for Math.cos, since the same rationale used for Math.sin applies to Math.cos and also because these two functions are typically used together, for example to compute rotation transformations. I'll upload a patch shortly.
Created attachment 215035 [details] Adds result caching for Math.cos A quick test consisting of averaging 5 consecutive runs of Sunspider shows a 1.9% benefit in my case.
Comment on attachment 215035 [details] Adds result caching for Math.cos r=me
Comment on attachment 215035 [details] Adds result caching for Math.cos Clearing flags on attachment: 215035 Committed r158281: <http://trac.webkit.org/changeset/158281>
All reviewed patches have been landed. Closing bug.
Was this based on any benchmarking? If so, why aren't the benchmarks part of the patch?
(In reply to comment #5) > Was this based on any benchmarking? If so, why aren't the benchmarks part of the patch? I ran the sunspider benchmark to assess performance improvement, my tests showed a 1.9% performance improvement on this benchmark as I mention in comment #1. I did not know there was a policy to include the benchmark results in the ChangeLog as part of the patch, so that was my bad :(
(In reply to comment #1) > Created an attachment (id=215035) [details] > Adds result caching for Math.cos > > A quick test consisting of averaging 5 consecutive runs of Sunspider shows a 1.9% benefit in my case. It's customary to post full performance results to bugzilla for optimizations like these. It's customary to run more than just SunSpider. It's unwise to rely on a single benchmark for optimizations. For example, Octane, JSBench, and Kraken are also important benchmarks.
Anyway, see: https://bugs.webkit.org/show_bug.cgi?id=123574
(In reply to comment #7) > (In reply to comment #1) > > Created an attachment (id=215035) [details] [details] > > Adds result caching for Math.cos > > > > A quick test consisting of averaging 5 consecutive runs of Sunspider shows a 1.9% benefit in my case. > > It's customary to post full performance results to bugzilla for optimizations like these. > > It's customary to run more than just SunSpider. It's unwise to rely on a single benchmark for optimizations. For example, Octane, JSBench, and Kraken are also important benchmarks. Also, I cannot reproduce the speed-up that you're reporting. I'm seeing a 0.35% difference before and after your patch and that difference is in the noise.