From 6814c9062538d048cd9ff28d00a7b361942de636 Mon Sep 17 00:00:00 2001 From: Viktor Lofgren Date: Sun, 28 May 2023 11:52:00 +0200 Subject: [PATCH] Fix N-width sorting bug --- .../array/algo/SortAlgoInsertionSort.java | 9 +- .../array/algo/SortAlgoQuickSort.java | 10 +- .../array/algo/LongArraySortNTest.java | 200 ++++++++++++++++++ .../array/algo/LongArraySortTest.java | 43 ---- 4 files changed, 213 insertions(+), 49 deletions(-) create mode 100644 code/libraries/array/src/test/java/nu/marginalia/array/algo/LongArraySortNTest.java diff --git a/code/libraries/array/src/main/java/nu/marginalia/array/algo/SortAlgoInsertionSort.java b/code/libraries/array/src/main/java/nu/marginalia/array/algo/SortAlgoInsertionSort.java index 5e6ce492..b520542b 100644 --- a/code/libraries/array/src/main/java/nu/marginalia/array/algo/SortAlgoInsertionSort.java +++ b/code/libraries/array/src/main/java/nu/marginalia/array/algo/SortAlgoInsertionSort.java @@ -17,11 +17,12 @@ class SortAlgoInsertionSort { for (int i = 1; i < span / sz; i++) { long key = array.get(start + (long) i * sz); - int j; - for (j = i - 1; j >= 0 && array.get(start + (long) j * sz) > key; j--) { - array.swap(start + (long) j * sz, start + (long) (j + 1) * sz); + long j; + for (j = i - 1; j >= 0 && array.get(start + j* sz) > key; j--) { + array.swapn(sz, start + j *sz, start + (j + 1)*sz); } - array.set(start + (long) (j + 1) * sz, key); + + array.set(start + (j + 1) * sz, key); } } diff --git a/code/libraries/array/src/main/java/nu/marginalia/array/algo/SortAlgoQuickSort.java b/code/libraries/array/src/main/java/nu/marginalia/array/algo/SortAlgoQuickSort.java index fe96a946..6b06d663 100644 --- a/code/libraries/array/src/main/java/nu/marginalia/array/algo/SortAlgoQuickSort.java +++ b/code/libraries/array/src/main/java/nu/marginalia/array/algo/SortAlgoQuickSort.java @@ -79,7 +79,7 @@ class SortAlgoQuickSort { static long _quickSortPartition(LongArraySort array, long low, long high) { - long pivotPoint = ((low + high) / (2L)); + long pivotPoint = low + ((high - low) / 2L); long pivot = array.get(pivotPoint); long i = low - 1; @@ -102,9 +102,15 @@ class SortAlgoQuickSort { static long _quickSortPartitionN(LongArraySort array, int wordSize, long low, long high) { - long pivotPoint = ((low + high) / (2L*wordSize)) * wordSize; + long delta = (high - low) / (2L); + long pivotPoint = low + (delta / wordSize) * wordSize; + long pivot = array.get(pivotPoint); + assert (pivotPoint - low) >= 0; + assert (pivotPoint - low) % wordSize == 0; + + long i = low - wordSize; long j = high + wordSize; diff --git a/code/libraries/array/src/test/java/nu/marginalia/array/algo/LongArraySortNTest.java b/code/libraries/array/src/test/java/nu/marginalia/array/algo/LongArraySortNTest.java new file mode 100644 index 00000000..af4c9328 --- /dev/null +++ b/code/libraries/array/src/test/java/nu/marginalia/array/algo/LongArraySortNTest.java @@ -0,0 +1,200 @@ +package nu.marginalia.array.algo; + +import it.unimi.dsi.fastutil.longs.Long2ObjectOpenHashMap; +import it.unimi.dsi.fastutil.longs.LongOpenHashSet; +import nu.marginalia.array.LongArray; +import nu.marginalia.array.page.LongArrayPage; +import nu.marginalia.array.page.PagingLongArray; +import nu.marginalia.array.scheme.PowerOf2PartitioningScheme; +import nu.marginalia.util.test.TestUtil; +import org.apache.commons.lang3.ArrayUtils; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.*; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +@Tag("slow") +class LongArraySortNTest { + + LongArray basic; + LongArray paged; + LongArray shifted; + + Long2ObjectOpenHashMap basicPairs; + Long2ObjectOpenHashMap pagedPairs; + Long2ObjectOpenHashMap shiftedPairs; + + final int size = 1026; + + @BeforeEach + public void setUp() { + basic = LongArrayPage.onHeap(size); + paged = PagingLongArray.newOnHeap(new PowerOf2PartitioningScheme(32), size); + shifted = LongArrayPage.onHeap(size + 30).shifted(30); + + var random = new Random(); + long[] values = new long[size]; + for (int i = 0; i < size; i++) { + values[i] = random.nextInt(0, 1000); + } + for (int i = 1; i < size; i+=2) { + values[i] = -values[i]; + } + + basic.transformEach(0, size, (i, old) -> values[(int) i]); + paged.transformEach(0, size, (i, old) -> values[(int) i]); + shifted.transformEach(0, size, (i, old) -> values[(int) i]); + + basicPairs = asPairs(basic); + pagedPairs = asPairs(paged); + shiftedPairs = asPairs(shifted); + } + + + interface SortOperation { + void sort(LongArray array, long start, long end) throws IOException; + } + + @Test + public void quickSortStressTest() throws IOException { + LongArray array = LongArray.allocate(65536); + sortAlgorithmTester(array, LongArraySort::quickSort); + } + + + @Test + public void insertionSortStressTest() throws IOException { + LongArray array = LongArray.allocate(8192); + sortAlgorithmTester(array, LongArraySort::insertionSort); + } + + @Test + public void mergeSortStressTest() throws IOException { + LongArray array = LongArray.allocate(65536); + Path tempDir = Files.createTempDirectory(getClass().getSimpleName()); + sortAlgorithmTester(array, (a, s, e) -> a.mergeSort(s, e, tempDir)); + TestUtil.clearTempDir(tempDir); + } + + void sortAlgorithmTester(LongArray array, SortOperation operation) throws IOException { + + long[] values = new long[(int) array.size()]; + for (int i = 0; i < values.length; i++) { + values[i] = i; + } + ArrayUtils.shuffle(values); + + long sentinelA = 0xFEEDBEEFL; + long sentinelB = 0xB000B000L; + + int start = 6; + for (int end = start + 1; end < values.length - 1; end+=97) { + + // Use sentinel values to catch if the sort algorithm overwrites end values + array.set(start - 1, sentinelA); + array.set(end, sentinelB); + + long orderInvariantChecksum = 0; + for (long i = 0; i < end - start; i++) { + array.set(start + i, values[start + (int)i]); + + // Try to checksum the contents to catch bugs where the result is sorted + // but a value has been duplicated, overwriting another + orderInvariantChecksum ^= values[start + (int)i]; + } + + operation.sort(array, start, end); + + assertTrue(array.isSorted(start, end), "Array wasn't sorted"); + + assertEquals(sentinelA, array.get(start - 1), "Start position sentinel overwritten"); + assertEquals(sentinelB, array.get(end), "End position sentinel overwritten"); + + long actualChecksum = 0; + for (long i = start; i < end; i++) { + actualChecksum ^= array.get(i); + } + + assertEquals(orderInvariantChecksum, actualChecksum, "Checksum validation failed"); + } + + } + + private void compare(LongArray sorted, Long2ObjectOpenHashMap expectedPairs) { + var actual = asPairs(sorted); + + assertEquals(expectedPairs, actual); + } + + @Test + void insertionSortN() { + basic.insertionSortN(2, 0, size); + assertTrue(basic.isSortedN(2, 0, size)); + + paged.insertionSortN(2, 0, size); + assertTrue(paged.isSortedN(2, 0, size)); + + shifted.insertionSortN(2, 0, size); + assertTrue(shifted.isSortedN(2, 0, size)); + + compare(basic, basicPairs); + compare(paged, pagedPairs); + compare(shifted, shiftedPairs); + } + + @Test + void quickSortN() { + basic.quickSortN(2, 0, size); + assertTrue(basic.isSortedN(2, 0, size)); + + paged.quickSortN(2, 0, size); + assertTrue(paged.isSortedN(2, 0, size)); + + shifted.quickSortN(2, 0, size); + assertTrue(shifted.isSortedN(2, 0, size)); + + compare(basic, basicPairs); + compare(paged, pagedPairs); + compare(shifted, shiftedPairs); + } + + @Test + void mergeSortN() throws IOException { + + basic.mergeSortN(2, 0, size, Path.of("/tmp")); + assertTrue(basic.isSortedN(2, 0, size)); + + paged.mergeSortN(2, 0, size, Path.of("/tmp")); + assertTrue(paged.isSortedN(2, 0, size)); + + shifted.mergeSortN(2, 0, size, Path.of("/tmp")); + assertTrue(shifted.isSortedN(2, 0, size)); + + compare(basic, basicPairs); + compare(paged, pagedPairs); + compare(shifted, shiftedPairs); + } + + private Long2ObjectOpenHashMap asPairs(LongArray array) { + Long2ObjectOpenHashMap map = new Long2ObjectOpenHashMap<>(); + for (long i = 0; i < array.size(); i+=2) { + long key = array.get(i); + long val = array.get(i+1); + if (null == map.get(key)) { + var set = new LongOpenHashSet(); + map.put(key, set); + } + map.get(key).add(val); + } + return map; + } + + +} \ No newline at end of file diff --git a/code/libraries/array/src/test/java/nu/marginalia/array/algo/LongArraySortTest.java b/code/libraries/array/src/test/java/nu/marginalia/array/algo/LongArraySortTest.java index 9b8c7857..535b7671 100644 --- a/code/libraries/array/src/test/java/nu/marginalia/array/algo/LongArraySortTest.java +++ b/code/libraries/array/src/test/java/nu/marginalia/array/algo/LongArraySortTest.java @@ -126,18 +126,6 @@ class LongArraySortTest { assertTrue(shifted.isSorted(0, 128)); } - @Test - void insertionSortN() { - basic.insertionSortN(2, 0, size); - assertTrue(basic.isSortedN(2, 0, size)); - - paged.insertionSortN(2, 0, size); - assertTrue(paged.isSortedN(2, 0, size)); - - shifted.insertionSortN(2, 0, size); - assertTrue(shifted.isSortedN(2, 0, size)); - } - @Test void quickSort() { basic.quickSort(0, size); @@ -150,36 +138,6 @@ class LongArraySortTest { assertTrue(shifted.isSorted(0, size)); } - @Test - void quickSortN() { - basic.quickSortN(2, 0, size); - - if (!basic.isSortedN(2, 0, size)) { - for (int i = 0; i < size; i+=2) { - System.out.println(basic.get(i)); - } - } - assertTrue(basic.isSortedN(2, 0, size)); - - paged.quickSortN(2, 0, size); - assertTrue(paged.isSortedN(2, 0, size)); - - shifted.quickSortN(2, 0, size); - assertTrue(shifted.isSortedN(2, 0, size)); - } - - @Test - void mergeSortN() throws IOException { - basic.mergeSortN(2, 0, size, Path.of("/tmp")); - assertTrue(basic.isSortedN(2, 0, size)); - - paged.mergeSortN(2, 0, size, Path.of("/tmp")); - assertTrue(paged.isSortedN(2, 0, size)); - - shifted.mergeSortN(2, 0, size, Path.of("/tmp")); - assertTrue(shifted.isSortedN(2, 0, size)); - } - @Test void mergeSort() throws IOException { basic.mergeSort(0, size, Path.of("/tmp")); @@ -192,7 +150,6 @@ class LongArraySortTest { assertTrue(shifted.isSorted(0, size)); } - @Test void keepUniqueFuzz() { var array = LongArray.allocate(1000);