Conversation
due performance issues - trying to end up with lower precision
as scale adjusting (10x) is performed by Java's constructor ... and since we're faster -> remove second precision guess
enebo
left a comment
There was a problem hiding this comment.
I see some delta changes. I am ok with this but I was wondering if you envision any changes in various test suites from this?
|
that the default delta is 0.01 or so - thus added an explicit delta to make those asserts meaningful |
|
@kares I just meant will we start seeing new errors from test suites like Rails which is assuming a particular rounding or something like that. I am ok with that but I just am wondering what potential fallout is possible if we are not rounding exactly the same as we were. |
|
we're not rounding exactly but weren't doing so before anyway. we do not match MRI with division, although have gotten better in 9.2 (compared to <= 9.1) and have some asserts guarding against regressions in terms of precision. so we should be doing the same (pbly even better regarding precision) as before. |
|
@kares thanks. lgtm! |
in certain cases its really slow e.g. a
x / 2as discovered from #5650here the
"/2 dec"case is very slow - on JRuby 9.2.6 taking 400s, while MRI takes 10s :... with this patch JRuby gets close to MRI's performance (over 30x improvement for the above / 2 case) :
Should be noted that only division (backed by
java.math.BigDecimal#divide(another, context)) seemed slow, other decimal operations are either faster on on par with MRI. The 'new' division algorithm is based on Android's sources (ASF2 license) with a few tweaks and a major (in terms of performance) change of not down scaling exact results (remainder 0). Leaving that off tojava.math.BigDecimal(BigInteger, int, MathContext)which will reduce the scale- n(doingBigInteger div 10^n) faster than we can using public API.