Remove brittle ‘contains’ optimization in minus/removeAll/retainAll #KT-45438

This commit is contained in:
Abduqodiri Qurbonzoda
2021-07-12 13:56:33 +03:00
committed by Ilya Gorbunov
parent fbc43cbebe
commit 94b371af5b
10 changed files with 88 additions and 37 deletions
@@ -135,3 +135,5 @@ internal actual inline fun checkCountOverflow(count: Int): Int {
} }
return count return count
} }
internal actual fun brittleContainsOptimizationEnabled(): Boolean = false
@@ -2995,7 +2995,7 @@ public operator fun <T> Iterable<T>.minus(element: T): List<T> {
*/ */
public operator fun <T> Iterable<T>.minus(elements: Array<out T>): List<T> { public operator fun <T> Iterable<T>.minus(elements: Array<out T>): List<T> {
if (elements.isEmpty()) return this.toList() if (elements.isEmpty()) return this.toList()
val other = elements.toHashSet() val other = elements.convertToSetForSetOperation()
return this.filterNot { it in other } return this.filterNot { it in other }
} }
@@ -3019,7 +3019,7 @@ public operator fun <T> Iterable<T>.minus(elements: Iterable<T>): List<T> {
* a correct and stable implementation of `hashCode()` that doesn't change between successive invocations. * a correct and stable implementation of `hashCode()` that doesn't change between successive invocations.
*/ */
public operator fun <T> Iterable<T>.minus(elements: Sequence<T>): List<T> { public operator fun <T> Iterable<T>.minus(elements: Sequence<T>): List<T> {
val other = elements.toHashSet() val other = elements.convertToSetForSetOperation()
if (other.isEmpty()) if (other.isEmpty())
return this.toList() return this.toList()
return this.filterNot { it in other } return this.filterNot { it in other }
@@ -2449,7 +2449,7 @@ public operator fun <T> Sequence<T>.minus(elements: Array<out T>): Sequence<T> {
if (elements.isEmpty()) return this if (elements.isEmpty()) return this
return object: Sequence<T> { return object: Sequence<T> {
override fun iterator(): Iterator<T> { override fun iterator(): Iterator<T> {
val other = elements.toHashSet() val other = elements.convertToSetForSetOperation()
return this@minus.filterNot { it in other }.iterator() return this@minus.filterNot { it in other }.iterator()
} }
} }
@@ -2492,7 +2492,7 @@ public operator fun <T> Sequence<T>.minus(elements: Iterable<T>): Sequence<T> {
public operator fun <T> Sequence<T>.minus(elements: Sequence<T>): Sequence<T> { public operator fun <T> Sequence<T>.minus(elements: Sequence<T>): Sequence<T> {
return object: Sequence<T> { return object: Sequence<T> {
override fun iterator(): Iterator<T> { override fun iterator(): Iterator<T> {
val other = elements.toHashSet() val other = elements.convertToSetForSetOperation()
if (other.isEmpty()) if (other.isEmpty())
return this@minus.iterator() return this@minus.iterator()
else else
@@ -265,4 +265,6 @@ internal actual fun mapCapacity(expectedSize: Int) = expectedSize
@PublishedApi @PublishedApi
internal fun checkBuilderCapacity(capacity: Int) { internal fun checkBuilderCapacity(capacity: Int) {
require(capacity >= 0) { "capacity must be non-negative." } require(capacity >= 0) { "capacity must be non-negative." }
} }
internal actual fun brittleContainsOptimizationEnabled(): Boolean = false
@@ -123,3 +123,5 @@ internal actual inline fun checkCountOverflow(count: Int): Int {
return count return count
} }
internal actual fun brittleContainsOptimizationEnabled(): Boolean = System.getProperty("kotlin.collections.removing.legacymode").toBoolean()
@@ -0,0 +1,66 @@
/*
* 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 kotlin.collections
/** Returns true if the brittle contains optimization is enabled. See KT-45438. */
internal expect fun brittleContainsOptimizationEnabled(): Boolean
/**
* Returns true if [brittleContainsOptimizationEnabled] is true
* and it's safe to convert this collection to a set without changing contains method behavior.
*/
private fun <T> Collection<T>.safeToConvertToSet() = brittleContainsOptimizationEnabled() && size > 2 && this is ArrayList
/**
* When [brittleContainsOptimizationEnabled] is true:
* - Converts this [Iterable] to a set if it is not a [Collection].
* - Converts this [Collection] to a set, when it's worth so and it doesn't change contains method behavior.
* - Otherwise returns this.
* When [brittleContainsOptimizationEnabled] is false:
* - Converts this [Iterable] to a list if it is not a [Collection].
* - Otherwise returns this.
*/
internal fun <T> Iterable<T>.convertToSetForSetOperationWith(source: Iterable<T>): Collection<T> =
when (this) {
is Set -> this
is Collection ->
when {
source is Collection && source.size < 2 -> this
else -> if (this.safeToConvertToSet()) toHashSet() else this
}
else -> if (brittleContainsOptimizationEnabled()) toHashSet() else toList()
}
/**
* When [brittleContainsOptimizationEnabled] is true:
* - Converts this [Iterable] to a set if it is not a [Collection].
* - Converts this [Collection] to a set, when it's worth so and it doesn't change contains method behavior.
* - Otherwise returns this.
* When [brittleContainsOptimizationEnabled] is false:
* - Converts this [Iterable] to a list if it is not a [Collection].
* - Otherwise returns this.
*/
internal fun <T> Iterable<T>.convertToSetForSetOperation(): Collection<T> =
when (this) {
is Set -> this
is Collection -> if (this.safeToConvertToSet()) toHashSet() else this
else -> if (brittleContainsOptimizationEnabled()) toHashSet() else toList()
}
/**
* Converts this sequence to a set if [brittleContainsOptimizationEnabled] is true,
* otherwise converts it to a list.
*/
internal fun <T> Sequence<T>.convertToSetForSetOperation(): Collection<T> =
if (brittleContainsOptimizationEnabled()) toHashSet() else toList()
/**
* Converts this array to a set if [brittleContainsOptimizationEnabled] is true,
* otherwise converts it to a list.
*/
internal fun <T> Array<T>.convertToSetForSetOperation(): Collection<T> =
if (brittleContainsOptimizationEnabled()) toHashSet() else asList()
@@ -38,29 +38,6 @@ internal fun <T> Iterable<T>.collectionSizeOrNull(): Int? = if (this is Collecti
@PublishedApi @PublishedApi
internal fun <T> Iterable<T>.collectionSizeOrDefault(default: Int): Int = if (this is Collection<*>) this.size else default internal fun <T> Iterable<T>.collectionSizeOrDefault(default: Int): Int = if (this is Collection<*>) this.size else default
/** Returns true when it's safe to convert this collection to a set without changing contains method behavior. */
private fun <T> Collection<T>.safeToConvertToSet() = size > 2 && this is ArrayList
/** Converts this collection to a set, when it's worth so and it doesn't change contains method behavior. */
internal fun <T> Iterable<T>.convertToSetForSetOperationWith(source: Iterable<T>): Collection<T> =
when (this) {
is Set -> this
is Collection ->
when {
source is Collection && source.size < 2 -> this
else -> if (this.safeToConvertToSet()) toHashSet() else this
}
else -> toHashSet()
}
/** Converts this collection to a set, when it's worth so and it doesn't change contains method behavior. */
internal fun <T> Iterable<T>.convertToSetForSetOperation(): Collection<T> =
when (this) {
is Set -> this
is Collection -> if (this.safeToConvertToSet()) toHashSet() else this
else -> toHashSet()
}
/** /**
* Returns a single list of all elements from all collections in the given collection. * Returns a single list of all elements from all collections in the given collection.
@@ -152,7 +152,7 @@ public fun <T> MutableCollection<in T>.removeAll(elements: Iterable<T>): Boolean
* Removes all elements from this [MutableCollection] that are also contained in the given [elements] sequence. * Removes all elements from this [MutableCollection] that are also contained in the given [elements] sequence.
*/ */
public fun <T> MutableCollection<in T>.removeAll(elements: Sequence<T>): Boolean { public fun <T> MutableCollection<in T>.removeAll(elements: Sequence<T>): Boolean {
val set = elements.toHashSet() val set = elements.convertToSetForSetOperation()
return set.isNotEmpty() && removeAll(set) return set.isNotEmpty() && removeAll(set)
} }
@@ -160,7 +160,7 @@ public fun <T> MutableCollection<in T>.removeAll(elements: Sequence<T>): Boolean
* Removes all elements from this [MutableCollection] that are also contained in the given [elements] array. * Removes all elements from this [MutableCollection] that are also contained in the given [elements] array.
*/ */
public fun <T> MutableCollection<in T>.removeAll(elements: Array<out T>): Boolean { public fun <T> MutableCollection<in T>.removeAll(elements: Array<out T>): Boolean {
return elements.isNotEmpty() && removeAll(elements.toHashSet()) return elements.isNotEmpty() && removeAll(elements.convertToSetForSetOperation())
} }
/** /**
@@ -175,7 +175,7 @@ public fun <T> MutableCollection<in T>.retainAll(elements: Iterable<T>): Boolean
*/ */
public fun <T> MutableCollection<in T>.retainAll(elements: Array<out T>): Boolean { public fun <T> MutableCollection<in T>.retainAll(elements: Array<out T>): Boolean {
if (elements.isNotEmpty()) if (elements.isNotEmpty())
return retainAll(elements.toHashSet()) return retainAll(elements.convertToSetForSetOperation())
else else
return retainNothing() return retainNothing()
} }
@@ -184,7 +184,7 @@ public fun <T> MutableCollection<in T>.retainAll(elements: Array<out T>): Boolea
* Retains only elements of this [MutableCollection] that are contained in the given [elements] sequence. * Retains only elements of this [MutableCollection] that are contained in the given [elements] sequence.
*/ */
public fun <T> MutableCollection<in T>.retainAll(elements: Sequence<T>): Boolean { public fun <T> MutableCollection<in T>.retainAll(elements: Sequence<T>): Boolean {
val set = elements.toHashSet() val set = elements.convertToSetForSetOperation()
if (set.isNotEmpty()) if (set.isNotEmpty())
return retainAll(set) return retainAll(set)
else else
@@ -108,4 +108,6 @@ internal actual inline fun <K, V> buildMapInternal(builderAction: MutableMap<K,
@kotlin.internal.InlineOnly @kotlin.internal.InlineOnly
internal actual inline fun <K, V> buildMapInternal(capacity: Int, builderAction: MutableMap<K, V>.() -> Unit): Map<K, V> { internal actual inline fun <K, V> buildMapInternal(capacity: Int, builderAction: MutableMap<K, V>.() -> Unit): Map<K, V> {
return TODO("Wasm stdlib: Collections") return TODO("Wasm stdlib: Collections")
} }
internal actual fun brittleContainsOptimizationEnabled(): Boolean = false
@@ -455,7 +455,7 @@ object Generators : TemplateGroupBase() {
body { body {
""" """
if (elements.isEmpty()) return this.toList() if (elements.isEmpty()) return this.toList()
val other = elements.toHashSet() val other = elements.convertToSetForSetOperation()
return this.filterNot { it in other } return this.filterNot { it in other }
""" """
} }
@@ -491,7 +491,7 @@ object Generators : TemplateGroupBase() {
if (elements.isEmpty()) return this if (elements.isEmpty()) return this
return object: Sequence<T> { return object: Sequence<T> {
override fun iterator(): Iterator<T> { override fun iterator(): Iterator<T> {
val other = elements.toHashSet() val other = elements.convertToSetForSetOperation()
return this@minus.filterNot { it in other }.iterator() return this@minus.filterNot { it in other }.iterator()
} }
} }
@@ -513,7 +513,7 @@ object Generators : TemplateGroupBase() {
specialFor(Sets, Sequences) { returns("SELF") } specialFor(Sets, Sequences) { returns("SELF") }
body { body {
""" """
val other = elements.toHashSet() val other = elements.convertToSetForSetOperation()
if (other.isEmpty()) if (other.isEmpty())
return this.toList() return this.toList()
@@ -552,7 +552,7 @@ object Generators : TemplateGroupBase() {
""" """
return object: Sequence<T> { return object: Sequence<T> {
override fun iterator(): Iterator<T> { override fun iterator(): Iterator<T> {
val other = elements.toHashSet() val other = elements.convertToSetForSetOperation()
if (other.isEmpty()) if (other.isEmpty())
return this@minus.iterator() return this@minus.iterator()
else else