From bc3dda2b0222d3b7cf3db491728b98f9b6110856 Mon Sep 17 00:00:00 2001
From: Jason Zaugg <jzaugg@gmail.com>
Date: Sat, 29 Sep 2012 18:15:15 +0200
Subject: SI-6448 Collecting the spoils of PartialFun#runWith

Avoids calling both `isDefinedAt` and `apply`. This pathological
case that would benefit the most looks like:

  xs collect {
    case x if {expensive(); true} => x
  }

The typical change looks like:

-    for (x <- this) if (pf.isDefinedAt(x)) b += pf(x)
+    foreach(pf.runWith(b += _))

Incorporates feedback provided by Pavel Pavlov:

  https://github.com/retronym/scala/commit/ef5430

A few more opportunities for optimization are noted in the
`Pending` section of the enclosed test. `Iterator.collect`
would be nice, but a solution eludes me.

Calling the guard less frequently does change the behaviour
of these functions in an obervable way, but not contravene
the documented semantics. That said, there is an alternative
opinion on the comment of the ticket:

  https://issues.scala-lang.org/browse/SI-6448
---
 test/files/run/t6448.check | 32 ++++++++++++++++++++++++
 test/files/run/t6448.scala | 61 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 93 insertions(+)
 create mode 100644 test/files/run/t6448.check
 create mode 100644 test/files/run/t6448.scala

(limited to 'test/files/run')

diff --git a/test/files/run/t6448.check b/test/files/run/t6448.check
new file mode 100644
index 0000000000..9401568319
--- /dev/null
+++ b/test/files/run/t6448.check
@@ -0,0 +1,32 @@
+
+=List.collect=
+f(1)
+f(2)
+List(1)
+
+=List.collectFirst=
+f(1)
+Some(1)
+
+=Option.collect=
+f(1)
+Some(1)
+
+=Option.collect=
+f(2)
+None
+
+=Stream.collect=
+f(1)
+f(2)
+List(1)
+
+=Stream.collectFirst=
+f(1)
+Some(1)
+
+=ParVector.collect=
+(ParVector(1),2)
+
+=ParArray.collect=
+(ParArray(1),2)
diff --git a/test/files/run/t6448.scala b/test/files/run/t6448.scala
new file mode 100644
index 0000000000..4d1528e500
--- /dev/null
+++ b/test/files/run/t6448.scala
@@ -0,0 +1,61 @@
+// Tests to show that various `collect` functions avoid calling
+// both `PartialFunction#isDefinedAt` and `PartialFunction#apply`.
+//
+object Test {
+  def f(i: Int) = { println("f(" + i + ")"); true }
+  class Counter {
+    var count = 0
+    def apply(i: Int) = synchronized {count += 1; true}
+  }
+
+  def testing(label: String)(body: => Any) {
+    println(s"\n=$label=")
+    println(body)
+  }
+
+  def main(args: Array[String]) {
+    testing("List.collect")(List(1, 2) collect { case x if f(x) && x < 2 => x})
+    testing("List.collectFirst")(List(1, 2) collectFirst { case x if f(x) && x < 2 => x})
+    testing("Option.collect")(Some(1) collect { case x if f(x) && x < 2 => x})
+    testing("Option.collect")(Some(2) collect { case x if f(x) && x < 2 => x})
+    testing("Stream.collect")((Stream(1, 2).collect { case x if f(x) && x < 2 => x}).toList)
+    testing("Stream.collectFirst")(Stream.continually(1) collectFirst { case x if f(x) && x < 2 => x})
+
+    import collection.parallel.ParIterable
+    import collection.parallel.immutable.ParVector
+    import collection.parallel.mutable.ParArray
+    testing("ParVector.collect") {
+      val counter = new Counter()
+      (ParVector(1, 2) collect { case x if counter(x) && x < 2 => x}, counter.synchronized(counter.count))
+    }
+
+    testing("ParArray.collect") {
+      val counter = new Counter()
+      (ParArray(1, 2) collect { case x if counter(x) && x < 2 => x}, counter.synchronized(counter.count))
+    }
+
+    object PendingTests {
+      testing("Iterator.collect")((Iterator(1, 2) collect { case x if f(x) && x < 2 => x}).toList)
+
+      testing("List.view.collect")((List(1, 2).view collect { case x if f(x) && x < 2 => x}).force)
+
+      // This would do the trick in Future.collect, but I haven't added this yet as there is a tradeoff
+      // with extra allocations to consider.
+      //
+      // pf.lift(v) match {
+      //   case Some(x) => p success x
+      //   case None    => fail(v)
+      // }
+      testing("Future.collect") {
+        import concurrent.ExecutionContext.Implicits.global
+        import concurrent.Await
+        import concurrent.duration.Duration
+        val result = concurrent.future(1) collect { case x if f(x) => x}
+        Await.result(result, Duration.Inf)
+      }
+
+      // TODO Future.{onSuccess, onFailure, recoverWith, andThen}
+    }
+
+  }
+}
-- 
cgit v1.2.3