From 9e265f5c5482a5d98ce24a341f5318be662ab14f Mon Sep 17 00:00:00 2001 From: Viktor Klang Date: Wed, 13 Jun 2012 10:48:54 +0200 Subject: [PATCH 1/7] Proposal to make it possible to fully discard the receive and replace it with become, unbecome then reverts to receive if no behavior left --- .../src/main/scala/akka/actor/ActorCell.scala | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/akka-actor/src/main/scala/akka/actor/ActorCell.scala b/akka-actor/src/main/scala/akka/actor/ActorCell.scala index 9dbe610195..893c81ac91 100644 --- a/akka-actor/src/main/scala/akka/actor/ActorCell.scala +++ b/akka-actor/src/main/scala/akka/actor/ActorCell.scala @@ -184,7 +184,7 @@ private[akka] object ActorCell { final val emptyReceiveTimeoutData: (Long, Cancellable) = (-1, emptyCancellable) - final val behaviorStackPlaceHolder: Stack[Actor.Receive] = Stack.empty.push(Actor.emptyBehavior) + final val behaviorStackPlaceHolder: Stack[Actor.Receive] = Stack.empty final val emptyActorRefSet: Set[ActorRef] = TreeSet.empty @@ -521,10 +521,9 @@ private[akka] class ActorCell( if (instance eq null) throw new ActorInitializationException(self, "Actor instance passed to actorOf can't be 'null'") - behaviorStack = behaviorStack match { - case `behaviorStackPlaceHolder` ⇒ Stack.empty.push(instance.receive) - case newBehaviors ⇒ Stack.empty.push(instance.receive).pushAll(newBehaviors.reverse.drop(1)) - } + // If no becomes were issued, the actors behavior is its receive method + if (behaviorStack eq behaviorStackPlaceHolder) + behaviorStack = Stack.empty.push(instance.receive) instance } finally { val stackAfter = contextStack.get @@ -683,10 +682,8 @@ private[akka] class ActorCell( } } - def become(behavior: Actor.Receive, discardOld: Boolean = true): Unit = { - if (discardOld) unbecome() - behaviorStack = behaviorStack.push(behavior) - } + def become(behavior: Actor.Receive, discardOld: Boolean = true): Unit = + behaviorStack = (if (discardOld && behaviorStack.nonEmpty) behaviorStack.pop else behaviorStack).push(behavior) /** * UntypedActorContext impl @@ -701,8 +698,8 @@ private[akka] class ActorCell( def unbecome(): Unit = { val original = behaviorStack - val popped = original.pop - behaviorStack = if (popped.isEmpty) original else popped + behaviorStack = if (original.isEmpty || original.pop.isEmpty) Stack.empty[Actor.Receive].push(actor.receive) + else original.pop } def autoReceiveMessage(msg: Envelope): Unit = { From b1fe6c709c783c0351c6c7c6195d7381bbc8901e Mon Sep 17 00:00:00 2001 From: Viktor Klang Date: Wed, 13 Jun 2012 10:55:47 +0200 Subject: [PATCH 2/7] Formatting --- akka-actor/src/main/scala/akka/actor/ActorCell.scala | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/akka-actor/src/main/scala/akka/actor/ActorCell.scala b/akka-actor/src/main/scala/akka/actor/ActorCell.scala index 893c81ac91..285c31b49d 100644 --- a/akka-actor/src/main/scala/akka/actor/ActorCell.scala +++ b/akka-actor/src/main/scala/akka/actor/ActorCell.scala @@ -698,8 +698,9 @@ private[akka] class ActorCell( def unbecome(): Unit = { val original = behaviorStack - behaviorStack = if (original.isEmpty || original.pop.isEmpty) Stack.empty[Actor.Receive].push(actor.receive) - else original.pop + behaviorStack = + if (original.isEmpty || original.pop.isEmpty) Stack.empty.push(actor.receive) + else original.pop } def autoReceiveMessage(msg: Envelope): Unit = { From 6199556ced894b8cad7ed791cec435e6fc4716cb Mon Sep 17 00:00:00 2001 From: Viktor Klang Date: Wed, 13 Jun 2012 11:39:04 +0200 Subject: [PATCH 3/7] Caching emptyBehaviorStack and remove all other uses of Stack.empty --- .../src/main/scala/akka/actor/ActorCell.scala | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/akka-actor/src/main/scala/akka/actor/ActorCell.scala b/akka-actor/src/main/scala/akka/actor/ActorCell.scala index 285c31b49d..d750b4964b 100644 --- a/akka-actor/src/main/scala/akka/actor/ActorCell.scala +++ b/akka-actor/src/main/scala/akka/actor/ActorCell.scala @@ -184,7 +184,7 @@ private[akka] object ActorCell { final val emptyReceiveTimeoutData: (Long, Cancellable) = (-1, emptyCancellable) - final val behaviorStackPlaceHolder: Stack[Actor.Receive] = Stack.empty + final val emptyBehaviorStack: Stack[Actor.Receive] = Stack.empty final val emptyActorRefSet: Set[ActorRef] = TreeSet.empty @@ -408,7 +408,7 @@ private[akka] class ActorCell( var currentMessage: Envelope = _ var actor: Actor = _ - private var behaviorStack: Stack[Actor.Receive] = Stack.empty + private var behaviorStack: Stack[Actor.Receive] = emptyBehaviorStack @volatile var _mailboxDoNotCallMeDirectly: Mailbox = _ //This must be volatile since it isn't protected by the mailbox status var nextNameSequence: Long = 0 var watching: Set[ActorRef] = emptyActorRefSet @@ -513,17 +513,16 @@ private[akka] class ActorCell( protected def newActor(): Actor = { contextStack.set(contextStack.get.push(this)) try { - import ActorCell.behaviorStackPlaceHolder + import ActorCell.emptyBehaviorStack - behaviorStack = behaviorStackPlaceHolder + behaviorStack = emptyBehaviorStack val instance = props.creator.apply() if (instance eq null) throw new ActorInitializationException(self, "Actor instance passed to actorOf can't be 'null'") // If no becomes were issued, the actors behavior is its receive method - if (behaviorStack eq behaviorStackPlaceHolder) - behaviorStack = Stack.empty.push(instance.receive) + behaviorStack = if (behaviorStack.isEmpty) behaviorStack.push(instance.receive) else behaviorStack instance } finally { val stackAfter = contextStack.get @@ -699,7 +698,7 @@ private[akka] class ActorCell( def unbecome(): Unit = { val original = behaviorStack behaviorStack = - if (original.isEmpty || original.pop.isEmpty) Stack.empty.push(actor.receive) + if (original.isEmpty || original.pop.isEmpty) emptyBehaviorStack.push(actor.receive) else original.pop } @@ -759,7 +758,7 @@ private[akka] class ActorCell( if (system.settings.DebugLifecycle) system.eventStream.publish(Debug(self.path.toString, clazz(a), "stopped")) } finally { - behaviorStack = behaviorStackPlaceHolder + behaviorStack = emptyBehaviorStack clearActorFields(a) actor = null } From d3e2f529f3bdcd9098223e3fe5b92e35b8da8773 Mon Sep 17 00:00:00 2001 From: Viktor Klang Date: Wed, 13 Jun 2012 11:53:27 +0200 Subject: [PATCH 4/7] Removing a pointless import and the only Scala return statement in our codebase --- .../src/main/scala/akka/actor/ActorCell.scala | 2 -- .../src/main/scala/akka/testkit/TestKit.scala | 28 ++++++++++--------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/akka-actor/src/main/scala/akka/actor/ActorCell.scala b/akka-actor/src/main/scala/akka/actor/ActorCell.scala index d750b4964b..39158b239d 100644 --- a/akka-actor/src/main/scala/akka/actor/ActorCell.scala +++ b/akka-actor/src/main/scala/akka/actor/ActorCell.scala @@ -513,8 +513,6 @@ private[akka] class ActorCell( protected def newActor(): Actor = { contextStack.set(contextStack.get.push(this)) try { - import ActorCell.emptyBehaviorStack - behaviorStack = emptyBehaviorStack val instance = props.creator.apply() diff --git a/akka-testkit/src/main/scala/akka/testkit/TestKit.scala b/akka-testkit/src/main/scala/akka/testkit/TestKit.scala index c0fb6e5267..4a5a880bb0 100644 --- a/akka-testkit/src/main/scala/akka/testkit/TestKit.scala +++ b/akka-testkit/src/main/scala/akka/testkit/TestKit.scala @@ -486,19 +486,21 @@ trait TestKitBase { @tailrec def doit(acc: List[T], count: Int): List[T] = { - if (count >= messages) return acc.reverse - receiveOne((stop - now) min idle) - lastMessage match { - case NullMessage ⇒ - lastMessage = msg - acc.reverse - case RealMessage(o, _) if (f isDefinedAt o) ⇒ - msg = lastMessage - doit(f(o) :: acc, count + 1) - case RealMessage(o, _) ⇒ - queue.offerFirst(lastMessage) - lastMessage = msg - acc.reverse + if (count >= messages) acc.reverse + else { + receiveOne((stop - now) min idle) + lastMessage match { + case NullMessage ⇒ + lastMessage = msg + acc.reverse + case RealMessage(o, _) if (f isDefinedAt o) ⇒ + msg = lastMessage + doit(f(o) :: acc, count + 1) + case RealMessage(o, _) ⇒ + queue.offerFirst(lastMessage) + lastMessage = msg + acc.reverse + } } } From 2d4067e21e410e6d70b408169be842be1440de34 Mon Sep 17 00:00:00 2001 From: Viktor Klang Date: Wed, 13 Jun 2012 13:56:54 +0200 Subject: [PATCH 5/7] Skipping immutable.Stack due to questionable implementation, going for immutable.List instead --- .../src/main/scala/akka/actor/Actor.scala | 8 ++----- .../src/main/scala/akka/actor/ActorCell.scala | 22 +++++++++---------- 2 files changed, 13 insertions(+), 17 deletions(-) diff --git a/akka-actor/src/main/scala/akka/actor/Actor.scala b/akka-actor/src/main/scala/akka/actor/Actor.scala index 2721ccffa0..cf35d68c8c 100644 --- a/akka-actor/src/main/scala/akka/actor/Actor.scala +++ b/akka-actor/src/main/scala/akka/actor/Actor.scala @@ -279,18 +279,14 @@ trait Actor { */ protected[akka] implicit val context: ActorContext = { val contextStack = ActorCell.contextStack.get - - def noContextError = + if ((contextStack.isEmpty) || (contextStack.head eq null)) throw new ActorInitializationException( "\n\tYou cannot create an instance of [" + getClass.getName + "] explicitly using the constructor (new)." + "\n\tYou have to use one of the factory methods to create a new actor. Either use:" + "\n\t\t'val actor = context.actorOf(Props[MyActor])' (to create a supervised child actor from within an actor), or" + "\n\t\t'val actor = system.actorOf(Props(new MyActor(..)))' (to create a top level actor from the ActorSystem)") - - if (contextStack.isEmpty) noContextError val c = contextStack.head - if (c eq null) noContextError - ActorCell.contextStack.set(contextStack.push(null)) + ActorCell.contextStack.set(null :: contextStack) c } diff --git a/akka-actor/src/main/scala/akka/actor/ActorCell.scala b/akka-actor/src/main/scala/akka/actor/ActorCell.scala index 39158b239d..72793513e2 100644 --- a/akka-actor/src/main/scala/akka/actor/ActorCell.scala +++ b/akka-actor/src/main/scala/akka/actor/ActorCell.scala @@ -13,7 +13,7 @@ import akka.japi.Procedure import java.io.{ NotSerializableException, ObjectOutputStream } import akka.serialization.SerializationExtension import akka.event.Logging.LogEventException -import collection.immutable.{ TreeSet, Stack, TreeMap } +import collection.immutable.{ TreeSet, TreeMap } import akka.util.{ Unsafe, Duration, Helpers, NonFatal } //TODO: everything here for current compatibility - could be limited more @@ -173,8 +173,8 @@ trait UntypedActorContext extends ActorContext { * for! (waves hand) */ private[akka] object ActorCell { - val contextStack = new ThreadLocal[Stack[ActorContext]] { - override def initialValue = Stack[ActorContext]() + val contextStack = new ThreadLocal[List[ActorContext]] { + override def initialValue: List[ActorContext] = Nil } final val emptyCancellable: Cancellable = new Cancellable { @@ -184,7 +184,7 @@ private[akka] object ActorCell { final val emptyReceiveTimeoutData: (Long, Cancellable) = (-1, emptyCancellable) - final val emptyBehaviorStack: Stack[Actor.Receive] = Stack.empty + final val emptyBehaviorStack: List[Actor.Receive] = Nil final val emptyActorRefSet: Set[ActorRef] = TreeSet.empty @@ -408,7 +408,7 @@ private[akka] class ActorCell( var currentMessage: Envelope = _ var actor: Actor = _ - private var behaviorStack: Stack[Actor.Receive] = emptyBehaviorStack + private var behaviorStack: List[Actor.Receive] = emptyBehaviorStack @volatile var _mailboxDoNotCallMeDirectly: Mailbox = _ //This must be volatile since it isn't protected by the mailbox status var nextNameSequence: Long = 0 var watching: Set[ActorRef] = emptyActorRefSet @@ -511,7 +511,7 @@ private[akka] class ActorCell( //This method is in charge of setting up the contextStack and create a new instance of the Actor protected def newActor(): Actor = { - contextStack.set(contextStack.get.push(this)) + contextStack.set(this :: contextStack.get) try { behaviorStack = emptyBehaviorStack val instance = props.creator.apply() @@ -520,12 +520,12 @@ private[akka] class ActorCell( throw new ActorInitializationException(self, "Actor instance passed to actorOf can't be 'null'") // If no becomes were issued, the actors behavior is its receive method - behaviorStack = if (behaviorStack.isEmpty) behaviorStack.push(instance.receive) else behaviorStack + behaviorStack = if (behaviorStack.isEmpty) instance.receive :: behaviorStack else behaviorStack instance } finally { val stackAfter = contextStack.get if (stackAfter.nonEmpty) - contextStack.set(if (stackAfter.head eq null) stackAfter.pop.pop else stackAfter.pop) // pop null marker plus our context + contextStack.set(if (stackAfter.head eq null) stackAfter.tail.tail else stackAfter.tail) // pop null marker plus our context } } @@ -680,7 +680,7 @@ private[akka] class ActorCell( } def become(behavior: Actor.Receive, discardOld: Boolean = true): Unit = - behaviorStack = (if (discardOld && behaviorStack.nonEmpty) behaviorStack.pop else behaviorStack).push(behavior) + behaviorStack = behavior :: (if (discardOld && behaviorStack.nonEmpty) behaviorStack.tail else behaviorStack) /** * UntypedActorContext impl @@ -696,8 +696,8 @@ private[akka] class ActorCell( def unbecome(): Unit = { val original = behaviorStack behaviorStack = - if (original.isEmpty || original.pop.isEmpty) emptyBehaviorStack.push(actor.receive) - else original.pop + if (original.isEmpty || original.tail.isEmpty) actor.receive :: emptyBehaviorStack + else original.tail } def autoReceiveMessage(msg: Envelope): Unit = { From d6e3642d9d79c6b80377c68189ba23daaeb63048 Mon Sep 17 00:00:00 2001 From: Viktor Klang Date: Wed, 13 Jun 2012 14:08:47 +0200 Subject: [PATCH 6/7] Removing all uses of immutable.Stack in Akka --- .../src/main/scala/akka/actor/Actor.scala | 1 - .../src/main/scala/akka/actor/ActorSystem.scala | 17 ++++++++--------- .../src/main/scala/akka/actor/Props.scala | 1 - .../main/scala/akka/testkit/TestActorRef.scala | 2 -- 4 files changed, 8 insertions(+), 13 deletions(-) diff --git a/akka-actor/src/main/scala/akka/actor/Actor.scala b/akka-actor/src/main/scala/akka/actor/Actor.scala index cf35d68c8c..8fc7df93e5 100644 --- a/akka-actor/src/main/scala/akka/actor/Actor.scala +++ b/akka-actor/src/main/scala/akka/actor/Actor.scala @@ -7,7 +7,6 @@ package akka.actor import akka.AkkaException import scala.reflect.BeanProperty import scala.util.control.NoStackTrace -import scala.collection.immutable.Stack import java.util.regex.Pattern /** diff --git a/akka-actor/src/main/scala/akka/actor/ActorSystem.scala b/akka-actor/src/main/scala/akka/actor/ActorSystem.scala index 721375adda..c874d75afc 100644 --- a/akka-actor/src/main/scala/akka/actor/ActorSystem.scala +++ b/akka-actor/src/main/scala/akka/actor/ActorSystem.scala @@ -13,7 +13,6 @@ import java.io.Closeable import akka.dispatch.Await.{ Awaitable, CanAwait } import akka.util._ import akka.util.internal.{ HashedWheelTimer, ConcurrentIdentityHashMap } -import collection.immutable.Stack import java.util.concurrent.{ ThreadFactory, CountDownLatch, TimeoutException, RejectedExecutionException } import java.util.concurrent.TimeUnit.MILLISECONDS @@ -685,8 +684,8 @@ private[akka] class ActorSystemImpl(val name: String, applicationConfig: Config, final class TerminationCallbacks extends Runnable with Awaitable[Unit] { private val lock = new ReentrantGuard - private var callbacks: Stack[Runnable] = _ //non-volatile since guarded by the lock - lock withGuard { callbacks = Stack.empty[Runnable] } + private var callbacks: List[Runnable] = _ //non-volatile since guarded by the lock + lock withGuard { callbacks = Nil } private val latch = new CountDownLatch(1) @@ -695,17 +694,17 @@ private[akka] class ActorSystemImpl(val name: String, applicationConfig: Config, case 0 ⇒ throw new RejectedExecutionException("Must be called prior to system shutdown.") case _ ⇒ lock withGuard { if (latch.getCount == 0) throw new RejectedExecutionException("Must be called prior to system shutdown.") - else callbacks = callbacks.push(callback) + else callbacks ::= callback } } } final def run(): Unit = lock withGuard { - @tailrec def runNext(c: Stack[Runnable]): Stack[Runnable] = c.headOption match { - case None ⇒ Stack.empty[Runnable] - case Some(callback) ⇒ - try callback.run() catch { case e ⇒ log.error(e, "Failed to run termination callback, due to [{}]", e.getMessage) } - runNext(c.pop) + @tailrec def runNext(c: List[Runnable]): List[Runnable] = c match { + case Nil ⇒ Nil + case callback :: _ ⇒ + try callback.run() catch { case NonFatal(e) ⇒ log.error(e, "Failed to run termination callback, due to [{}]", e.getMessage) } + runNext(c.tail) } try { callbacks = runNext(callbacks) } finally latch.countDown() } diff --git a/akka-actor/src/main/scala/akka/actor/Props.scala b/akka-actor/src/main/scala/akka/actor/Props.scala index fc01a5ba36..82d97f5465 100644 --- a/akka-actor/src/main/scala/akka/actor/Props.scala +++ b/akka-actor/src/main/scala/akka/actor/Props.scala @@ -6,7 +6,6 @@ package akka.actor import akka.dispatch._ import akka.japi.Creator -import collection.immutable.Stack import akka.routing._ /** diff --git a/akka-testkit/src/main/scala/akka/testkit/TestActorRef.scala b/akka-testkit/src/main/scala/akka/testkit/TestActorRef.scala index ed151b6b12..f8efe4e2e5 100644 --- a/akka-testkit/src/main/scala/akka/testkit/TestActorRef.scala +++ b/akka-testkit/src/main/scala/akka/testkit/TestActorRef.scala @@ -5,9 +5,7 @@ package akka.testkit import akka.actor._ -import akka.util.Duration import java.util.concurrent.atomic.AtomicLong -import scala.collection.immutable.Stack import akka.dispatch._ import akka.pattern.ask From 6d114fb3e2a7db4067c8cdfed79cb27d7074e938 Mon Sep 17 00:00:00 2001 From: Viktor Klang Date: Wed, 13 Jun 2012 15:14:51 +0200 Subject: [PATCH 7/7] Review fixes --- akka-actor/src/main/scala/akka/actor/ActorSystem.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/akka-actor/src/main/scala/akka/actor/ActorSystem.scala b/akka-actor/src/main/scala/akka/actor/ActorSystem.scala index c874d75afc..0c5be77889 100644 --- a/akka-actor/src/main/scala/akka/actor/ActorSystem.scala +++ b/akka-actor/src/main/scala/akka/actor/ActorSystem.scala @@ -702,9 +702,9 @@ private[akka] class ActorSystemImpl(val name: String, applicationConfig: Config, final def run(): Unit = lock withGuard { @tailrec def runNext(c: List[Runnable]): List[Runnable] = c match { case Nil ⇒ Nil - case callback :: _ ⇒ + case callback :: rest ⇒ try callback.run() catch { case NonFatal(e) ⇒ log.error(e, "Failed to run termination callback, due to [{}]", e.getMessage) } - runNext(c.tail) + runNext(rest) } try { callbacks = runNext(callbacks) } finally latch.countDown() }