Avoid creating new PersistentList instance when adding all elements o…

…f an empty collection
Kotlin · Mar 12, 2024 · 6264df8 · 6264df8
1 parent 7fb0d74
commit 6264df8
Show file tree

Hide file tree

Showing 4 changed files with 27 additions and 0 deletions.
diff --git a/core/commonMain/src/implementations/immutableList/SmallPersistentVector.kt b/core/commonMain/src/implementations/immutableList/SmallPersistentVector.kt
@@ -36,6 +36,7 @@ internal class SmallPersistentVector<E>(private val buffer: Array<Any?>) : Immut
     }
 
     override fun addAll(elements: Collection<E>): PersistentList<E> {
+        if (elements.isEmpty()) return this
         if (size + elements.size <= MAX_BUFFER_SIZE) {
             val newBuffer = buffer.copyOf(size + elements.size)
             // TODO: investigate performance of elements.toArray + copyInto
@@ -80,6 +81,7 @@ internal class SmallPersistentVector<E>(private val buffer: Array<Any?>) : Immut
 
     override fun addAll(index: Int, c: Collection<E>): PersistentList<E> {
         checkPositionIndex(index, size)
+        if (c.isEmpty()) return this
         if (size + c.size <= MAX_BUFFER_SIZE) {
             val newBuffer = bufferOfSize(size + c.size)
             buffer.copyInto(newBuffer, endIndex = index)

diff --git a/core/commonTest/src/contract/list/ImmutableListTest.kt b/core/commonTest/src/contract/list/ImmutableListTest.kt
@@ -68,6 +68,13 @@ class ImmutableListTest {
         compareLists(list, immList)
     }
 
+    @Test fun emptyListToPersistentList() {
+        val empty = emptyList<Int>()
+        val emptyPersistent = empty.toPersistentList()
+
+        assertSame(emptyPersistent, empty.toPersistentList())
+    }
+
     @Test fun addElements() {
         var list = persistentListOf<String>()
         list = list.add("x")
@@ -194,6 +201,9 @@ class ImmutableListTest {
             testNoOperation({ remove('d') }, { remove('d') })
             testNoOperation({ removeAll(listOf('d', 'e')) }, { removeAll(listOf('d', 'e')) })
             testNoOperation({ removeAll { it.isUpperCase() } }, { removeAll { it.isUpperCase() } })
+            testNoOperation({ removeAll(emptyList()) }, { removeAll(emptyList())})
+            testNoOperation({ addAll(emptyList()) }, { addAll(emptyList())})
+            testNoOperation({ addAll(2, emptyList()) }, { addAll(2, emptyList())})
         }
     }
 

diff --git a/core/commonTest/src/contract/map/ImmutableMapTest.kt b/core/commonTest/src/contract/map/ImmutableMapTest.kt
@@ -191,6 +191,12 @@ abstract class ImmutableMapTest {
         assertEquals<Map<*, *>>(map, immMap) // problem
     }
 
+    @Test fun emptyMapToPersistentMap() {
+        val empty = emptyMap<String, Int>()
+        val emptyPersistentMap = empty.toPersistentMap()
+
+        assertSame(emptyPersistentMap, empty.toPersistentMap())
+    }
 
     @Test fun putElements() {
         var map = immutableMapOf<String, Int?>().toPersistentMap()

diff --git a/core/commonTest/src/contract/set/ImmutableSetTest.kt b/core/commonTest/src/contract/set/ImmutableSetTest.kt
@@ -339,13 +339,22 @@ abstract class ImmutableSetTestBase {
         val set = immutableSetOf("abcxyz12".toList())
         with(set) {
             testNoOperation({ add('a') }, { add('a') })
+            testNoOperation({ addAll(emptySet()) }, { addAll(emptySet()) })
             testNoOperation({ addAll(listOf('a', 'b')) }, { addAll(listOf('a', 'b')) })
             testNoOperation({ remove('d') }, { remove('d') })
             testNoOperation({ removeAll(listOf('d', 'e')) }, { removeAll(listOf('d', 'e')) })
             testNoOperation({ removeAll { it.isUpperCase() } }, { removeAll { it.isUpperCase() } })
+            testNoOperation({ removeAll(emptySet()) }, { removeAll(emptySet()) })
         }
     }
 
+    @Test fun emptySetToPersistentSet() {
+        val empty = emptySet<Int>()
+        val emptyPersistentSet = empty.toPersistentSet()
+
+        assertSame(emptyPersistentSet, empty.toPersistentSet())
+    }
+
     fun <T> PersistentSet<T>.testNoOperation(persistent: PersistentSet<T>.() -> PersistentSet<T>, mutating: MutableSet<T>.() -> Unit) {
         val result = this.persistent()
         val buildResult = this.mutate(mutating)