Remove the brittle contains optimization code #KT-47707

libraries/stdlib/common/src/generated/_Collections.kt

@@ -3141,26 +3141,17 @@ public operator fun <T> Iterable<T>.minus(element: T): List<T> {
 
 /**
  * Returns a list containing all elements of the original collection except the elements contained in the given [elements] array.
- * 
- * Before Kotlin 1.6, the [elements] array may have been converted to a [HashSet] to speed up the operation, thus the elements were required to have
- * a correct and stable implementation of `hashCode()` that didn't change between successive invocations.
- * On JVM, you can enable this behavior back with the system property `kotlin.collections.convert_arg_to_set_in_removeAll` set to `true`.
  */
 public operator fun <T> Iterable<T>.minus(elements: Array<out T>): List<T> {
     if (elements.isEmpty()) return this.toList()
-    val other = elements.convertToSetForSetOperation()
+    return this.filterNot { it in elements }
-    return this.filterNot { it in other }
 }
 
 /**
  * Returns a list containing all elements of the original collection except the elements contained in the given [elements] collection.
- * 
- * Before Kotlin 1.6, the [elements] collection may have been converted to a [HashSet] to speed up the operation, thus the elements were required to have
- * a correct and stable implementation of `hashCode()` that didn't change between successive invocations.
- * On JVM, you can enable this behavior back with the system property `kotlin.collections.convert_arg_to_set_in_removeAll` set to `true`.
  */
 public operator fun <T> Iterable<T>.minus(elements: Iterable<T>): List<T> {
-    val other = elements.convertToSetForSetOperationWith(this)
+    val other = elements.convertToListIfNotCollection()
     if (other.isEmpty())
         return this.toList()
     return this.filterNot { it in other }
@@ -3168,13 +3159,9 @@ public operator fun <T> Iterable<T>.minus(elements: Iterable<T>): List<T> {
 
 /**
  * Returns a list containing all elements of the original collection except the elements contained in the given [elements] sequence.
- * 
- * Before Kotlin 1.6, the [elements] sequence may have been converted to a [HashSet] to speed up the operation, thus the elements were required to have
- * a correct and stable implementation of `hashCode()` that didn't change between successive invocations.
- * On JVM, you can enable this behavior back with the system property `kotlin.collections.convert_arg_to_set_in_removeAll` set to `true`.
  */
 public operator fun <T> Iterable<T>.minus(elements: Sequence<T>): List<T> {
-    val other = elements.convertToSetForSetOperation()
+    val other = elements.toList()
     if (other.isEmpty())
         return this.toList()
     return this.filterNot { it in other }

libraries/stdlib/common/src/generated/_Sequences.kt

@@ -2610,10 +2610,6 @@ public operator fun <T> Sequence<T>.minus(element: T): Sequence<T> {
  * 
  * Note that the source sequence and the array being subtracted are iterated only when an `iterator` is requested from
  * the resulting sequence. Changing any of them between successive calls to `iterator` may affect the result.
- * 
- * Before Kotlin 1.6, the [elements] array may have been converted to a [HashSet] to speed up the operation, thus the elements were required to have
- * a correct and stable implementation of `hashCode()` that didn't change between successive invocations.
- * On JVM, you can enable this behavior back with the system property `kotlin.collections.convert_arg_to_set_in_removeAll` set to `true`.
  *
  * The operation is _intermediate_ and _stateful_.
  */
@@ -2621,8 +2617,7 @@ public operator fun <T> Sequence<T>.minus(elements: Array<out T>): Sequence<T> {
     if (elements.isEmpty()) return this
     return object: Sequence<T> {
         override fun iterator(): Iterator<T> {
-            val other = elements.convertToSetForSetOperation()
+            return this@minus.filterNot { it in elements }.iterator()
-            return this@minus.filterNot { it in other }.iterator()
         }
     }
 }
@@ -2632,17 +2627,13 @@ public operator fun <T> Sequence<T>.minus(elements: Array<out T>): Sequence<T> {
  * 
  * Note that the source sequence and the collection being subtracted are iterated only when an `iterator` is requested from
  * the resulting sequence. Changing any of them between successive calls to `iterator` may affect the result.
- * 
- * Before Kotlin 1.6, the [elements] collection may have been converted to a [HashSet] to speed up the operation, thus the elements were required to have
- * a correct and stable implementation of `hashCode()` that didn't change between successive invocations.
- * On JVM, you can enable this behavior back with the system property `kotlin.collections.convert_arg_to_set_in_removeAll` set to `true`.
  *
  * The operation is _intermediate_ and _stateful_.
  */
 public operator fun <T> Sequence<T>.minus(elements: Iterable<T>): Sequence<T> {
     return object: Sequence<T> {
         override fun iterator(): Iterator<T> {
-            val other = elements.convertToSetForSetOperation()
+            val other = elements.convertToListIfNotCollection()
             if (other.isEmpty())
                 return this@minus.iterator()
             else
@@ -2658,15 +2649,11 @@ public operator fun <T> Sequence<T>.minus(elements: Iterable<T>): Sequence<T> {
  * the resulting sequence. Changing any of them between successive calls to `iterator` may affect the result.
  * 
  * The operation is _intermediate_ for this sequence and _terminal_ and _stateful_ for the [elements] sequence.
- * 
- * Before Kotlin 1.6, the [elements] sequence may have been converted to a [HashSet] to speed up the operation, thus the elements were required to have
- * a correct and stable implementation of `hashCode()` that didn't change between successive invocations.
- * On JVM, you can enable this behavior back with the system property `kotlin.collections.convert_arg_to_set_in_removeAll` set to `true`.
  */
 public operator fun <T> Sequence<T>.minus(elements: Sequence<T>): Sequence<T> {
     return object: Sequence<T> {
         override fun iterator(): Iterator<T> {
-            val other = elements.convertToSetForSetOperation()
+            val other = elements.toList()
             if (other.isEmpty())
                 return this@minus.iterator()
             else

libraries/stdlib/common/src/generated/_Sets.kt

@@ -32,10 +32,6 @@ public operator fun <T> Set<T>.minus(element: T): Set<T> {
  * Returns a set containing all elements of the original set except the elements contained in the given [elements] array.
  * 
  * The returned set preserves the element iteration order of the original set.
- * 
- * Before Kotlin 1.6, the [elements] array may have been converted to a [HashSet] to speed up the operation, thus the elements were required to have
- * a correct and stable implementation of `hashCode()` that didn't change between successive invocations.
- * On JVM, you can enable this behavior back with the system property `kotlin.collections.convert_arg_to_set_in_removeAll` set to `true`.
  */
 public operator fun <T> Set<T>.minus(elements: Array<out T>): Set<T> {
     val result = LinkedHashSet<T>(this)
@@ -47,13 +43,9 @@ public operator fun <T> Set<T>.minus(elements: Array<out T>): Set<T> {
  * Returns a set containing all elements of the original set except the elements contained in the given [elements] collection.
  * 
  * The returned set preserves the element iteration order of the original set.
- * 
- * Before Kotlin 1.6, the [elements] collection may have been converted to a [HashSet] to speed up the operation, thus the elements were required to have
- * a correct and stable implementation of `hashCode()` that didn't change between successive invocations.
- * On JVM, you can enable this behavior back with the system property `kotlin.collections.convert_arg_to_set_in_removeAll` set to `true`.
  */
 public operator fun <T> Set<T>.minus(elements: Iterable<T>): Set<T> {
-    val other = elements.convertToSetForSetOperationWith(this)
+    val other = elements.convertToListIfNotCollection()
     if (other.isEmpty())
         return this.toSet()
     if (other is Set)
@@ -67,10 +59,6 @@ public operator fun <T> Set<T>.minus(elements: Iterable<T>): Set<T> {
  * Returns a set containing all elements of the original set except the elements contained in the given [elements] sequence.
  * 
  * The returned set preserves the element iteration order of the original set.
- * 
- * Before Kotlin 1.6, the [elements] sequence may have been converted to a [HashSet] to speed up the operation, thus the elements were required to have
- * a correct and stable implementation of `hashCode()` that didn't change between successive invocations.
- * On JVM, you can enable this behavior back with the system property `kotlin.collections.convert_arg_to_set_in_removeAll` set to `true`.
  */
 public operator fun <T> Set<T>.minus(elements: Sequence<T>): Set<T> {
     val result = LinkedHashSet<T>(this)

libraries/stdlib/js/src/kotlin/collections.kt

@@ -260,5 +260,3 @@ internal actual fun mapCapacity(expectedSize: Int) = expectedSize
 internal fun checkBuilderCapacity(capacity: Int) {
     require(capacity >= 0) { "capacity must be non-negative." }
 }
-
-internal actual fun brittleContainsOptimizationEnabled(): Boolean = false

libraries/stdlib/jvm/src/kotlin/collections/CollectionsJVM.kt

@@ -117,13 +117,3 @@ internal actual inline fun checkCountOverflow(count: Int): Int {
     }
     return count
 }
-
-
-@Suppress("NOTHING_TO_INLINE")
-internal actual inline fun brittleContainsOptimizationEnabled(): Boolean = CollectionSystemProperties.brittleContainsOptimizationEnabled
-
-internal object CollectionSystemProperties {
-    @JvmField
-    internal val brittleContainsOptimizationEnabled: Boolean =
-        System.getProperty("kotlin.collections.convert_arg_to_set_in_removeAll")?.toBoolean() ?: false
-}

libraries/stdlib/native-wasm/src/kotlin/collections/Collections.kt

@@ -133,5 +133,3 @@ internal actual inline fun checkCountOverflow(count: Int): Int {
     }
     return count
 }
-
-internal actual fun brittleContainsOptimizationEnabled(): Boolean = false

libraries/stdlib/src/kotlin/collections/BrittleContainsOptimization.kt

@@ -1,66 +0,0 @@
-/*
- * 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()

libraries/stdlib/src/kotlin/collections/MutableCollections.kt

@@ -8,8 +8,6 @@
 
 package kotlin.collections
 
-import kotlin.random.Random
-
 /**
  * Removes a single instance of the specified element from this
  * collection, if it is present.
@@ -141,33 +139,40 @@ public fun <T> MutableCollection<in T>.addAll(elements: Array<out T>): Boolean {
     return addAll(elements.asList())
 }
 
+/**
+ * Converts this [Iterable] to a list if it is not a [Collection].
+ * Otherwise, returns this.
+ */
+internal fun <T> Iterable<T>.convertToListIfNotCollection(): Collection<T> =
+    if (this is Collection) this else toList()
+
 /**
  * Removes all elements from this [MutableCollection] that are also contained in the given [elements] collection.
  */
 public fun <T> MutableCollection<in T>.removeAll(elements: Iterable<T>): Boolean {
-    return removeAll(elements.convertToSetForSetOperationWith(this))
+    return removeAll(elements.convertToListIfNotCollection())
 }
 
 /**
  * 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 {
-    val set = elements.convertToSetForSetOperation()
+    val list = elements.toList()
-    return set.isNotEmpty() && removeAll(set)
+    return list.isNotEmpty() && removeAll(list)
 }
 
 /**
  * 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 {
-    return elements.isNotEmpty() && removeAll(elements.convertToSetForSetOperation())
+    return elements.isNotEmpty() && removeAll(elements.asList())
 }
 
 /**
  * Retains only elements of this [MutableCollection] that are contained in the given [elements] collection.
  */
 public fun <T> MutableCollection<in T>.retainAll(elements: Iterable<T>): Boolean {
-    return retainAll(elements.convertToSetForSetOperationWith(this))
+    return retainAll(elements.convertToListIfNotCollection())
 }
 
 /**
@@ -175,7 +180,7 @@ public fun <T> MutableCollection<in T>.retainAll(elements: Iterable<T>): Boolean
  */
 public fun <T> MutableCollection<in T>.retainAll(elements: Array<out T>): Boolean {
     if (elements.isNotEmpty())
-        return retainAll(elements.convertToSetForSetOperation())
+        return retainAll(elements.asList())
     else
         return retainNothing()
 }
@@ -184,9 +189,9 @@ 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.
  */
 public fun <T> MutableCollection<in T>.retainAll(elements: Sequence<T>): Boolean {
-    val set = elements.convertToSetForSetOperation()
+    val list = elements.toList()
-    if (set.isNotEmpty())
+    if (list.isNotEmpty())
-        return retainAll(set)
+        return retainAll(list)
     else
         return retainNothing()
 }

libraries/tools/kotlin-stdlib-gen/src/templates/Generators.kt

@@ -367,13 +367,6 @@ object Generators : TemplateGroupBase() {
         }
     }
 
-    private fun elementsConversionClause(elements: Family) =
-            """
-            Before Kotlin 1.6, the [elements] ${elements.doc.collection} may have been converted to a [HashSet] to speed up the operation, thus the elements were required to have
-            a correct and stable implementation of `hashCode()` that didn't change between successive invocations. 
-            On JVM, you can enable this behavior back with the system property `kotlin.collections.convert_arg_to_set_in_removeAll` set to `true`.
-            """
-
     val f_minus_iterable = fn("minus(elements: Iterable<T>)") {
         include(Iterables, Sets, Sequences)
     } builder {
@@ -384,7 +377,7 @@ object Generators : TemplateGroupBase() {
         specialFor(Sets, Sequences) { returns("SELF") }
         body {
             """
-            val other = elements.convertToSetForSetOperationWith(this)
+            val other = elements.convertToListIfNotCollection()
             if (other.isEmpty())
                 return this.toList()
 
@@ -402,7 +395,7 @@ object Generators : TemplateGroupBase() {
             }
             body {
                 """
-                val other = elements.convertToSetForSetOperationWith(this)
+                val other = elements.convertToListIfNotCollection()
                 if (other.isEmpty())
                     return this.toSet()
                 if (other is Set)
@@ -429,7 +422,7 @@ object Generators : TemplateGroupBase() {
                 """
                 return object: Sequence<T> {
                     override fun iterator(): Iterator<T> {
-                        val other = elements.convertToSetForSetOperation()
+                        val other = elements.convertToListIfNotCollection()
                         if (other.isEmpty())
                             return this@minus.iterator()
                         else
@@ -440,9 +433,6 @@ object Generators : TemplateGroupBase() {
             }
 
         }
-        doc {
-            doc + elementsConversionClause(Iterables)
-        }
     }
 
     val f_minus_array = fn("minus(elements: Array<out T>)") {
@@ -456,8 +446,7 @@ object Generators : TemplateGroupBase() {
         body {
             """
             if (elements.isEmpty()) return this.toList()
-            val other = elements.convertToSetForSetOperation()
+            return this.filterNot { it in elements }
-            return this.filterNot { it in other }
             """
         }
         specialFor(Sets) {
@@ -492,16 +481,12 @@ object Generators : TemplateGroupBase() {
                 if (elements.isEmpty()) return this
                 return object: Sequence<T> {
                     override fun iterator(): Iterator<T> {
-                        val other = elements.convertToSetForSetOperation()
+                        return this@minus.filterNot { it in elements }.iterator()
-                        return this@minus.filterNot { it in other }.iterator()
                     }
                 }
                 """
             }
         }
-        doc {
-            doc + elementsConversionClause(ArraysOfObjects)
-        }
     }
 
     val f_minus_sequence = fn("minus(elements: Sequence<T>)") {
@@ -514,7 +499,7 @@ object Generators : TemplateGroupBase() {
         specialFor(Sets, Sequences) { returns("SELF") }
         body {
             """
-            val other = elements.convertToSetForSetOperation()
+            val other = elements.toList()
             if (other.isEmpty())
                 return this.toList()
 
@@ -553,7 +538,7 @@ object Generators : TemplateGroupBase() {
                 """
                 return object: Sequence<T> {
                     override fun iterator(): Iterator<T> {
-                        val other = elements.convertToSetForSetOperation()
+                        val other = elements.toList()
                         if (other.isEmpty())
                             return this@minus.iterator()
                         else
@@ -563,9 +548,6 @@ object Generators : TemplateGroupBase() {
                 """
             }
         }
-        doc {
-            doc + elementsConversionClause(Sequences)
-        }
     }
 
     val f_partition = fn("partition(predicate: (T) -> Boolean)") {