From b67ebf36a2b363e94755d920e0f51e1db0be8832 Mon Sep 17 00:00:00 2001 From: Abduqodiri Qurbonzoda Date: Tue, 30 Jan 2024 16:01:14 +0200 Subject: [PATCH] Implement optimized removeRange for ArrayDeque #KT-64956 Test ArrayDeque with size 15. With the existing sizes the removeRange implementation couldn't reach the maximum number of iterations - 3. Benchmark results for ArrayDeque.subList().clear() that uses removeRange() can be found here: https://github.com/qurbonzoda/KotlinArrayDequeBenchmarks/tree/master/results --- libraries/stdlib/api/js/kotlin.collections.kt | 2 + .../kotlin/collections/AbstractMutableList.kt | 5 ++ .../kotlin/collections/AbstractMutableList.kt | 7 ++ .../src/kotlin/collections/ArrayDeque.kt | 85 +++++++++++++++++-- .../stdlib/test/collections/ArrayDequeTest.kt | 36 +++++++- 5 files changed, 128 insertions(+), 7 deletions(-) diff --git a/libraries/stdlib/api/js/kotlin.collections.kt b/libraries/stdlib/api/js/kotlin.collections.kt index fa19b51adbc..f97809155e2 100644 --- a/libraries/stdlib/api/js/kotlin.collections.kt +++ b/libraries/stdlib/api/js/kotlin.collections.kt @@ -10085,6 +10085,8 @@ public final class ArrayDeque : kotlin.collections.AbstractMutableList { public final fun removeLastOrNull(): E? + protected open override fun removeRange(fromIndex: kotlin.Int, toIndex: kotlin.Int): kotlin.Unit + public open override fun retainAll(elements: kotlin.collections.Collection): kotlin.Boolean public open override operator fun set(index: kotlin.Int, element: E): E diff --git a/libraries/stdlib/js/src/kotlin/collections/AbstractMutableList.kt b/libraries/stdlib/js/src/kotlin/collections/AbstractMutableList.kt index df2cf8a31c2..fff0be233fd 100644 --- a/libraries/stdlib/js/src/kotlin/collections/AbstractMutableList.kt +++ b/libraries/stdlib/js/src/kotlin/collections/AbstractMutableList.kt @@ -203,6 +203,11 @@ public actual abstract class AbstractMutableList protected actual constructor return list.set(fromIndex + index, element) } + override fun removeRange(fromIndex: Int, toIndex: Int) { + list.removeRange(this.fromIndex + fromIndex, this.fromIndex + toIndex) + _size -= toIndex - fromIndex + } + override val size: Int get() = _size internal override fun checkIsMutable(): Unit = list.checkIsMutable() diff --git a/libraries/stdlib/native-wasm/src/kotlin/collections/AbstractMutableList.kt b/libraries/stdlib/native-wasm/src/kotlin/collections/AbstractMutableList.kt index d0b1e12020e..0590336eafc 100644 --- a/libraries/stdlib/native-wasm/src/kotlin/collections/AbstractMutableList.kt +++ b/libraries/stdlib/native-wasm/src/kotlin/collections/AbstractMutableList.kt @@ -221,6 +221,13 @@ public actual abstract class AbstractMutableList protected actual constructor return list.set(fromIndex + index, element) } + override fun removeRange(fromIndex: Int, toIndex: Int) { + checkForComodification() + list.removeRange(this.fromIndex + fromIndex, this.fromIndex + toIndex) + _size -= toIndex - fromIndex + modCount = list.modCount + } + override val size: Int get() { checkForComodification() diff --git a/libraries/stdlib/src/kotlin/collections/ArrayDeque.kt b/libraries/stdlib/src/kotlin/collections/ArrayDeque.kt index 79dc53f3244..887e6f5ab27 100644 --- a/libraries/stdlib/src/kotlin/collections/ArrayDeque.kt +++ b/libraries/stdlib/src/kotlin/collections/ArrayDeque.kt @@ -517,12 +517,9 @@ public class ArrayDeque : AbstractMutableList { } public override fun clear() { - val tail = internalIndex(size) - if (head < tail) { - elementData.fill(null, head, tail) - } else if (isNotEmpty()) { - elementData.fill(null, head, elementData.size) - elementData.fill(null, 0, tail) + if (isNotEmpty()) { + val tail = internalIndex(size) + nullifyNonEmpty(head, tail) } head = 0 size = 0 @@ -550,9 +547,85 @@ public class ArrayDeque : AbstractMutableList { return toArray(arrayOfNulls(size)) } + override fun removeRange(fromIndex: Int, toIndex: Int) { + AbstractList.checkRangeIndexes(fromIndex, toIndex, size) + // TODO: registerModification() + + val length = toIndex - fromIndex + when (length) { + 0 -> return + size -> { + clear() + return + } + 1 -> { + removeAt(fromIndex) + return + } + } + + if (fromIndex < size - toIndex) { + // closer to the first element -> shift preceding elements + removeRangeShiftPreceding(fromIndex, toIndex) + + val newHead = positiveMod(head + length) + nullifyNonEmpty(head, newHead) + head = newHead + } else { + // closer to the last element -> shift succeeding elements + removeRangeShiftSucceeding(fromIndex, toIndex) + + val tail = internalIndex(size) + nullifyNonEmpty(negativeMod(tail - length), tail) + } + + size -= length + } + + private fun removeRangeShiftPreceding(fromIndex: Int, toIndex: Int) { + var copyFromIndex = internalIndex(fromIndex - 1) // upper bound of range, inclusive + var copyToIndex = internalIndex(toIndex - 1) // upper bound of range, inclusive + var copyCount = fromIndex + + while (copyCount > 0) { // maximum 3 iterations + val segmentLength = minOf(copyCount, copyFromIndex + 1, copyToIndex + 1) + elementData.copyInto(elementData, copyToIndex - segmentLength + 1, copyFromIndex - segmentLength + 1, copyFromIndex + 1) + + copyFromIndex = negativeMod(copyFromIndex - segmentLength) + copyToIndex = negativeMod(copyToIndex - segmentLength) + copyCount -= segmentLength + } + } + + private fun removeRangeShiftSucceeding(fromIndex: Int, toIndex: Int) { + var copyFromIndex = internalIndex(toIndex) // lower bound of range, inclusive + var copyToIndex = internalIndex(fromIndex) // lower bound of range, inclusive + var copyCount = size - toIndex + + while (copyCount > 0) { // maximum 3 iterations + val segmentLength = minOf(copyCount, elementData.size - copyFromIndex, elementData.size - copyToIndex) + elementData.copyInto(elementData, copyToIndex, copyFromIndex, copyFromIndex + segmentLength) + + copyFromIndex = positiveMod(copyFromIndex + segmentLength) + copyToIndex = positiveMod(copyToIndex + segmentLength) + copyCount -= segmentLength + } + } + + /** If `internalFromIndex == internalToIndex`, the buffer is considered full and all elements are nullified. */ + private fun nullifyNonEmpty(internalFromIndex: Int, internalToIndex: Int) { + if (internalFromIndex < internalToIndex) { + elementData.fill(null, internalFromIndex, internalToIndex) + } else { + elementData.fill(null, internalFromIndex, elementData.size) + elementData.fill(null, 0, internalToIndex) + } + } + // for testing internal fun testToArray(array: Array): Array = toArray(array) internal fun testToArray(): Array = toArray() + internal fun testRemoveRange(fromIndex: Int, toIndex: Int) = removeRange(fromIndex, toIndex) internal companion object { private val emptyElementData = emptyArray() diff --git a/libraries/stdlib/test/collections/ArrayDequeTest.kt b/libraries/stdlib/test/collections/ArrayDequeTest.kt index e04993e1bb8..c8c6a9a403c 100644 --- a/libraries/stdlib/test/collections/ArrayDequeTest.kt +++ b/libraries/stdlib/test/collections/ArrayDequeTest.kt @@ -352,7 +352,7 @@ class ArrayDequeTest { } private fun testArrayDeque(test: (bufferSize: Int, dequeSize: Int, head: Int, tail: Int) -> Unit) { - for (bufferSize in listOf(0, 2, 8)) { + for (bufferSize in listOf(0, 2, 8, 15)) { for (dequeSize in 0..bufferSize) { for (tail in 0 until bufferSize) { val head = tail - dequeSize @@ -609,6 +609,40 @@ class ArrayDequeTest { } } + @Test + fun removeRange() = testArrayDeque { bufferSize: Int, dequeSize: Int, head: Int, tail: Int -> + for (fromIndex in 0..dequeSize) { + for (toIndex in fromIndex..dequeSize) { + val deque = generateArrayDeque(head, tail, bufferSize).apply { testRemoveRange(fromIndex, toIndex) } + + val length = toIndex - fromIndex + val expectedHead = when { + length == 0 -> head + length == dequeSize -> 0 + fromIndex < dequeSize - toIndex -> head + length // shift preceding elements + else -> // shift succeeding elements + if (tail <= length - 1) + head + bufferSize // head becomes positive(head < tail) + else + head + } + + val expectedElements = (head until tail).toMutableList().apply { + repeat(length) { removeAt(fromIndex) } + } + + deque.internalStructure { actualHead, actualElements -> + assertEquals( + expectedHead, + actualHead, + "bufferSize: $bufferSize, head: $head, tail: $tail, fromIndex: $fromIndex, toIndex: $toIndex" + ) + assertEquals(expectedElements, actualElements.toList()) + } + } + } + } + @Suppress("INVISIBLE_MEMBER") @Test fun toArray() {