From bf9d3d87a620a06ee78bce7c2dc76a647f3c2b2f Mon Sep 17 00:00:00 2001 From: Fleshgrinder Date: Wed, 25 Dec 2019 14:22:19 +0100 Subject: [PATCH] KT-15363 Collection Builders MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Added container builders for lists, sets, and maps. The new experimental type inference only works for the simple builders with a single generic type. The versions with nullability and the maps still require explicit specification of the types. Obviously explicit specification is required for all users who are not using the new experimental inference. Improving the type inference is something that should be done separately and many things – including these builders – will benefit from it, however, this is not a blocker for these builders in my opinion. --- libraries/stdlib/js/src/kotlin/collections.kt | 7 ++ .../jvm/src/kotlin/collections/MapsJVM.kt | 18 +++++ .../test/samples/collections/builders.kt | 51 ++++++++++++++ .../src/kotlin/collections/Collections.kt | 29 ++++++++ .../stdlib/src/kotlin/collections/Maps.kt | 43 ++++++++---- .../stdlib/src/kotlin/collections/Sets.kt | 31 +++++++++ .../test/collections/ContainerBuilderTest.kt | 68 +++++++++++++++++++ 7 files changed, 234 insertions(+), 13 deletions(-) create mode 100644 libraries/stdlib/samples/test/samples/collections/builders.kt create mode 100644 libraries/stdlib/test/collections/ContainerBuilderTest.kt diff --git a/libraries/stdlib/js/src/kotlin/collections.kt b/libraries/stdlib/js/src/kotlin/collections.kt index 3b18cfdd45c..7d453019d87 100644 --- a/libraries/stdlib/js/src/kotlin/collections.kt +++ b/libraries/stdlib/js/src/kotlin/collections.kt @@ -182,3 +182,10 @@ internal actual fun checkCountOverflow(count: Int): Int { return count } + +/** + * JS map and set implementations do not make use of capacities or load factors. + */ +@PublishedApi +@kotlin.internal.InlineOnly +internal actual inline fun mapCapacity(expectedSize: Int) = expectedSize diff --git a/libraries/stdlib/jvm/src/kotlin/collections/MapsJVM.kt b/libraries/stdlib/jvm/src/kotlin/collections/MapsJVM.kt index d4c8d2c7220..366fc86f161 100644 --- a/libraries/stdlib/jvm/src/kotlin/collections/MapsJVM.kt +++ b/libraries/stdlib/jvm/src/kotlin/collections/MapsJVM.kt @@ -95,3 +95,21 @@ internal actual inline fun Map.toSingletonMapOrSelf(): Map = internal actual fun Map.toSingletonMap(): Map = with(entries.iterator().next()) { java.util.Collections.singletonMap(key, value) } +/** + * Calculate the initial capacity of a map, based on Guava's + * [com.google.common.collect.Maps.capacity](https://github.com/google/guava/blob/master/guava/src/com/google/common/collect/Maps.java) + * approach. This is similar to [java.util.HashMap.putMapEntries] (JDK8+) but provides further optimizations for small or large sizes. + */ +@PublishedApi +internal actual fun mapCapacity(expectedSize: Int): Int = when { + // We are not coercing the value to a valid one and not throwing an exception. It is up to the receiving class to + // properly handle negative values. Note that all built-in classes do exactly that and will throw. + expectedSize < 0 -> expectedSize + expectedSize < MIN_CAPACITY -> expectedSize + 1 + expectedSize < MAX_CAPACITY -> ((expectedSize / 0.75F) + 1.0F).toInt() + else -> MAX_CAPACITY +} + +// java.util.HashMap.MAX_CAPACITY +private const val MAX_CAPACITY = 1 shl 30 +private const val MIN_CAPACITY = 3 diff --git a/libraries/stdlib/samples/test/samples/collections/builders.kt b/libraries/stdlib/samples/test/samples/collections/builders.kt new file mode 100644 index 00000000000..772c22a8e9d --- /dev/null +++ b/libraries/stdlib/samples/test/samples/collections/builders.kt @@ -0,0 +1,51 @@ +package samples.collections + +import samples.* + +@RunWith(Enclosed::class) +class Builders { + class Lists { + @Sample + fun buildListSample() { + val x = listOf('b', 'c') + + val y = buildList(x.size + 2) { + add('a') + addAll(x) + add('d') + } + + assertPrints(y, "[a, b, c, d]") + } + } + + class Sets { + @Sample + fun buildSetSample() { + val x = setOf('b', 'c') + + val y = buildSet(x.size + 2) { + add('a') + addAll(x) + add('d') + } + + assertPrints(y, "[a, b, c, d]") + } + } + + class Maps { + @Sample + fun buildMapSample() { + val x = mapOf('b' to 2, 'c' to 3) + + val y = buildMap(x.size + 2) { + put('a', 1) + putAll(x) + put('d', 4) + } + + assertPrints(y, "{a=1, b=2, c=3, d=4}") + } + } +} diff --git a/libraries/stdlib/src/kotlin/collections/Collections.kt b/libraries/stdlib/src/kotlin/collections/Collections.kt index a124d0f96c6..8158128307a 100644 --- a/libraries/stdlib/src/kotlin/collections/Collections.kt +++ b/libraries/stdlib/src/kotlin/collections/Collections.kt @@ -5,6 +5,7 @@ @file:kotlin.jvm.JvmMultifileClass @file:kotlin.jvm.JvmName("CollectionsKt") +@file:UseExperimental(kotlin.experimental.ExperimentalTypeInference::class) package kotlin.collections @@ -153,6 +154,34 @@ public inline fun MutableList(size: Int, init: (index: Int) -> T): MutableLi return list } +/** + * Build a new read-only [List] with the [elements][E] from the [builderAction]. + * + * @sample samples.collections.Builders.Lists.buildListSample + */ +@SinceKotlin("1.3") +@ExperimentalStdlibApi +@kotlin.internal.InlineOnly +public inline fun buildList(@BuilderInference builderAction: MutableList.() -> Unit): List { + contract { callsInPlace(builderAction, InvocationKind.EXACTLY_ONCE) } + return ArrayList().apply(builderAction) +} + +/** + * Build a new read-only [List] with the given [expectedSize] and [elements][E] from the [builderAction]. + * + * @sample samples.collections.Builders.Lists.buildListSample + * @throws IllegalArgumentException if the given [expectedSize] is negative. + */ +@SinceKotlin("1.3") +@ExperimentalStdlibApi +@kotlin.internal.InlineOnly +public inline fun buildList(expectedSize: Int, @BuilderInference builderAction: MutableList.() -> Unit): List { + contract { callsInPlace(builderAction, InvocationKind.EXACTLY_ONCE) } + return ArrayList(expectedSize).apply(builderAction) +} + + /** * Returns an [IntRange] of the valid indices for this collection. * @sample samples.collections.Collections.Collections.indicesOfCollection diff --git a/libraries/stdlib/src/kotlin/collections/Maps.kt b/libraries/stdlib/src/kotlin/collections/Maps.kt index fc530a5eaf2..8b3484b975a 100644 --- a/libraries/stdlib/src/kotlin/collections/Maps.kt +++ b/libraries/stdlib/src/kotlin/collections/Maps.kt @@ -5,6 +5,7 @@ @file:kotlin.jvm.JvmMultifileClass @file:kotlin.jvm.JvmName("MapsKt") +@file:UseExperimental(kotlin.experimental.ExperimentalTypeInference::class) package kotlin.collections @@ -123,22 +124,38 @@ public inline fun linkedMapOf(): LinkedHashMap = LinkedHashMap linkedMapOf(vararg pairs: Pair): LinkedHashMap = pairs.toMap(LinkedHashMap(mapCapacity(pairs.size))) /** - * Calculate the initial capacity of a map, based on Guava's com.google.common.collect.Maps approach. This is equivalent - * to the Collection constructor for HashSet, (c.size()/.75f) + 1, but provides further optimisations for very small or - * very large sizes, allows support non-collection classes, and provides consistency for all map based class construction. + * Build a new read-only [Map] with the [key][K]-[value][V] pairs from the [builderAction] while preserving the insertion order. + * + * @sample samples.collections.Builders.Maps.buildMapSample */ -@PublishedApi -internal fun mapCapacity(expectedSize: Int): Int { - if (expectedSize < 3) { - return expectedSize + 1 - } - if (expectedSize < INT_MAX_POWER_OF_TWO) { - return expectedSize + expectedSize / 3 - } - return Int.MAX_VALUE // any large value +@SinceKotlin("1.3") +@ExperimentalStdlibApi +@kotlin.internal.InlineOnly +public inline fun buildMap(@BuilderInference builderAction: MutableMap.() -> Unit): Map { + contract { callsInPlace(builderAction, InvocationKind.EXACTLY_ONCE) } + return LinkedHashMap().apply(builderAction) } -private const val INT_MAX_POWER_OF_TWO: Int = Int.MAX_VALUE / 2 + 1 +/** + * Build a new read-only [Map] with the given [expectedSize] and [key][K]-[value][V] pairs from the [builderAction] while preserving + * the insertion order. + * + * @sample samples.collections.Builders.Maps.buildMapSample + * @throws IllegalArgumentException if the given [expectedSize] is negative. + */ +@SinceKotlin("1.3") +@ExperimentalStdlibApi +@kotlin.internal.InlineOnly +public inline fun buildMap(expectedSize: Int, @BuilderInference builderAction: MutableMap.() -> Unit): Map { + contract { callsInPlace(builderAction, InvocationKind.EXACTLY_ONCE) } + return LinkedHashMap(mapCapacity(expectedSize)).apply(builderAction) +} + +/** + * Calculate the initial capacity of a map. + */ +@PublishedApi +internal expect fun mapCapacity(expectedSize: Int): Int /** Returns `true` if this map is not empty. */ @kotlin.internal.InlineOnly diff --git a/libraries/stdlib/src/kotlin/collections/Sets.kt b/libraries/stdlib/src/kotlin/collections/Sets.kt index b88026c1621..c5af60f100b 100644 --- a/libraries/stdlib/src/kotlin/collections/Sets.kt +++ b/libraries/stdlib/src/kotlin/collections/Sets.kt @@ -5,9 +5,11 @@ @file:kotlin.jvm.JvmMultifileClass @file:kotlin.jvm.JvmName("SetsKt") +@file:UseExperimental(kotlin.experimental.ExperimentalTypeInference::class) package kotlin.collections +import kotlin.contracts.* internal object EmptySet : Set, Serializable { private const val serialVersionUID: Long = 3406603774387020532 @@ -88,6 +90,35 @@ public inline fun linkedSetOf(): LinkedHashSet = LinkedHashSet() */ public fun linkedSetOf(vararg elements: T): LinkedHashSet = elements.toCollection(LinkedHashSet(mapCapacity(elements.size))) +/** + * Build a new read-only [Set] with the [elements][E] from the [builderAction] while preserving the insertion order. + * + * @sample samples.collections.Builders.Sets.buildSetSample + */ +@SinceKotlin("1.3") +@ExperimentalStdlibApi +@kotlin.internal.InlineOnly +public inline fun buildSet(@BuilderInference builderAction: MutableSet.() -> Unit): Set { + contract { callsInPlace(builderAction, InvocationKind.EXACTLY_ONCE) } + return LinkedHashSet().apply(builderAction) +} + +/** + * Build a new read-only [Set] with the given [expectedSize] and [elements][E] from the [builderAction] while preserving the insertion + * order. + * + * @sample samples.collections.Builders.Sets.buildSetSample + * @throws IllegalArgumentException if the given [expectedSize] is negative. + */ +@SinceKotlin("1.3") +@ExperimentalStdlibApi +@kotlin.internal.InlineOnly +public inline fun buildSet(expectedSize: Int, @BuilderInference builderAction: MutableSet.() -> Unit): Set { + contract { callsInPlace(builderAction, InvocationKind.EXACTLY_ONCE) } + return LinkedHashSet(mapCapacity(expectedSize)).apply(builderAction) +} + + /** Returns this Set if it's not `null` and the empty set otherwise. */ @kotlin.internal.InlineOnly public inline fun Set?.orEmpty(): Set = this ?: emptySet() diff --git a/libraries/stdlib/test/collections/ContainerBuilderTest.kt b/libraries/stdlib/test/collections/ContainerBuilderTest.kt new file mode 100644 index 00000000000..141aa6da731 --- /dev/null +++ b/libraries/stdlib/test/collections/ContainerBuilderTest.kt @@ -0,0 +1,68 @@ +package test.collections + +import kotlin.test.* + +class ContainerBuilderTest { + @Test fun buildList() { + val x = buildList { + add('b') + add('c') + } + + val y = buildList(4) { + add('a') + addAll(x) + add('d') + } + + assertEquals(listOf('a', 'b', 'c', 'd'), y) + } + + @Test fun exceptionIsThrownIfExpectedListSizeIsNegative() { + assertFailsWith(IllegalArgumentException::class) { + buildList(-1) {} + } + } + + @Test fun buildSet() { + val x = buildSet { + add('b') + add('c') + } + + val y = buildSet(4) { + add('a') + addAll(x) + add('d') + } + + assertEquals(setOf('a', 'b', 'c', 'd'), y) + } + + @Test fun exceptionIsThrownIfExpectedSetSizeIsNegative() { + assertFailsWith(IllegalArgumentException::class) { + buildSet(-1) {} + } + } + + @Test fun buildMap() { + val x = buildMap { + put('b', 2) + put('c', 3) + } + + val y = buildMap(4) { + put('a', 1) + putAll(x) + put('d', 4) + } + + assertEquals(mapOf('a' to 1, 'b' to 2, 'c' to 3, 'd' to 4), y) + } + + @Test fun exceptionIsThrownIfExpectedMapSizeIsNegative() { + assertFailsWith(IllegalArgumentException::class) { + buildMap(-1) {} + } + } +}