-
Bug
-
Resolution: Won't Fix
-
P4
-
None
-
7
-
generic
-
generic
Josh writes:
Stupidity: The signature is Arrays.sort(Object[]), not Arrays.sort(Comparable[])
Casting (in the inner loop, no less):
private static void mergeSort(Object[] src,
Object[] dest,
int low,
int high,
int off) {
int length = high - low;
// Insertion sort on smallest arrays
if (length < INSERTIONSORT_THRESHOLD) {
for (int i=low; i<high; i++)
for (int j=i; j>low &&
((Comparable) dest[j-1]).compareTo(dest[j])>0; j--)
swap(dest, j, j-1);
return;
}
// Recursively sort halves of dest into src
int destLow = low;
int destHigh = high;
low += off;
high += off;
int mid = (low + high) >>> 1;
mergeSort(dest, src, low, mid, -off);
mergeSort(dest, src, mid, high, -off);
// If list is already sorted, just copy from src to dest. This is an
// optimization that results in faster sorts for nearly ordered lists.
if (((Comparable)src[mid-1]).compareTo(src[mid]) <= 0) {
System.arraycopy(src, low, dest, destLow, length);
return;
}
// Merge sorted halves (now in src) into dest
for(int i = destLow, p = low, q = mid; i < destHigh; i++) {
if (q >= high || p < mid && ((Comparable)src[p]).compareTo(src[q])<=0)
dest[i] = src[p++];
else
dest[i] = src[q++];
}
}
Performance: can be improved substantially by doing this:
public static void sort(Object[] a) {
if (a instanceof Comparable[]) {
Comparable[] aux = (Comparable[])a.clone();
mergeSort2(aux, (Comparable[])a, 0, a.length, 0);
} else {
Object[] aux = (Object[])a.clone();
mergeSort(aux, a, 0, a.length, 0);
}
}
As a practical matter, you always take the first branch of the if-statement. Of course you have to duplicate the above code with the argument types changed from Object[] to Comparable[], but you get a 9.7% improvement in sorting speed. To me, this is a no-brainer.
Josh adds:
One other thing: Collections.sort dumps the List into an Object array prior to sorting (even though it *knows* that the elements are all Comparable). In order to make this optimization work for Collections.sort, it should dump the Collection into an array of Comparable, and (what the heck) directly invoke mergesort2.
Stupidity: The signature is Arrays.sort(Object[]), not Arrays.sort(Comparable[])
Casting (in the inner loop, no less):
private static void mergeSort(Object[] src,
Object[] dest,
int low,
int high,
int off) {
int length = high - low;
// Insertion sort on smallest arrays
if (length < INSERTIONSORT_THRESHOLD) {
for (int i=low; i<high; i++)
for (int j=i; j>low &&
((Comparable) dest[j-1]).compareTo(dest[j])>0; j--)
swap(dest, j, j-1);
return;
}
// Recursively sort halves of dest into src
int destLow = low;
int destHigh = high;
low += off;
high += off;
int mid = (low + high) >>> 1;
mergeSort(dest, src, low, mid, -off);
mergeSort(dest, src, mid, high, -off);
// If list is already sorted, just copy from src to dest. This is an
// optimization that results in faster sorts for nearly ordered lists.
if (((Comparable)src[mid-1]).compareTo(src[mid]) <= 0) {
System.arraycopy(src, low, dest, destLow, length);
return;
}
// Merge sorted halves (now in src) into dest
for(int i = destLow, p = low, q = mid; i < destHigh; i++) {
if (q >= high || p < mid && ((Comparable)src[p]).compareTo(src[q])<=0)
dest[i] = src[p++];
else
dest[i] = src[q++];
}
}
Performance: can be improved substantially by doing this:
public static void sort(Object[] a) {
if (a instanceof Comparable[]) {
Comparable[] aux = (Comparable[])a.clone();
mergeSort2(aux, (Comparable[])a, 0, a.length, 0);
} else {
Object[] aux = (Object[])a.clone();
mergeSort(aux, a, 0, a.length, 0);
}
}
As a practical matter, you always take the first branch of the if-statement. Of course you have to duplicate the above code with the argument types changed from Object[] to Comparable[], but you get a 9.7% improvement in sorting speed. To me, this is a no-brainer.
Josh adds:
One other thing: Collections.sort dumps the List into an Object array prior to sorting (even though it *knows* that the elements are all Comparable). In order to make this optimization work for Collections.sort, it should dump the Collection into an array of Comparable, and (what the heck) directly invoke mergesort2.