RFR 8057793 BigDecimal is no longer effectively immutable
Joe Darcy
joe.darcy at oracle.com
Sat Sep 13 17:10:08 UTC 2014
Hi Brian,
Other than removing the
// bug 8057793
comment on the new test method, this looks good to be pushed.
Thanks,
-Joe
On 9/13/2014 7:56 AM, Brian Burkhalter wrote:
> I forgot to add setScaleDoesNotMutateTest() to main() in ZeroScalingTests. D’oh! Here’s the corrected webrev:
>
> http://cr.openjdk.java.net/~bpb/8057793/webrev.01/
>
> Thanks,
>
> Brian
>
> On Sep 12, 2014, at 4:54 PM, Brian Burkhalter <brian.burkhalter at oracle.com> wrote:
>
>> Hello,
>>
>> I created a formal webrev:
>>
>> Issue: https://bugs.openjdk.java.net/browse/JDK-8057793
>> Webrev: http://cr.openjdk.java.net/~bpb/8057793/webrev.00/
>>
>> Based on manual inspection of the revised code the patch looks good to me. The test submitted with the issue now succeeds as do all regression tests in jdk/test/java/math to which I also added the code from the test case in the issue report.
>>
>> Note that this webrev is with respect to JDK 9.
>>
>> Thanks,
>>
>> Brian
>>
>> On Sep 11, 2014, at 6:35 PM, Joe Darcy <joe.darcy at oracle.com> wrote:
>>
>>> Hello,
>>>
>>> Hmmm. I haven't dived into the details of the code, but setScale calls out to divide functionality so it is plausible a bug in divide could cause a problem in setScale.
>>>
>>> Thanks for the bug report,
>>>
>>> -Joe
>>>
>>> On 9/9/2014 1:30 AM, Robert Gibson wrote:
>>>>
>>>> Hi there,
>>>>
>>>> I came across a case in BigDecimal division where the dividend ends up getting mutated, which is rather strange for a seemingly immutable class! (It's a subset of the cases where the Burnikel-Ziegler algorithm is used, I haven't done the analysis to find out under which exact conditions it's triggered.)
>>>>
>>>> The attached patch - against the JDK8 version - should fix the problem, at the cost of an extra array copy. Could somebody review and/or comment please?
>>>>
>>>> Thanks,
>>>> Robert
>>>>
>>>> --- MutableBigInteger.java 2014-09-04 09:42:23.426815000 +0200
>>>> +++ MutableBigInteger.java.patched 2014-09-04 09:46:21.344132000 +0200
>>>> @@ -1261,19 +1261,20 @@
>>>> int sigma = (int) Math.max(0, n32 - b.bitLength()); // step 3: sigma = max{T | (2^T)*B < beta^n}
>>>> MutableBigInteger bShifted = new MutableBigInteger(b);
>>>> bShifted.safeLeftShift(sigma); // step 4a: shift b so its length is a multiple of n
>>>> - safeLeftShift(sigma); // step 4b: shift this by the same amount
>>>> + MutableBigInteger aShifted = new MutableBigInteger (this);
>>>> + aShifted.safeLeftShift(sigma); // step 4b: shift a by the same amount
>>>> - // step 5: t is the number of blocks needed to accommodate this plus one additional bit
>>>> - int t = (int) ((bitLength()+n32) / n32);
>>>> + // step 5: t is the number of blocks needed to accommodate a plus one additional bit
>>>> + int t = (int) ((aShifted.bitLength()+n32) / n32);
>>>> if (t < 2) {
>>>> t = 2;
>>>> }
>>>> - // step 6: conceptually split this into blocks a[t-1], ..., a[0]
>>>> - MutableBigInteger a1 = getBlock(t-1, t, n); // the most significant block of this
>>>> + // step 6: conceptually split a into blocks a[t-1], ..., a[0]
>>>> + MutableBigInteger a1 = aShifted.getBlock(t-1, t, n); // the most significant block of a
>>>> // step 7: z[t-2] = [a[t-1], a[t-2]]
>>>> - MutableBigInteger z = getBlock(t-2, t, n); // the second to most significant block
>>>> + MutableBigInteger z = aShifted.getBlock(t-2, t, n); // the second to most significant block
>>>> z.addDisjoint(a1, n); // z[t-2]
>>>> // do schoolbook division on blocks, dividing 2-block numbers by 1-block numbers
>>>> @@ -1284,7 +1285,7 @@
>>>> ri = z.divide2n1n(bShifted, qi);
>>>> // step 8b: z = [ri, a[i-1]]
>>>> - z = getBlock(i-1, t, n); // a[i-1]
>>>> + z = aShifted.getBlock(i-1, t, n); // a[i-1]
>>>> z.addDisjoint(ri, n);
>>>> quotient.addShifted(qi, i*n); // update q (part of step 9)
>>>> }
>>>> @@ -1292,7 +1293,7 @@
>>>> ri = z.divide2n1n(bShifted, qi);
>>>> quotient.add(qi);
>>>> - ri.rightShift(sigma); // step 9: this and b were shifted, so shift back
>>>> + ri.rightShift(sigma); // step 9: a and b were shifted, so shift back
>>>> return ri;
>>>> }
>>>> }
More information about the core-libs-dev
mailing list