Skip to content

Commit

Permalink
Fast PartialFunction # orElse.
Browse files Browse the repository at this point in the history
  • Loading branch information
odersky committed Nov 24, 2011
1 parent 32a7535 commit 5fb26c6
Show file tree
Hide file tree
Showing 11 changed files with 32 additions and 24 deletions.
2 changes: 1 addition & 1 deletion src/actors/scala/actors/Actor.scala
Expand Up @@ -247,7 +247,7 @@ object Actor extends Combinators {

private class RecursiveProxyHandler(a: ReplyReactor, f: PartialFunction[Any, Unit])
extends scala.runtime.AbstractPartialFunction[Any, Unit] {
def isDefinedAt(m: Any): Boolean =
def _isDefinedAt(m: Any): Boolean =
true // events are immediately removed from the mailbox
def apply(m: Any) {
if (f.isDefinedAt(m)) f(m)
Expand Down
2 changes: 1 addition & 1 deletion src/actors/scala/actors/Future.scala
Expand Up @@ -201,7 +201,7 @@ object Futures {

def awaitWith(partFuns: Seq[PartialFunction[Any, Pair[Int, Any]]]) {
val reaction: PartialFunction[Any, Unit] = new scala.runtime.AbstractPartialFunction[Any, Unit] {
def isDefinedAt(msg: Any) = msg match {
def _isDefinedAt(msg: Any) = msg match {
case TIMEOUT => true
case _ => partFuns exists (_ isDefinedAt msg)
}
Expand Down
2 changes: 1 addition & 1 deletion src/actors/scala/actors/Reactor.scala
Expand Up @@ -39,7 +39,7 @@ private[actors] object Reactor {
}

val waitingForNone: PartialFunction[Any, Unit] = new scala.runtime.AbstractPartialFunction[Any, Unit] {
def isDefinedAt(x: Any) = false
def _isDefinedAt(x: Any) = false
def apply(x: Any) {}
}
}
Expand Down
1 change: 1 addition & 0 deletions src/compiler/scala/reflect/internal/Flags.scala
Expand Up @@ -96,6 +96,7 @@ class ModifierFlags {
final val INTERFACE = 0x00000080 // symbol is an interface (i.e. a trait which defines only abstract methods)
final val MUTABLE = 0x00001000 // symbol is a mutable variable.
final val PARAM = 0x00002000 // symbol is a (value or type) parameter to a method
final val MACRO = 0x00008000 // symbol is a macro definition

final val COVARIANT = 0x00010000 // symbol is a covariant type variable
final val BYNAMEPARAM = 0x00010000 // parameter is by name
Expand Down
10 changes: 5 additions & 5 deletions src/compiler/scala/tools/cmd/FromString.scala
Expand Up @@ -16,7 +16,7 @@ import scala.reflect.OptManifest
*/
abstract class FromString[+T](implicit m: OptManifest[T]) extends scala.runtime.AbstractPartialFunction[String, T] {
def apply(s: String): T
def isDefinedAt(s: String): Boolean = true
def _isDefinedAt(s: String): Boolean = true
def zero: T = apply("")

def targetString: String = m.toString
Expand All @@ -30,20 +30,20 @@ object FromString {
/** Path related stringifiers.
*/
val ExistingFile: FromString[File] = new FromString[File] {
override def isDefinedAt(s: String) = toFile(s).isFile
override def _isDefinedAt(s: String) = toFile(s).isFile
def apply(s: String): File =
if (isDefinedAt(s)) toFile(s)
else cmd.runAndExit(println("'%s' is not an existing file." format s))
}
val ExistingDir: FromString[Directory] = new FromString[Directory] {
override def isDefinedAt(s: String) = toDir(s).isDirectory
override def _isDefinedAt(s: String) = toDir(s).isDirectory
def apply(s: String): Directory =
if (isDefinedAt(s)) toDir(s)
else cmd.runAndExit(println("'%s' is not an existing directory." format s))
}
def ExistingDirRelativeTo(root: Directory) = new FromString[Directory] {
private def resolve(s: String) = toDir(s) toAbsoluteWithRoot root toDirectory
override def isDefinedAt(s: String) = resolve(s).isDirectory
override def _isDefinedAt(s: String) = resolve(s).isDirectory
def apply(s: String): Directory =
if (isDefinedAt(s)) resolve(s)
else cmd.runAndExit(println("'%s' is not an existing directory." format resolve(s)))
Expand All @@ -65,7 +65,7 @@ object FromString {
/** Implicit as the most likely to be useful as-is.
*/
implicit val IntFromString: FromString[Int] = new FromString[Int] {
override def isDefinedAt(s: String) = safeToInt(s).isDefined
override def _isDefinedAt(s: String) = safeToInt(s).isDefined
def apply(s: String) = safeToInt(s).get
def safeToInt(s: String): Option[Int] = try Some(java.lang.Integer.parseInt(s)) catch { case _: NumberFormatException => None }
}
Expand Down
Expand Up @@ -7,7 +7,7 @@ package scala.tools.nsc
package interpreter

class AbstractOrMissingHandler[T](onError: String => Unit, value: T) extends scala.runtime.AbstractPartialFunction[Throwable, T] {
def isDefinedAt(t: Throwable) = t match {
def _isDefinedAt(t: Throwable) = t match {
case _: AbstractMethodError => true
case _: NoSuchMethodError => true
case _: MissingRequirementError => true
Expand Down
Expand Up @@ -433,7 +433,7 @@ trait ParallelMatching extends ast.TreeDSL
case (false, false) => pivotLen == x.nonStarLength
}

def isDefinedAt(pat: Pattern) = pat match {
def _isDefinedAt(pat: Pattern) = pat match {
case x: SequenceLikePattern => seqIsDefinedAt(x)
case WildcardPattern() => true
case _ => false
Expand Down
2 changes: 1 addition & 1 deletion src/library/scala/Function.scala
Expand Up @@ -41,7 +41,7 @@ object Function {
*/
def unlift[T, R](f: T => Option[R]): PartialFunction[T, R] = new runtime.AbstractPartialFunction[T, R] {
def apply(x: T): R = f(x).get
def isDefinedAt(x: T): Boolean = f(x).isDefined
def _isDefinedAt(x: T): Boolean = f(x).isDefined
override def lift: T => Option[R] = f
}

Expand Down
7 changes: 4 additions & 3 deletions src/library/scala/PartialFunction.scala
Expand Up @@ -41,7 +41,7 @@ trait PartialFunction[-A, +B] extends (A => B) {
*/
def orElse[A1 <: A, B1 >: B](that: PartialFunction[A1, B1]) : PartialFunction[A1, B1] =
new runtime.AbstractPartialFunction[A1, B1] {
def isDefinedAt(x: A1): Boolean =
def _isDefinedAt(x: A1): Boolean =
PartialFunction.this.isDefinedAt(x) || that.isDefinedAt(x)
def apply(x: A1): B1 =
if (PartialFunction.this.isDefinedAt(x)) PartialFunction.this.apply(x)
Expand All @@ -59,7 +59,7 @@ trait PartialFunction[-A, +B] extends (A => B) {
* arguments `x` to `k(this(x))`.
*/
override def andThen[C](k: B => C) : PartialFunction[A, C] = new runtime.AbstractPartialFunction[A, C] {
def isDefinedAt(x: A): Boolean = PartialFunction.this.isDefinedAt(x)
def _isDefinedAt(x: A): Boolean = PartialFunction.this.isDefinedAt(x)
def apply(x: A): C = k(PartialFunction.this.apply(x))
}

Expand Down Expand Up @@ -90,7 +90,8 @@ trait PartialFunction[-A, +B] extends (A => B) {
*/
object PartialFunction {
private[this] final val empty_pf: PartialFunction[Any, Nothing] = new runtime.AbstractPartialFunction[Any, Nothing] {
def isDefinedAt(x: Any) = false
def _isDefinedAt(x: Any) = false
override def isDefinedAt(x: Any) = false
def apply(x: Any): Nothing = throw new MatchError(x)
override def orElse[A1, B1](that: PartialFunction[A1, B1]): PartialFunction[A1, B1] = that
override def orElseFast[A1, B1](that: PartialFunction[A1, B1]): PartialFunction[A1, B1] = that
Expand Down
24 changes: 15 additions & 9 deletions src/library/scala/runtime/AbstractPartialFunction.scala
Expand Up @@ -23,24 +23,30 @@ abstract class AbstractPartialFunction[-T1, +R]
with PartialFunction[T1, R]
with Cloneable {

private var fallBack: PartialFunction[T1 @uncheckedVariance, R @uncheckedVariance] = PartialFunction.empty
private var fallBackField: PartialFunction[T1 @uncheckedVariance, R @uncheckedVariance] = _

override protected def missingCase(x: T1): R = synchronized {
fallBack(x)
def fallBack: PartialFunction[T1, R] = synchronized {
if (fallBackField == null) fallBackField = PartialFunction.empty
fallBackField
}

override protected def missingCase(x: T1): R = fallBack(x)

// Question: Need to ensure that fallBack is overwritten before any access
// Is the `synchronized` here the right thing to achieve this?
// Is there a cheaper way?
override def orElseFast[A1 <: T1, B1 >: R](that: PartialFunction[A1, B1]) : PartialFunction[A1, B1] = {
override def orElse[A1 <: T1, B1 >: R](that: PartialFunction[A1, B1]) : PartialFunction[A1, B1] = {
val result = this.clone.asInstanceOf[AbstractPartialFunction[A1, B1]]
result.synchronized {
result.fallBack = this.fallBack orElseFast that
result.fallBackField = this.fallBackField orElse that
result
}
}
/*
def isDefinedAt(x: T1): scala.Boolean = isDefinedAtCurrent(x) || fallBack.isDefinedAt(x)
def isDefinedAtCurrent(x: T1): scala.Boolean = false
*/

def isDefinedAt(x: T1): scala.Boolean = _isDefinedAt(x) || fallBack.isDefinedAt(x)
def _isDefinedAt(x: T1): scala.Boolean

}



2 changes: 1 addition & 1 deletion src/library/scala/util/control/Exception.scala
Expand Up @@ -232,6 +232,6 @@ object Exception {
private def pfFromExceptions(exceptions: Class[_]*): PartialFunction[Throwable, Nothing] =
new scala.runtime.AbstractPartialFunction[Throwable, Nothing] {
def apply(x: Throwable) = throw x
def isDefinedAt(x: Throwable) = wouldMatch(x, exceptions)
def _isDefinedAt(x: Throwable) = wouldMatch(x, exceptions)
}
}

2 comments on commit 5fb26c6

@pavelpavlov
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, this implementation seems to be broken for me.

  1. All the non-compiler-generated subclasses of AbstractPartialFunction
    must call missingCase from apply unless its _isDefnedAt always returns true or its orElse is redefined.
    Otherwise PFs returned by orElse on them will be broken.
    With quick grep on nightly built scala-libray-src.jar & scala-compiler-src.jar I found 8 such broken classes:
  • Function.unlift
  • PartialFunction.orElse (non-optimized)
  • PartialFunction.andThen
  • actors.Futures.awaitWith
  • actors.Reactor.waitingForNone
  • util.control.Exception.pfFromExceptions
  • tools.nsc.interpreter.AbstractOrMissingHandler
  • tools.nsc.matching.ParalleMatching.scala, lazy val successMatrixFn

A few other comments on the code:

  1. // Question: Need to ensure that fallBack is overwritten before any access
    // Is the synchronized here the right thing to achieve this?
    // Is there a cheaper way?

It is enough to declare fallBackField field as volatile.
As you have not yet exposed just cloned object (in orElse) to anyone,
it is safe to write into volatile fallBackField field then let the object escape.
In that case JMM guarantees that anyone will see right value of the field here.
Moreover, to simplify overall scheme, it is better to remove assignment to the field from the accessor
(moving it back to constructor as in previous commit).
With this you can eliminate both synchronized blocks from the code.

  1. It seems that orElseFast is not used anywhere.

@nafg
Copy link
Contributor

@nafg nafg commented on 5fb26c6 Jan 18, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this commit add ModifierFlags.MACRO?

Please sign in to comment.