From cbd38f007c8a29a796c477dc16ad9f2939418ab8 Mon Sep 17 00:00:00 2001 From: Ilya Gorbunov Date: Wed, 17 Feb 2016 19:30:26 +0300 Subject: [PATCH] JS: Do not create a copy of already cloned vararg array. JVM: Do not create copy of vararg array it it's already Object[]. --- js/js.libraries/src/core/kotlin.kt | 10 +++-- .../src/kotlin/collections/Collections.kt | 18 +++++--- .../stdlib/test/js/javautilCollectionsTest.kt | 44 ++++++++++++++----- 3 files changed, 51 insertions(+), 21 deletions(-) diff --git a/js/js.libraries/src/core/kotlin.kt b/js/js.libraries/src/core/kotlin.kt index 96e0c8eded2..f06847cd98c 100644 --- a/js/js.libraries/src/core/kotlin.kt +++ b/js/js.libraries/src/core/kotlin.kt @@ -75,6 +75,10 @@ internal fun arrayPlusCollection(array: dynamic, collection: Collection): return result } -// copies vararg array due to different spread vararg behavior in JS. -// After fixing #KT-6491 may return `this` -internal inline fun Array.varargToArrayOfAny(): Array = this.copyOf() + +internal inline fun Array.copyToArrayOfAny(isVarargs: Boolean): Array = + if (isVarargs) + // no need to copy vararg array in JS + this + else + this.copyOf() \ No newline at end of file diff --git a/libraries/stdlib/src/kotlin/collections/Collections.kt b/libraries/stdlib/src/kotlin/collections/Collections.kt index 49f3cde95c7..af1a9ca736a 100644 --- a/libraries/stdlib/src/kotlin/collections/Collections.kt +++ b/libraries/stdlib/src/kotlin/collections/Collections.kt @@ -45,16 +45,16 @@ internal object EmptyList : List, Serializable { private fun readResolve(): Any = EmptyList } -internal fun Array.asCollection(): Collection = ArrayAsCollection(this) +internal fun Array.asCollection(): Collection = ArrayAsCollection(this, isVarargs = false) -private class ArrayAsCollection(val values: Array): Collection { +private class ArrayAsCollection(val values: Array, val isVarargs: Boolean): Collection { override val size: Int get() = values.size override fun isEmpty(): Boolean = values.isEmpty() override fun contains(element: T): Boolean = values.contains(element) override fun containsAll(elements: Collection): Boolean = elements.all { contains(it) } override fun iterator(): Iterator = values.iterator() // override hidden toArray implementation to prevent copying of values array - public fun toArray(): Array = values.varargToArrayOfAny() + public fun toArray(): Array = values.copyToArrayOfAny(isVarargs) } /** Returns an empty read-only list. The returned list is serializable (JVM). */ @@ -76,11 +76,11 @@ public fun listOf(element: T): List = Collections.singletonList(element) /** Returns a new [MutableList] with the given elements. */ public fun mutableListOf(vararg elements: T): MutableList - = if (elements.size == 0) ArrayList() else ArrayList(ArrayAsCollection(elements)) + = if (elements.size == 0) ArrayList() else ArrayList(ArrayAsCollection(elements, isVarargs = true)) /** Returns a new [ArrayList] with the given elements. */ public fun arrayListOf(vararg elements: T): ArrayList - = if (elements.size == 0) ArrayList() else ArrayList(ArrayAsCollection(elements)) + = if (elements.size == 0) ArrayList() else ArrayList(ArrayAsCollection(elements, isVarargs = true)) /** Returns a new read-only list either of single given element, if it is not null, or empty list it the element is null. The returned list is serializable (JVM). */ public fun listOfNotNull(element: T?): List = if (element != null) listOf(element) else emptyList() @@ -133,8 +133,12 @@ public inline fun <@kotlin.internal.OnlyInputTypes T> Collection.containsAll( // copies typed varargs array to array of objects @JvmVersion -private fun Array.varargToArrayOfAny(): Array - = Arrays.copyOf(this, this.size, Array::class.java) +private fun Array.copyToArrayOfAny(isVarargs: Boolean): Array = + if (isVarargs && this.javaClass == Array::class.java) + // if the array came from varargs and already is array of Any, copying isn't required + @Suppress("CAST_NEVER_SUCCEEDS") (this as Array) + else + Arrays.copyOf(this, this.size, Array::class.java) /** * Searches this list or its range for the provided [element] index using binary search algorithm. diff --git a/libraries/stdlib/test/js/javautilCollectionsTest.kt b/libraries/stdlib/test/js/javautilCollectionsTest.kt index f6f1019475d..053fed4cd75 100644 --- a/libraries/stdlib/test/js/javautilCollectionsTest.kt +++ b/libraries/stdlib/test/js/javautilCollectionsTest.kt @@ -35,17 +35,39 @@ class JavautilCollectionsTest { assertEquals(array.toList(), TEST_LIST) } - @test fun arrayListDoesNotCreateArrayView() { - val array = arrayOf(1) - val list = arrayListOf(*array) - assertEquals(1, list[0]) - array[0] = 2 - assertEquals(1, list[0]) + @test fun toListDoesNotCreateArrayView() { + snapshotDoesNotCreateView(arrayOf("first", "last"), { it.toList() }) + snapshotDoesNotCreateView(arrayOf("item", 1), { it.toList() }) + } - val arrayOfAny = arrayOf("first") - val listOfAny = arrayListOf(*arrayOfAny) - assertEquals("first", listOfAny[0]) - arrayOfAny[0] = "last" - assertEquals("first", listOfAny[0]) + @test fun toMutableListDoesNotCreateArrayView() { + snapshotDoesNotCreateView(arrayOf("first", "last"), { it.toMutableList() }) + snapshotDoesNotCreateView(arrayOf("item", 2), { it.toMutableList() }) + } + + @test fun listOfDoesNotCreateView() { + snapshotDoesNotCreateView(arrayOf("first", "last"), { listOf(*it) }) + snapshotDoesNotCreateView(arrayOf("item", 3), { listOf(*it) }) + } + + @test fun mutableListOfDoesNotCreateView() { + snapshotDoesNotCreateView(arrayOf("first", "last"), { mutableListOf(*it) }) + snapshotDoesNotCreateView(arrayOf("item", 4), { mutableListOf(*it) }) + } + + @test fun arrayListDoesNotCreateArrayView() { + snapshotDoesNotCreateView(arrayOf(1, 2), { arrayListOf(*it) }) + snapshotDoesNotCreateView(arrayOf("first", "last"), { arrayListOf(*it) }) + } + + + private fun snapshotDoesNotCreateView(array: Array, snapshot: (Array) -> List) { + val first = array.first() + val last = array.last() + + val list = snapshot(array) + assertEquals(first, list[0]) + array[0] = last + assertEquals(first, list[0]) } }