Merge pull request scala#7708 from joshlemer/set-filterInPlace

Optimize s.c.m.Set#filterInPlace

Author: diesalbla

src/library/scala/collection/mutable/Set.scala

@@ -83,8 +83,18 @@ trait SetOps[A, +CC[X], +C <: SetOps[A, CC, C]]
     *             are removed.
     */
   def filterInPlace(p: A => Boolean): this.type = {
-    for (elem <- this.toList) // scala/bug#7269 toList avoids ConcurrentModificationException
-      if (!p(elem)) this -= elem
+    if (nonEmpty) {
+      val array = this.toArray[Any] // scala/bug#7269 toArray avoids ConcurrentModificationException
+      val arrayLength = array.length
+      var i = 0
+      while (i < arrayLength) {
+        val elem = array(i).asInstanceOf[A]
+        if (!p(elem)) {
+          this -= elem
+        }
+        i += 1
+      }
+    }
     this
   }
 

test/scalacheck/scala/collection/mutable/SetProperties.scala

@@ -0,0 +1,54 @@
+/*
+ * Scala (https://www.scala-lang.org)
+ *
+ * Copyright EPFL and Lightbend, Inc.
+ *
+ * Licensed under Apache License 2.0
+ * (http://www.apache.org/licenses/LICENSE-2.0).
+ *
+ * See the NOTICE file distributed with this work for
+ * additional information regarding copyright ownership.
+ */
+
+package scala.collection.mutable
+
+import org.scalacheck._
+import org.scalacheck.Prop._
+import org.scalacheck.Arbitrary.arbitrary
+import Gen._
+
+
+object SetProperties extends Properties("mutable.Set") {
+
+  type Elem = Int
+
+  /** returns a set that certainly does not override `filterInPlace` implementation */
+  def testSet(elems: collection.immutable.Set[Elem]): Set[Elem] = new Set[Elem] {
+    override protected[this] def className: String = "TestSet"
+    private[this] var _elems = elems
+    override def iterator: Iterator[Elem] = _elems.iterator
+    override def clear(): Unit = _elems = collection.immutable.Set.empty
+    override def subtractOne(elem: Elem): this.type = { _elems -= elem; this }
+    override def addOne(elem: Elem): this.type = { _elems += elem; this }
+    override def contains(elem: Elem): Boolean = { _elems.contains(elem) }
+  }
+
+  implicit val arbSet: Arbitrary[Set[Elem]] =
+    Arbitrary {
+      for {
+        elems <- arbitrary[collection.immutable.HashSet[Elem]]
+        set <- oneOf(
+          elems.to(HashSet),
+          elems.to(TreeSet),
+          elems.to(LinkedHashSet),
+          testSet(elems)
+        )
+      } yield set
+    }
+
+  property("filterInPlace(p)") = forAll { (set: Set[Elem], p: Elem => Boolean) =>
+    val expected: collection.Set[Elem] = set.to(collection.immutable.HashSet).filter(p)
+    set.filterInPlace(p)
+    (set: collection.Set[Elem]) ?= expected
+  }
+}