Optimized toArray method for ArrayDeque and ListBuilder #KT-42720

libraries/stdlib/api/js-v1/kotlin.collections.kt

@@ -10519,6 +10519,10 @@ public final class ArrayDeque<E> : kotlin.collections.AbstractMutableList<E> {
     public open override fun retainAll(elements: kotlin.collections.Collection<E>): kotlin.Boolean
 
     public open override operator fun set(index: kotlin.Int, element: E): E
+
+    protected open override fun toArray(): kotlin.Array<kotlin.Any?>
+
+    protected open override fun <T> toArray(array: kotlin.Array<T>): kotlin.Array<T>
 }
 
 public open class ArrayList<E> : kotlin.collections.AbstractMutableList<E>, kotlin.collections.MutableList<E>, kotlin.collections.RandomAccess {
@@ -10558,6 +10562,8 @@ public open class ArrayList<E> : kotlin.collections.AbstractMutableList<E>, kotl
 
     protected open override fun toArray(): kotlin.Array<kotlin.Any?>
 
+    protected open override fun <T> toArray(array: kotlin.Array<T>): kotlin.Array<T>
+
     public open override fun toString(): kotlin.String
 
     public final fun trimToSize(): kotlin.Unit

libraries/stdlib/api/js/kotlin.collections.kt

@@ -10483,6 +10483,10 @@ public final class ArrayDeque<E> : kotlin.collections.AbstractMutableList<E> {
     public open override fun retainAll(elements: kotlin.collections.Collection<E>): kotlin.Boolean
 
     public open override operator fun set(index: kotlin.Int, element: E): E
+
+    protected open override fun toArray(): kotlin.Array<kotlin.Any?>
+
+    protected open override fun <T> toArray(array: kotlin.Array<T>): kotlin.Array<T>
 }
 
 public open class ArrayList<E> : kotlin.collections.AbstractMutableList<E>, kotlin.collections.MutableList<E>, kotlin.collections.RandomAccess {
@@ -10522,6 +10526,8 @@ public open class ArrayList<E> : kotlin.collections.AbstractMutableList<E>, kotl
 
     protected open override fun toArray(): kotlin.Array<kotlin.Any?>
 
+    protected open override fun <T> toArray(array: kotlin.Array<T>): kotlin.Array<T>
+
     public open override fun toString(): kotlin.String
 
     public final fun trimToSize(): kotlin.Unit

libraries/stdlib/js/src/kotlin/collections/ArrayList.kt

@@ -133,7 +133,25 @@ public actual open class ArrayList<E> internal constructor(private var array: Ar
     actual override fun lastIndexOf(element: E): Int = array.lastIndexOf(element)
 
     override fun toString() = arrayToString(array)
-    override fun toArray(): Array<Any?> = js("[]").slice.call(array)
+
+    @Suppress("UNCHECKED_CAST")
+    override fun <T> toArray(array: Array<T>): Array<T> {
+        if (array.size < size) {
+            return toArray() as Array<T>
+        }
+
+        (this.array as Array<T>).copyInto(array)
+
+        if (array.size > size) {
+            array[size] = null as T // null-terminate
+        }
+
+        return array
+    }
+
+    override fun toArray(): Array<Any?> {
+        return js("[]").slice.call(array)
+    }
 
 
     internal override fun checkIsMutable() {

libraries/stdlib/jvm/src/kotlin/collections/builders/ListBuilder.kt

@@ -130,13 +130,25 @@ internal class ListBuilder<E> private constructor(
         return ListBuilder(array, offset + fromIndex, toIndex - fromIndex, isReadOnly, this, root ?: this)
     }
 
-    @OptIn(ExperimentalStdlibApi::class)
+    override fun <T> toArray(destination: Array<T>): Array<T> {
-    private fun ensureCapacity(minCapacity: Int) {
+        if (destination.size < length) {
-        if (backing != null) throw IllegalStateException() // just in case somebody casts subList to ListBuilder
+            return java.util.Arrays.copyOfRange(array, offset, offset + length, destination.javaClass)
-        if (minCapacity > array.size) {
-            val newSize = ArrayDeque.newCapacity(array.size, minCapacity)
-            array = array.copyOfUninitializedElements(newSize)
         }
+
+        @Suppress("UNCHECKED_CAST")
+        (array as Array<T>).copyInto(destination, 0, startIndex = offset, endIndex = offset + length)
+
+        if (destination.size > length) {
+            @Suppress("UNCHECKED_CAST")
+            destination[length] = null as T // null-terminate
+        }
+
+        return destination
+    }
+
+    override fun toArray(): Array<Any?> {
+        @Suppress("UNCHECKED_CAST")
+        return array.copyOfRange(fromIndex = offset, toIndex = offset + length) as Array<Any?>
     }
 
     override fun equals(other: Any?): Boolean {
@@ -154,6 +166,14 @@ internal class ListBuilder<E> private constructor(
 
     // ---------------------------- private ----------------------------
 
+    private fun ensureCapacity(minCapacity: Int) {
+        if (backing != null) throw IllegalStateException() // just in case somebody casts subList to ListBuilder
+        if (minCapacity > array.size) {
+            val newSize = ArrayDeque.newCapacity(array.size, minCapacity)
+            array = array.copyOfUninitializedElements(newSize)
+        }
+    }
+
     private fun checkIsMutable() {
         if (isReadOnly || root != null && root.isReadOnly) throw UnsupportedOperationException()
     }

libraries/stdlib/jvm/test/collections/ListBuilderTest.kt

@@ -0,0 +1,60 @@
+/*
+ * Copyright 2010-2021 JetBrains s.r.o. and Kotlin Programming Language contributors.
+ * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file.
+ */
+
+package test.collections
+
+import kotlin.collections.builders.*
+import kotlin.test.Test
+import kotlin.test.assertSame
+import kotlin.test.assertTrue
+
+@Suppress("INVISIBLE_MEMBER")
+class ListBuilderTest {
+    @Test
+    @Suppress("PLATFORM_CLASS_MAPPED_TO_KOTLIN", "UNCHECKED_CAST")
+    fun toArray() {
+        val numberOfElements = 5
+        val expected = ArrayList<Int>().apply { addAll(0 until numberOfElements) }
+        val builder = ListBuilder<Int>().apply { addAll(0 until numberOfElements) }
+
+        fun testToArray(getter: List<Int>.() -> Array<Any?>) {
+            assertTrue(expected.getter() contentEquals builder.getter())
+            assertTrue(expected.subList(2, 4).getter() contentEquals builder.subList(2, 4).getter())
+        }
+
+        testToArray { (this as java.util.Collection<Int>).toArray() }
+    }
+
+    @Test
+    @Suppress("PLATFORM_CLASS_MAPPED_TO_KOTLIN", "UNCHECKED_CAST")
+    fun toArrayDestination() {
+        val numberOfElements = 5
+        val expected = ArrayList<Int>().apply { addAll(0 until numberOfElements) }
+        val builder = ListBuilder<Int>().apply { addAll(0 until numberOfElements) }
+
+        fun testToArray(destSize: Int, getter: List<Int>.(Array<Int>) -> Array<Int>) {
+            repeat(2) { index ->
+                val expectedDest = Array(destSize) { -it - 1 }
+                val builderDest = Array(destSize) { -it - 1 }
+
+                val takeSubList = index == 1
+                val expectedResult = (if (!takeSubList) expected else expected.subList(2, 4)).getter(expectedDest)
+                val builderResult = (if (!takeSubList) builder else builder.subList(2, 4)).getter(builderDest)
+
+                if (expectedResult.size <= expectedDest.size) {
+                    assertSame(expectedDest, expectedResult)
+                    assertSame(builderDest, builderResult)
+                }
+                assertTrue(expectedResult contentEquals builderResult)
+            }
+        }
+
+        testToArray(0) { (this as java.util.Collection<Int>).toArray(it) }
+        testToArray(numberOfElements - 1) { (this as java.util.Collection<Int>).toArray(it) }
+        testToArray(numberOfElements) { (this as java.util.Collection<Int>).toArray(it) }
+        testToArray(numberOfElements + 1) { (this as java.util.Collection<Int>).toArray(it) }
+        testToArray(numberOfElements + 2) { (this as java.util.Collection<Int>).toArray(it) }
+    }
+}

libraries/stdlib/src/kotlin/collections/ArrayDeque.kt

@@ -529,6 +529,35 @@ public class ArrayDeque<E> : AbstractMutableList<E> {
         size = 0
     }
 
+    @Suppress("NOTHING_TO_OVERRIDE")
+    override fun <T> toArray(array: Array<T>): Array<T> {
+        @Suppress("UNCHECKED_CAST")
+        val dest = (if (array.size >= size) array else arrayOfNulls(array, size)) as Array<Any?>
+
+        val tail = internalIndex(size)
+        if (head < tail) {
+            elementData.copyInto(dest, startIndex = head, endIndex = tail)
+        } else if (isNotEmpty()) {
+            elementData.copyInto(dest, destinationOffset = 0, startIndex = head, endIndex = elementData.size)
+            elementData.copyInto(dest, destinationOffset = elementData.size - head, startIndex = 0, endIndex = tail)
+        }
+        if (dest.size > size) {
+            dest[size] = null // null-terminate
+        }
+
+        @Suppress("UNCHECKED_CAST")
+        return dest as Array<T>
+    }
+
+    @Suppress("NOTHING_TO_OVERRIDE")
+    override fun toArray(): Array<Any?> {
+        return toArray(arrayOfNulls<Any?>(size))
+    }
+
+    // for testing
+    internal fun <T> testToArray(array: Array<T>): Array<T> = toArray(array)
+    internal fun testToArray(): Array<Any?> = toArray()
+
     internal companion object {
         private val emptyElementData = emptyArray<Any?>()
         private const val maxArraySize = Int.MAX_VALUE - 8
@@ -548,20 +577,7 @@ public class ArrayDeque<E> : AbstractMutableList<E> {
     // For testing only
     internal fun internalStructure(structure: (head: Int, elements: Array<Any?>) -> Unit) {
         val tail = internalIndex(size)
-
+        val head = if (isEmpty() || head < tail) head else head - elementData.size
-        if (isEmpty()) {
+        structure(head, toArray())
-            structure(head, emptyArray())
-            return
-        }
-
-        val elements = arrayOfNulls<Any?>(size)
-        if (head < tail) {
-            elementData.copyInto(elements, startIndex = head, endIndex = tail)
-            structure(head, elements)
-        } else {
-            elementData.copyInto(elements, startIndex = head)
-            elementData.copyInto(elements, elementData.size - head, startIndex = 0, endIndex = tail)
-            structure(head - elementData.size, elements)
-        }
     }
 }

libraries/stdlib/test/collections/ArrayDequeTest.kt

@@ -8,7 +8,6 @@ package test.collections
 import test.collections.behaviors.iteratorBehavior
 import test.collections.behaviors.listIteratorBehavior
 import test.collections.behaviors.listIteratorProperties
-import test.collections.compare
 import kotlin.random.Random
 import kotlin.random.nextInt
 import kotlin.test.*
@@ -647,4 +646,48 @@ class ArrayDequeTest {
             assertEquals(Int.MAX_VALUE, ArrayDeque.newCapacity(oldCapacity, minCapacity))
         }
     }
+
+    @Suppress("INVISIBLE_MEMBER")
+    @Test
+    fun toArray() {
+        val deque = ArrayDeque<Int>()
+
+        // empty
+        assertTrue(deque.testToArray().isEmpty())
+
+        fun testContentEquals(expected: Array<Int>) {
+            assertTrue(expected contentEquals deque.testToArray())
+            assertTrue(expected contentEquals deque.testToArray(emptyArray()))
+
+            val dest = Array(expected.size + 2) { it + 100 }
+
+            @Suppress("UNCHECKED_CAST")
+            val nullTerminatedExpected = (expected as Array<Any?>) + null + (expected.size + 101)
+            val actual = deque.testToArray(dest)
+            assertTrue(
+                nullTerminatedExpected contentEquals actual,
+                message = "Expected: ${nullTerminatedExpected.contentToString()}, Actual: ${actual.contentToString()}"
+            )
+        }
+
+        // head < tail
+        deque.addAll(listOf(0, 1, 2, 3))
+        deque.internalStructure { head, _ -> assertEquals(0, head) }
+        testContentEquals(arrayOf(0, 1, 2, 3))
+        deque.removeFirst()
+        deque.internalStructure { head, _ -> assertEquals(1, head) }
+        testContentEquals(arrayOf(1, 2, 3))
+
+        // head > tail
+        deque.addFirst(-1)
+        deque.addFirst(-2)
+        deque.addFirst(-3)
+        deque.internalStructure { head, _ -> assertEquals(-2, head) } // deque min capacity is 10
+        testContentEquals(arrayOf(-3, -2, -1, 1, 2, 3))
+
+        // head == tail
+        deque.addAll(listOf(4, 5, 6, 7))
+        deque.internalStructure { head, _ -> assertEquals(-2, head) } // deque min capacity is 10
+        testContentEquals(arrayOf(-3, -2, -1, 1, 2, 3, 4, 5, 6, 7))
+    }
 }

libraries/tools/binary-compatibility-validator/reference-public-api/kotlin-stdlib-runtime-merged.txt

@@ -727,6 +727,8 @@ public final class kotlin/collections/ArrayDeque : kotlin/collections/AbstractMu
 	public final fun removeLastOrNull ()Ljava/lang/Object;
 	public fun retainAll (Ljava/util/Collection;)Z
 	public fun set (ILjava/lang/Object;)Ljava/lang/Object;
+	public fun toArray ()[Ljava/lang/Object;
+	public fun toArray ([Ljava/lang/Object;)[Ljava/lang/Object;
 }
 
 public final class kotlin/collections/ArraysKt {