akkadotnet / akka.net

Canonical actor model implementation for .NET with local + distributed actors in C# and F#.
http://getakka.net
Other
4.73k stars 1.04k forks source link

Implement stashing #301

Closed HCanber closed 10 years ago

HCanber commented 10 years ago

We have stashing somewhat implemented. However it does not work as unstashing appends to the mailbox queue instead of prepends.

public virtual void UnstashInternal()
{
    ...
    EnqueueFirst(TheStash.Head());
    ...
}

public virtual void EnqueueFirst(Envelope msg)
{
    //TODO: need to add double-ended queue semantics for this to work
    ActorCell.Mailbox.Post(msg);
}

https://github.com/akkadotnet/akka.net/blob/dev/src/core/Akka/Actor/Stash.cs#L393

We need to add a mailbox that has this ability and make sure that it is used when the actor wants stash support.

In Akka JVM this is mainly handled in

We have the abstract class Mailbox and ConcurrentQueueMailbox that uses ConcurrentQueue.

Akka JVM also has Mailbox (a trait) that takes a MessageQueue in the constructor. There are many implementations of MessageQueue. Akka JVM has MailboxType which is a factory for creating mailboxes. It is a trait, and the concrete classes are the ones specified in the config.

The dispatcher is responsible for creating a mailbox.

Should we use the same separation?

Aaronontheweb commented 10 years ago

The Stash implementation I put together initially is very quick and dirty - needed it for a bunch of the system actors responsible for remoting (namely the protocol state actors), since they have to stash incoming messages while they wait for the handshake process to finish.

The biggest issue that needs to be fixed, as you point out, is being able to prepend messages to the front of the mailbox so we can try to preserve the order of the messages. Not possible to do at the moment without changing the mailbox implementation itself.

As for the canonical Akka implementation, I originally tried implementing this structure in Hyperion before I joined forces with Roger to work on Akka.NET. The original Akka implementation is very complicated. I don't know why they implemented it the way they did - maybe some of it was for performance optimization or backwards compatibility with older versions. Or maybe the person who wrote it had a strong passion for traits of traits of interfaces.

I broke it down like this when I was thinking about your discussion area this morning:

  1. Should mailboxes that support stashing be required to have a double-ended queue? or some other structure for supporting message prepending. Given that we're probably going to be introducing some new types of mailboxes such as #299, probably worth discussing how that plays out generically across types of mailboxes that don't exist yet, such as the PriorityMailbox and maybe DurableMailbox.
  2. Should all mailboxes support stashing by default? Simplifies a lot of things if you just make stash support a requirement for all mailbox implementations. Is there a good reason not to have stash support on a mailbox? Performance, maybe, if stashing support comes with a high overhead cost even when it's not being used. But this is a question worth exploring.

I don't have a strong opinion on question 1 just yet, but I'm leaning towards "yes" for question 2.

I feel like it's less expensive to just make mailboxes that always support stashing versus implementing (1) mailboxes that optionally do or don't support stashing, (2) configuration overhead for specifying what type of underlying data structure to use for the mailbox, (3) validation work to ensure that actors who require stashing don't get configured to use a mailbox or queue that doesn't support stashing, and (4) re-implementing the entire mailbox creation process.

HCanber commented 10 years ago

After going over Akka JVM's implementation over and over again for hours, I actually think I got it sorted out. :) (Note: I think :). As you say, there are a traits upon traits... I think I can simplify it a bit, so I'll take a stab at it.

In their implementation, there is only one Mailbox used (well, actually one all-purpose, and three used for specific purposes: SharingMailbox for BalancingDispatcher; DeadletterMailbox; CallingThreadMailbox for CallingThreadDispatcher) but many MessageQueue implementations. These are rather small and easy to implement.

  1. Should mailboxes that support stashing be required to have a double-ended queue? They (meaning the underlying MessageQueue) must support EnqueueFirst(), otherwise unstashing will be in the wrong order and not prepended first.
  2. Should all mailboxes support stashing by default? No, the user must be able to create it's own kind of hyperfast mailbox (or more correctly: MessageQueue) if they want to. Or something where stashing is totatally irrelevant.

Regarding your remarks:

  1. mailboxes that optionally do or don't support stashing. The Mailbox does not know of the concept stashing. The MessageQueue do. Creating one that do not support stashing (ie one that uses concurrentqueue) and one that do (using a double linked list for example) is pretty easy I think. A MessageQueue only has 5 members (6 when stashing), see https://github.com/akka/akka/blob/master/akka-actor/src/main/scala/akka/dispatch/Mailbox.scala#L322
  2. configuration overhead for specifying what type of underlying data structure to use for the mailbox As long as you stick with the default stashable actors this will be handled automatically for you. This is handled in the reference.conf (pigeon.conf). Just inherit from a stashable actor base class and you're good to go. Only need to configure if you want to use your own.
  3. Validation work to ensure that actors who require stashing don't get configured to use a mailbox or queue that doesn't support stashing This is handled in one place, and is an easy check if the MessageQueue implements DequeBasedMessageQueueSemantics.
  4. Re-implementing the entire mailbox creation processThis is the hardest one, but I think it will be worth it.

I'll make a try and see how it goes. :)

Aaronontheweb commented 10 years ago

Go for it - if you can simplify items 1-4, then it's probably worthwhile making the tradeoffs.

mattnischan commented 10 years ago

Does the Deque need to be thread-safe? I see that the JVM implementation is using BlockingDeque in some places, and a normal Deque in others, and I'm not entirely sure why.

Isn't there always the possibility that multiple writers (including the actor itself, in the stashing case) will be appending or prepending to the queue at the same time?

Aaronontheweb commented 10 years ago

@mattnischan I think the answer to that question depends on the mailbox implementation.

Aaronontheweb commented 10 years ago

This is not implemented - any further stashing changes should be discussed on #429

Aaronontheweb commented 10 years ago

This is now implemented, whoops.