JDK 8 core review request for 6989067 BigInteger's array copiers should be converted to System.arraycopy() (original) (raw)

Rémi Forax forax at univ-mlv.fr
Fri Sep 2 23:25:29 UTC 2011

On 09/03/2011 01:19 AM, joe.darcy at oracle.com wrote:

Hi Remi.

Modified as suggested to use Arrays.copyOf before being pushed. The performance team recommend to me this class of change be implemented so I assume (and hope) the VM properly handles small arrays in a performance sense.

good to know :)

Thanks, -Joe

cheers, Rémi

On 9/2/2011 3:30 PM, Rémi Forax wrote: On 09/03/2011 12:06 AM, joe.darcy at oracle.com wrote:

Hello.

Please review this simple patch to replace explicit array copy loops with System.arraycopy: 6989067 BigInteger's array copiers should be converted to System.arraycopy() http://cr.openjdk.java.net/~darcy/6989067.0/ Patch below. Thanks, -Joe Hi Joe, for me, some occurence you can use Arrays.copyOf instead of System.arraycopy in order to avoid to fill (or partially fill part of) the array with zero. manual patch inlined below :)

--- old/src/share/classes/java/math/BigInteger.java 2011-09-02 14:48:34.000000000 -0700 +++ new/src/share/classes/java/math/BigInteger.java 2011-09-02 14:48:34.000000000 -0700 @@ -1612,14 +1612,12 @@ } else { // Array must be resized if (nBits <= (32-bitsInHighWord)) {_ _int result[] = new int[nInts+len];_ _- for (int i=0; i<len; i++)_ _- result[i] = a[i];_ _+ System.arraycopy(a, 0, result, 0, len);_ _primitiveLeftShift(result, result.length, nBits);_ _return result;_ _} else {_ _int result[] = new int[nInts+len+1];_ _- for (int i=0; i<len; i++)_ _- result[i] = a[i];_ _+ System.arraycopy(a, 0, result, 0, len);_ _primitiveRightShift(result, result.length, 32 - nBits);_ _return result;_ _}_ _@@ -1908,8 +1906,7 @@_ _// Set t to high half of b_ _int[] t = new int[modLen];_ _- for(int i=0; i<modLen; i++)_ _- t[i] = b[i];_ _+ System.arraycopy(b, 0, t, 0, modLen);_ _can be simplified to:_ _int[] t = Array.copyOf(b, modLen);_ _// Fill in the table with odd powers of the base_ _for (int i=1; i<tblmask; i++) {_ _@@ -2006,14 +2003,12 @@_ _// Convert result out of Montgomery form and return_ _int[] t2 = new int[2*modLen];_ _- for(int i=0; i<modLen; i++)_ _- t2[i+modLen] = b[i];_ _+ System.arraycopy(b, 0, t2, modLen, modLen);_ _b = montReduce(t2, mod, modLen, inv);_ _t2 = new int[modLen];_ _- for(int i=0; i<modLen; i++)_ _- t2[i] = b[i];_ _+ System.arraycopy(b, 0, t2, 0, modLen);_ _can be simplified to:_ _t2 = Arrays.copyOf(b, modLen);_ _return new BigInteger(1, t2);_ _}_ _@@ -2154,8 +2149,7 @@_ _// Copy remaining ints of mag_ _int numInts = (p + 31) >>> 5; int[] mag = new int[numInts]; - for (int i=0; i<numInts; i++)_ _- mag[i] = this.mag[i + (this.mag.length - numInts)];_ _+ System.arraycopy(this.mag, (this.mag.length - numInts),_ _mag, 0, numInts);_ _// Mask out any excess bits_ _int excessBits = (numInts << 5) - p;_ _@@ -2221,7 +2215,7 @@_ _return shiftRight(-n);_ _}_ _}_ _- int[] newMag = shiftLeft(mag,n);_ _+ int[] newMag = shiftLeft(mag, n);_ _return new BigInteger(newMag, signum);_ _}_ _@@ -2234,8 +2228,7 @@_ _if (nBits == 0) {_ _newMag = new int[magLen + nInts];_ _- for (int i=0; i<magLen; i++)_ _- newMag[i] = mag[i];_ _+ System.arraycopy(mag, 0, newMag, 0, magLen);_ _newMag = Arrays.copyOf(magLen + nInts);_ _} else {_ _int i = 0;_ _int nBits2 = 32 - nBits;_ _@@ -2289,8 +2282,7 @@_ _if (nBits == 0) {_ _int newMagLen = magLen - nInts;_ _newMag = new int[newMagLen];_ _- for (int i=0; i<newMagLen; i++)_ _- newMag[i] = mag[i];_ _+ System.arraycopy(mag, 0, newMag, 0, newMagLen);_ _newMag = Arrays.copyOf(newMagLen);_ _} else {_ _int i = 0;_ _int highBits = mag[0] >>> nBits; @@ -2561,7 +2553,7 @@ if (signum < 0) { // Check if magnitude is a power of two boolean pow2 = (Integer.bitCount(mag[0]) == 1); - for(int i=1; i< len && pow2; i++) + for (int i=1; i< len && pow2; i++) pow2 = (mag[i] == 0); n = (pow2 ? magBitLength -1 : magBitLength); Also, this change can have a negative impact because as far as I know system.arraycopy/Arrays.copyOf uses a loop even if the number of iteration is really small. A for loop will be unrolled by the VM. regards, Rémi

More information about the core-libs-dev mailing list