JDK 9 RFR of JDK-8035279: Clean up internal deprecations in BigInteger

[Paul Sandoz]

On Feb 20, 2014, at 9:13 AM, Alan Bateman <Alan.Bateman at oracle.com> wrote:

> On 19/02/2014 21:44, Brian Burkhalter wrote:
>> This patch concerns cleaning up some ugly internal deprecations.
>> 
>> Issue:	https://bugs.openjdk.java.net/browse/JDK-8035279
>> Webrev:	http://cr.openjdk.java.net/~bpb/8035279/webrev.00/
>> 
>> All JTREG BigInteger tests pass, and the serialized form has been unaltered as verified by bidirectional read-write testing between Java 7 and this version.
>> 
>> I would appreciate scrutiny of the arithmetic to make sure that I've made no silly errors. Also I would be interested in opinions as to whether the "volatile" keyword should in fact be used for the affected instance variables.
>> 
> It's not clear (to me) that these should be volatile. I also wonder about setting the initial value too as volatile could make this expensive in some environments.
> 

Yes, IIUC only atomic access is required. Here be dragons!


Not sure the static powerCache field, in the original code, needs to be volatile either:

1137     private static volatile BigInteger[][] powerCache;


If the recently added volatile on the fields are removed the setting of them for deserialization should be moved above the putMag call  to preserve construction semantics (although see Alan's point on serialization compat):
 
4272         // Calculate mag field from magnitude and discard magnitude
4273         UnsafeHolder.putMag(this, mag);
4274         if (mag.length >= MAX_MAG_LENGTH) {
4275             try {
4276                 checkRange();
4277             } catch (ArithmeticException e) {
4278                 throw new java.io.StreamCorruptedException("BigInteger: Out of the supported range");
4279             }
4280         }
4281 
4282         // Initialize cached fields to indicate not computed yet.
4283         bitCount = Integer.MIN_VALUE;
4284         bitLength = Integer.MIN_VALUE;
4285         lowestSetBit = Integer.MIN_VALUE;
4286         firstNonzeroIntNum = Integer.MIN_VALUE;


In fact the UnsafeHolder.putSign impl could be changed to use putObject rather than putObjectVolatile, since we just need to stamp a fence in after all fields have been set in readObject (alternatively Unsafe.fullFence could be used as the last call before readObject returns).

Paul.

> Are you sure there isn't any serialization impact? Zeros has also meant these are uninitialized so I just wonder about deserializing on an older JDK.
> 
> -Alan.
>