From 368f21461c0ba722f26c5ee767a0d744183d7fbd Mon Sep 17 00:00:00 2001 From: Filipp Zhinkin Date: Tue, 11 Apr 2023 11:51:01 +0200 Subject: [PATCH] KT-55091 Optimize Sequence::toSet/toList implementation Sequence::toSet and Sequence::toList both create a collection, fill it with elements, and then try to optimize a result by returning empty, singleton, or the allocated collection depending on the elements count. Instead of allocating the collection and then trying to return an optimized instance it is worth checking the sequence's size beforehand and return empty/singleton collection when possible. Proposed change optimize performance of aforementioned functions and also reduce allocation rate when a sequence consists of 0 or 1 elements. ^KT-55091 fixed --- .../stdlib/common/src/generated/_Sequences.kt | 22 +++++++++++++++++-- .../stdlib/test/collections/SequenceTest.kt | 16 +++++++++++++- .../src/templates/Snapshots.kt | 20 ++++++++++++++--- 3 files changed, 52 insertions(+), 6 deletions(-) diff --git a/libraries/stdlib/common/src/generated/_Sequences.kt b/libraries/stdlib/common/src/generated/_Sequences.kt index 9f4a78dc054..a9245b4a1d0 100644 --- a/libraries/stdlib/common/src/generated/_Sequences.kt +++ b/libraries/stdlib/common/src/generated/_Sequences.kt @@ -805,7 +805,16 @@ public fun Sequence.toHashSet(): HashSet { * The operation is _terminal_. */ public fun Sequence.toList(): List { - return this.toMutableList().optimizeReadOnlyList() + val it = iterator() + if (!it.hasNext()) + return emptyList() + val element = it.next() + if (!it.hasNext()) + return listOf(element) + val dst = ArrayList() + dst.add(element) + while (it.hasNext()) dst.add(it.next()) + return dst } /** @@ -825,7 +834,16 @@ public fun Sequence.toMutableList(): MutableList { * The operation is _terminal_. */ public fun Sequence.toSet(): Set { - return toCollection(LinkedHashSet()).optimizeReadOnlySet() + val it = iterator() + if (!it.hasNext()) + return emptySet() + val element = it.next() + if (!it.hasNext()) + return setOf(element) + val dst = LinkedHashSet() + dst.add(element) + while (it.hasNext()) dst.add(it.next()) + return dst } /** diff --git a/libraries/stdlib/test/collections/SequenceTest.kt b/libraries/stdlib/test/collections/SequenceTest.kt index 9ae24f556f8..84b169ca8d7 100644 --- a/libraries/stdlib/test/collections/SequenceTest.kt +++ b/libraries/stdlib/test/collections/SequenceTest.kt @@ -1,5 +1,5 @@ /* - * Copyright 2010-2020 JetBrains s.r.o. and Kotlin Programming Language contributors. + * Copyright 2010-2023 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. */ @@ -745,6 +745,20 @@ public class SequenceTest { assertNull(fibonacci().take(10).firstNotNullOfOrNull { it.doubleIfNotMonodigit() }) } + @Test fun toSet() { + assertEquals(emptySet(), emptySequence().toSet()) + assertEquals(setOf(42), sequenceOf(42).toSet()) + assertEquals(setOf(3, 2, 1), sequenceOf(3, 2, 1).toSet()) + assertEquals(setOf(1, 2, 3), sequenceOf(1, 2, 1, 3, 2, 3).toSet()) + } + + @Test fun toList() { + assertEquals(emptyList(), emptySequence().toList()) + assertEquals(listOf(42), sequenceOf(42).toList()) + assertEquals(listOf(3, 2, 1), sequenceOf(3, 2, 1).toList()) + assertEquals(listOf(1, 2, 1, 3, 2, 3), sequenceOf(1, 2, 1, 3, 2, 3).toList()) + } + /* test fun pairIterator() { val pairStr = (fibonacci() zip fibonacci().map { i -> i*2 }).joinToString(limit = 10) diff --git a/libraries/tools/kotlin-stdlib-gen/src/templates/Snapshots.kt b/libraries/tools/kotlin-stdlib-gen/src/templates/Snapshots.kt index 6a73dbf779b..3f58410a326 100644 --- a/libraries/tools/kotlin-stdlib-gen/src/templates/Snapshots.kt +++ b/libraries/tools/kotlin-stdlib-gen/src/templates/Snapshots.kt @@ -1,5 +1,5 @@ /* - * Copyright 2010-2018 JetBrains s.r.o. and Kotlin Programming Language contributors. + * Copyright 2010-2023 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. */ @@ -35,6 +35,20 @@ object Snapshots : TemplateGroupBase() { } } + private fun optimizedSequenceToCollection(emptyFactory: String, singleElementFactory: String, dstType: String) = + """ + val it = iterator() + if (!it.hasNext()) + return $emptyFactory() + val element = it.next() + if (!it.hasNext()) + return $singleElementFactory(element) + val dst = $dstType() + dst.add(element) + while (it.hasNext()) dst.add(it.next()) + return dst + """ + val f_toSet = fn("toSet()") { includeDefault() include(CharSequences) @@ -59,8 +73,7 @@ object Snapshots : TemplateGroupBase() { return toCollection(LinkedHashSet()).optimizeReadOnlySet() """ } - body(Sequences) { "return toCollection(LinkedHashSet()).optimizeReadOnlySet()" } - + body(Sequences) { optimizedSequenceToCollection("emptySet", "setOf", "LinkedHashSet") } body(CharSequences, ArraysOfObjects, ArraysOfPrimitives) { val size = f.code.size val capacity = if (f == CharSequences || primitive == PrimitiveType.Char) "$size.coerceAtMost(128)" else size @@ -169,6 +182,7 @@ object Snapshots : TemplateGroupBase() { } """ } + body(Sequences) { optimizedSequenceToCollection("emptyList", "listOf", "ArrayList") } specialFor(Maps) { doc { "Returns a [List] containing all key-value pairs." } returns("List>")