<< Back to previous view

[MTOWER-3] BigDecimal: unnecessary reflection used in math functions for coercion (bigint) Created: 13/May/13  Updated: 02/Jan/14  Resolved: 02/Jan/14

Status: Closed
Project: math.numeric-tower
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Jürgen Hötzel Assignee: Mark Engelberg
Resolution: Declined Votes: 0
Labels: patch,

Attachments: Text File 0001-Use-coercion-and-rounding-methods-provided-by-BigDec.patch    
Patch: Code

 Description   

This improves performance, because no reflection has to be used (bigint):

Patched:
clojure.math.numeric-tower> (time (let [b (BigDecimal. "2.2")] (dotimes [x 10000000] (round b))))
"Elapsed time: 1640.177041 msecs"
Orig:
clojure.math.numeric-tower> (time (let [b (BigDecimal. "2.2")] (dotimes [x 10000000] (round b))))
"Elapsed time: 3498.339271 msecs"



 Comments   
Comment by Mark Engelberg [ 02/Jan/14 12:36 AM ]

Andy, thanks for letting me know about this issue. I had no idea it's been sitting here in JIRA for months.

1. These numbers really do need to be converted to BigInt, not BigInteger.

2. There's nothing inherently problematic with the call to bigint. In my tests, it appears that this patch doesn't meaningfully improve the performance of floor or ceiling, only round. That's because it's not the call to bigint that is slow, but in round, you've used a call to .setScale with ROUND_HALF_UP, and that's faster than the method used in the code (adding 0.5M and then flooring).

3. Even though .setScale with ROUND_HALF_UP is faster than the current code, it does not return the correct results for negative numbers like -4.5M. The goal is to make the behavior match Math/round on doubles. For some reason, BigDecimal's ROUND_HALF_UP does the wrong thing for negative numbers (rounding up in magnitude, rather than rounding up in the sense of making it more positive).

So this patch is a no-go. The changes to floor and ceiling return BigInteger rather than BigInt and when modified to return BigInt, do not yield speed benefits. The change to round does yield a speed benefit, but the results are incorrect for negative numbers halfway between integers.

If you find a faster way to round decimal numbers than the existing mechanism that yields the correct results for negative numbers, let me know.

Generated at Fri Oct 24 04:41:05 CDT 2014 using JIRA 4.4#649-r158309.