On 09/03/2011 12:06 AM, [email protected] 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
