Akka.net: Discussion: How to provide better support for async/await inside actors

[nvivo]

This issue is to discuss how to better support async/await inside actors.

Currently, all actors are synchronous and support for Tasks is achieved using PipeTo/Tell. Most of .NET nowadays makes heavy use of tasks, so even simple things like writing files and accessing databases can be async. It seems to me that Akka.NET should support async/await receive methods without requiring users to implement this logic by breaking the processing into multiple messages with PipeTo.

The "async" support shouldn't break the synchronous model where one message is processed at a time. That means that any implementation of an "async Receive method" should somehow wait for the "Receive" method to finish its processing (possibly awaiting for the task) before processing a new message. This behavior allows any mix of implementations like using "async/await" but also allowing "PipeTo/Tell" where needed.

I have been thinking about how to support this, and here are some thoughts about the problem and some proposed solutions to discuss. I must say my knowledge of the project code is nowhere good enough to provide a complete list, and as @rogeralsing said, it must support the ActorTaskScheduler, but I hope this at least helps with the research.

Solution 1 - Implementing Async versions for each actor

Pros:

• Can be achieved with no changes to existing code, thus less risk of breaking things
• Allows a very simple syntax, usually requiring the user to implement some form of Task<bool> ReceiveAsync instead of bool Receive

Cons:

• Requires stashing support to prevent blocking, leaving the user without option to choose which stash to use
• Requires implementing (and maintaining) repetitive pattern for each existing actor

There is a sample implementation of an async actor here: https://github.com/akkadotnet/akka.net/pull/660. Also, not all actors require a new class. Both FSM and ReceiveActor can be implemented by just adding methods to the existing actors (ReceiveAsync/WhenAsync).

Actors like EventStream or Logger shouldn't require async implementations in my opinion, thus making the total of only 3 implementations (4 if Handler/TypedActor should be included) to fully support the official Actors. This seems to be the easiest way to achieve better async support today.

Solution 2 - Extensions methods for ActorBase

Pros:

• Exposes a single implementation that could in theory work with any actor

Cons:

• Still requires stashing support
• Would require users to add method calls to their implementations, making it more complex to use
• Different actors have different ways to implement Receive, which seems to indicate multiple extension implementations would be required anyway

I tried this solution, but any implementation I could think of would be at least as complex to maintain, and more complex to use than solution 1.

Solution 3 - Change ActorBase to support Task directly

Pros:

• Removes the requirement for stashing, leaving the user free to use it only if needed
• Creates a standard way to handle tasks for all actors

Cons:

• Requires changes to a lot of existing code
• Would move the await logic to mailboxes, turning that code more complex and possibly requiring multiple implementations
• May require either change the signature of current actor methods or implement different async actors, but these would be only to expose the correct signatures, not to implement the logic

This one seems the "most correct" way to introduce async actors, but I agree would be unfeasible in the near future due to the amount of changes this requires. From what I have seen, this change would require the Akka.Actor.Receive delegate to return something other than "bool" and start refactoring everything from this place up.

This could go two ways - make it return directly Task<bool>, and emulate synchronous implementation returning a completed task (.NET 4.6 seems to make this a little bit cheaper), or make it return object, and possibly avoid a tasks at all by checking for null/bool for non-async implementations.

In this case, most of the "await" logic would move to the mailboxes, which would not dispatch any new messages to actors while the previous task didn't finish. The nice thing about this solution is that it really prevents messages from arriving while awaiting, removing completely the necessity of stashing.

In the case of the ConcurrentQueueMailbox, it would involve doing something in this loop, possibly SpinWaiting for some time or even really awaiting, which would probably require "Run" to be async to avoid more complexity.

In Short

Async support is a must-have for new .NET projects and Akka should support a better story than breaking messages with PipeTo.

A good async support without stashing will require changes to the core of Akka. Solution 3 seems to me more like an Akka 2.0 roadmap than something that should be done now.

Providing async implementations like the solution 1 may be enough to most people and may feasible for 1.0 or maybe 1.1, but still requires more testing, which I'd be glad to do, but I still need to get more familiar with TestKit.

If I'm missing something, feel free to speak up.

[Aaronontheweb]

It seems to me that Akka.NET should support async/await receive methods without requiring users to implement this logic by breaking the processing into multiple messages with PipeTo.

I read through this entire post; you never explained "why" ?

Actors are already inherently asynchronous and have a simpler, more reliable async model than async / await does - what's so hard about adapting the TPL to use a simple extension method (PipeTo) ?

If the issue really distills down to "well, we use async everywhere else so why not in Akka.NET" - then let's take a step back. The actor model is an entirely different way of programming than traditional OOP - even though Akka.NET lets you do it in C#.

Developers who want to use the actor model have to think in terms of messages and actors - adding await and having the actor execute its OnReceive method in multiple areas concurrently / asynchronously is really something that lies outside the intent of the Actor model. Those await callbacks really should be messages.

[rogeralsing]

The easiest, safest and possibly with the least impact, would be to add a method like RunTask inside ActorBase that schedules the task using the ActorTaskScheduler
Then one could easily run tasks from within an actor and have safe semantics.

public class MyActor : UntypedActor
{
     protected override void OnReceive(object message)
     {
           if (message is MyMessage)
           {
                 //schedule a task on the actor task scheduler
                 RunTask(async () => {
                        await Task.Delay(TimeSpan.FromSeconds(10));
                        //restore the actor context
                        //and do stuff 
                 });
                //no blocking, return here
           }
     }
}

It might even be a good idea to specify semantics in there, if the tasks should run with re-entrancy or pause mailbox.

[nvivo]

@Aaronontheweb,

Okay, maybe I thought that part was implicit from my PR, and it's not. =)

In short, this discussion has more to do with being able to deal with non-blocking IO calls than concurrent processing.

Imagine for example a simple actor that:

1. Receives a message
2. Goes to the database to read or write data
3. Sets some state in the actor depending on the database result
4. Responds the sender and goes to next message

You could go to the database and then pipe to yourself, but you still need to handle the fact that you cannot process anything until you set your state after the database call, and keep stashing things to process in the correct order. This is very simple flow, but most real code today do more than a single call to a task-returning method. Take HttpClient for example, it's entire API returns tasks, and any new API from .NET will follow this pattern.

I agree that a lot of asynchronous processing should be done by message passing, but forcing _every_ type of async processing to be a message is just the old hammer problem, and making everything a nail.

In the solution I'm discussing, you are not using tasks because you want to process things in parallel and get away of the actor model. You are using tasks because all the .NET API is returning tasks, and all your business logic is using tasks because of that as well. And C# already has a way to handle tasks and synchronize back the code, and that is the async/await pattern.

Async/await is a perfect complement for akka because it frees threads and allow non-blocking IO. People can do concurrent processing just as they can do with PipeTo, but there is nothing wrong with supporting the pattern.

[nvivo]

@rogeralsing,

This solution is the simplest, but it's blocking a thread for as long as you have the processing. That's what I'm trying to avoid.

[rogeralsing]

No it would not be blocking, it would schedule the task on the task scheduler and then return.
When the task is completed, the await continuation will safely run in our actor context

This would however be re-entrant, that is, other messages can be processed while awaiting, but never process two messages at a time.

There are two approaches to tasks in actors, re-entrant and pausing the mailbox.

[nvivo]

@rogeralsing ok, so that is another solution I couldn't see!
That would be a nice solution, and probably easier to implement than my solution 1.

Edit: I'm quite curious though on how you would make the RunTask block without blocking. =)

[rogeralsing]

@nvivo
We do all the magic inside the task scheduler.
So you could do stuff like this kind of orchestrations safely:

 RunTask(async () => {
                       var t1 = Task.Delay(TimeSpan.FromSeconds(10));
                       var t2 = SomeOtherTask();

                       await Task.WhenAll(t1,t2, otherActor.Ask(...));

                       //Do stuff
                 });

[Aaronontheweb]

You could go to the database and then pipe to yourself, but you still need to handle the fact that you cannot process anything until you set your state after the database call

This is what Stashing and Switchable Behaviors are for. I've used this very pattern in production at significant scale with Akka.NET.

But here's the thing - await-ing the database call inside an actor, from _purely a timing standpoint_, semantically no different than just processing its results as a message. From a state management standpoint it's disastrous. Modifying an actor's state concurrently using async methods breaks every single guarantee about an actor's internal state being automatically thread safe.

The day people start using locks and concurrent collections inside actors is when everything goes to hell. await is not magic. It's just syntactic sugar on top of the TPL.

I agree that a lot of asynchronous processing should be done by message passing, but forcing every type of async processing to be a message is just the old hammer problem, and making everything a nail.

You have this backwards - you're trying to fit async and await on top of a model that doesn't support it. Not the other way around.

[nvivo]

This would however be re-entrant, that is, other messages can be processed while awaiting, but never process two messages at a time.

But the point of async actors is to prevent any other messages from being processed while the task is not finished.

The thing is that almost anything I write in .NET nowadays involve a call to an Async method. That may be because the API is really asyncrhonous, or because my interfaces returns tasks. Most of my real actor code involves calls to 5 or 6 async methods.

RunTask seems to solve one problem, and that is that currently you need to do:

var self = Self;
var sender = Sender;

Task.Run(async () => {
   var t1 = Task.Delay(TimeSpan.FromSeconds(10));
   var t2 = SomeOtherTask();

   await Task.WhenAll(t1, t2);

   sender.Tell(new Result(t1, t2), self)
});

It doesn't solve the problem that you need the state set after await Task.WhenAll(t1, t2); to process the next request.

[nvivo]

From a state management standpoint it's disastrous.

I think we are not talking the same thing. There is absolutely nothing inherent to async/await that breaks actor state. You insist on modifying the actor concurrently, and this solution has nothing to do with that.

await-ing the database call inside an actor, from purely a timing standpoint, semantically no different than just processing its results as a message

It's not awaiting for purely timing. It's freeing a thread. If I need to do an http call and that call takes 5 seconds to come back, the thread shouldn't be blocked. But the main point is that in some cases, I shouldn't process new requests either. The next request needs to wait for the first one to be processed, which is one of the main advertisements of Akka: the actor state can be kept consistent.

It seems we are talking apples and oranges here. =) Again, it has nothing to do with concurrent execution. It has to do with non-blocking IO.

It seems that by your definition, it is acceptable to block a thread for 5 seconds doing nothing, but not blocking it seems to break some paradigm, which seems backwards.

[Aaronontheweb]

@nvivo

You insist on modifying the actor concurrently, and this solution has nothing to do with that.

You do this in point 3

1. Sets some state in the actor depending on the database result

Unless the actor is blocking while it waits (in which case there's no need for await at all), this inherently means modifying the actor's state concurrently while it's doing something else.

[nvivo]

@Aaronontheweb,

You insist on modifying the actor concurrently, and this solution has nothing to do with that.

You do this in point #3

No, I don't. You are the one trying to see that. =)

Let me try to give an example. If my actor code is:

object value;

void Receive(object message) {
    if (value == null)
        value  = CreateValue();

    var result = Process(message, value);

    Sender.Tell(result);
}

If CreateValue takes 3 seconds of actual CPU processing at full speed (and let's ignore why it takes 3 seconds, or any pre-processing alternative). This code seems ok, right?

Now, let's change CreateValue to make an Http call, and from those 3 seconds are actually waiting for some network data. The code is exactly the same. This somehow becomes wrong?

It's exactly the same code, but because my implementation changed, suddenly I'm forced to use PipeTo and Stashing, break the processing into multiple messages and keep a lot of code checking message types, errors, etc?

This makes no sense. And that's because there is already a better alternative, and that is:

object value;

async Task Receive(object message) {
    if (value == null)
        value  = await CreateValue();

    var result = Process(message, value);

    Sender.Tell(result);
}

Since the idea is that this won't process any new messages until the method finishes, note:

1. There is no concurrent processing anywhere, at no moment there are two threads modifying the actor
2. The actor state is kept intact, exactly like the first example
3. The thread is free to run other things

What is wrong with this idea?

[rogeralsing]

(Just for clarity, @Aaronontheweb , @nvivo's implementation stashes any incoming message untill the task completes. So in his case, there is no concurrent message processing, and no blocking)

[Aaronontheweb]

I don't have any more time to discuss this today (I'm interested! But, deadlines :), but I'm just going to link to some source from Petabridge's PipeTo sample:

//Command to begin downloading an image
            Receive<DownloadImage>(image => 
            {
                 SendMessage(string.Format("Beginning download of img {0} for feed {1}", image.ImageUrl, image.FeedUri));

                //check for relative URLs
                var imageUrl = image.ImageUrl;
                if (!Uri.IsWellFormedUriString(image.ImageUrl, UriKind.Absolute))
                {
                    var baseAddress = new Uri(image.FeedUri);

                    //Combine the base address and relative URL of image to form an absolute one.
                    imageUrl = string.Format("{0}://{1}{2}", baseAddress.Scheme, baseAddress.Host, imageUrl);
                }

                //asynchronously download the image and pipe the results to ourself
                _httpClient.GetAsync(imageUrl).ContinueWith(httpRequest =>
                {
                    var response = httpRequest.Result;

                    //successful img download - which happened in a DIFFERENT THREAD
                    if (response.StatusCode == HttpStatusCode.OK)
                    {
                        // async call, inside an async call
                        // and we wait on it...
                        // BUT THIS IS STILL ASYNCHRONOUS?!?!
                        // INSERT INCEPTION HORN SOUND EFFECT HERE https://www.youtube.com/watch?v=ZKGJZt83_JE
                        var contentStream = response.Content.ReadAsStreamAsync();
                        try
                        {
                            contentStream.Wait(TimeSpan.FromSeconds(1));
                            return new ImageDownloadResult(image, response.StatusCode, contentStream.Result);
                        }
                        catch //timeout exceptions!
                        {
                            return new ImageDownloadResult(image, HttpStatusCode.PartialContent);
                        }
                    }

                    return new ImageDownloadResult(image, response.StatusCode);
                }, TaskContinuationOptions.AttachedToParent & TaskContinuationOptions.ExecuteSynchronously).PipeTo(Self);
            });

A single instance of this actor can process 100s of downloads in parallel; never blocks; and does post-processing inline (on the same thread that the HttpRequest returns on, different from the main actor's) before it returns the message result to itself. It even kicks off a second asynchronous task inside itself. And it could easily pipe these results back to a different actor.

The other vibe I'm starting to get from reading your answers is that you've coupled your state and your commands in a couple of ways that should be refactored.

This HttpDownloaderActor I've linked to, for instance, has no mutable on purpose. It processes commands - that's all it does. All of the actual state related to the batch processing it does belongs to its parent actor, whose only job is to collect and monitor state from its children.

Every time I've had dedicated actors for flushing data to a database, for instance, they themselves maintain zero state other than their database connection. They run commands on behalf of actors who _do have state_ and send the responses back to them asynchronously, whether it uses the TPL or not.

You should do the same - it sounds like what you've tried to do is bring your pre-Akka.NET designs and drop them into actors as-is. A better way to do it is to follow decouple state from operations. It's safer, for one thing, but it also gives you a more extensible, cleaner design.

[rogeralsing]

@Aaronontheweb what do you mean in the code below, that you wait but it is still async?

contentStream.Wait(TimeSpan.FromSeconds(1));
return new ImageDownloadResult(image, response.StatusCode, contentStream.Result);

You are clearly blocking a thread, for up to one sec there.
even if it is a background thread.
Maybe the stream reference is quick to resolve and that task completes fast, but still that is what is going on as far as I can tell.
One thread in the treadpool is beeing hogged.

Or am I missing something?

[nvivo]

Just for reference if someone else is reading it:

I'm proposing supporting async/await pattern inside Receive method because that is an useful language feature for a lot of reasons. It makes it easy to reason about methods that returns tasks, and makes it simpler to handle exceptions from them.

I'm not proposing adding blocking patterns, neither concurrency inside actors, as those are different things. In fact, I'm proposing a way to make Akka more non-blocking for some cases where it would block unnecessarily. Any other direction in this discussion means something was misunderstood.

There seems to be some focus on parallel processing and actor blocking in the discussion, and although it is possible to do those things, the async/await pattern is a language feature, just as a foreach loop, and there is nothing intrinsic to it that breaks any assumptions or patterns from Akka.

It is also possible to break a lot of Akka model by using PipeTo incorrectly. It's up to the developer to use these tools correctly.

I won't keep forcing something the team clearly doesn't want in the framework. If that's the case we can close the issue and move on.

[Aaronontheweb]

@rogeralsing yessr, it blocks - for not very long. It's just an in-memory buffer copy operation.

It's asynchronous because it's already running on a separate thread, woken up by the OS as the result of an I/O completion port: https://msdn.microsoft.com/en-us/library/windows/desktop/aa365198(v=vs.85).aspx

If I did some crazy TPL unwrapping to add that step as a continuation to the previous it would do the same thing but need a context switch and create a new I/O completion port to do the work itself, which is overkill for an in-memory operation.

Did it because of tradeoff 2 on this list: http://www.aaronstannard.com/tradeoffs-in-high-performance-software/

[rogeralsing]

Ok, so if this had been anorher senario and the operation was eg a DB call, you would had designed it differently, right?

Isnt there a risk in showing blocking ops w/o explaining why that approach was taken?

In this case you knew about the internal behavior and optimized based on that info.

[rogeralsing]

I think this is a good discussion, no matter what the outcome will be.

So, @Aaronontheweb , @nvivo

[Big Edit]
What if there was a task extension called Receive ?
And it has the semantics of registering a one time message receiver for this specific message flow.

Actors can alter how they handle the next message, that is one of the three axioms that makes up an actor.
So if we compose a flow of tasks, and explicitly tell that this flow should be handled with this special receiver once all completed.
That should be semantically sane in terms of actors, right?

e.g.

void OnReceive(object message)
{
   ...
   var t1 = someActor.Ask("foo").ContinueWith(r => file.WriteTextAsync(....);
   var t2 = SomeOtherTask(.,)

   //schedule execution using the actor task scheduler for safe state changes for Self
   Task
       .WhenAll(t1,t2)
       .Receive(message=> { //one time receive for the message produced by this task flow
          this.SomeState = res.xyz;
       });

This carries the exact same semantics as PipeTo, but relying on the "become" axiom of actors.
And thus, eliminating the need to create extra message types (which in all honesty can be a bit cumbersome)

It would be easier to debug, because you would be able to see why you arrive in this receive method that is, you can see what tasks was involved in arriving at the point where you are now.

It doesn't break or abuse any conceptual stuff in the actor model.

Discuss :)

[nvivo]

@rogeralsing,

The thing I'm worried about in this discussion is that all your fears from async/await seem to do only with creating tasks, not with awaiting them. Async/await has nothing to do with how you start a tasks, but only how you wait for them to finish.

And I'm saying that because in Aaron's example, it can be done with async/await in a simpler way without breaking any optimization he is trying to achieve. You can use async/await and have all tasks running synchronously in the same thread with no context switching if you want. And your example can also deadlock today without async/await. You could have:

actor1 -- (ask --> actor2).Wait()
actor2 -- (ask --> actor1).Wait()

You can argue that it's a bad pattern, but people will do it anyway. It's like you are saying "foreach" is wrong, but "for" is fine, and both things are the same. Any bad pattern you are worried about with async/await can already be achieved today just because tasks _exist_.

You need also to consider that Aaron's example is too simple. Most .NET APIs today are returning tasks nowadays. It is already becoming impossible to do any real work without having to deal with 5 or 10 tasks. The API I'm using returns tasks for everything. It is impractical to break any request into 10 different messages just for the sake of it, and it brings no performance improvements doing so, just complexity.

About the "Flow", If I understood, PipeTo is what it is today, while PipeToExecute would schedule the code in the actor context. Is that it? In this case, isn't this achieved by your actor task scheduler by using simple tasks? I'm probably missing something, but what would a new Flow class bring to the table?

[rogeralsing]

The flow class was just an example of safely encapsulating the task stuff inside something else that is not related to actors, and then consuming that something from within your actor.
That is, moving concurrency out of the actor and only pass in the result of that async flow.

Sort of like

1. Do async work/flow control
2. Pass the result to an actor

Maybe it was a bad example, but by doing so, all moving volatile parts run somewhere else, not being able to corrupt actor state.
I think it could be a decent pattern to use to avoid most of the issues we speak of here.

I'm all with you in your example

object value;

void Receive(object message) {
    if (value == null)
        value  = CreateValue();

    var result = Process(message, value);

    Sender.Tell(result);
}

vs.

object value;

async Task Receive(object message) {
    if (value == null)
        value  = await CreateValue();

    var result = Process(message, value);

    Sender.Tell(result);
}

I totally agree that that _could_ mean the same thing.. 100%

But there are two conflicting ways to implement this:
Reentrant vs. suspend mailbox/processing

Reentrant scales better, it is also pretty much just sugar ontop of PipeTo
But could be perceived as somewhat strange that messages can arrive and be processed while the await is waiting.

Suspend mailbox maps 1 to 1 to how something would work using an synchronous flow inside the actor. just like your example shows.
_but_ async operation tend to be longer running, e.g. db calls, writing to files or whatever.
By promoting this design, users might get the impression that it is a good idea to have long running tasks inside of an actor, and have the mailbox suspended from processing other messages meanwhile.
This is also more prone to deadlocking than any other approach, because it could happen w/o doing any Wait or Result calls.

I absolutely see benefits in ease of use in both cases.
But we have to very very carefully decide what to add to the framework itself, what our scope is.
And if the benefit outweigh the drawbacks of doing so.

I do believe that you are right that users will try to use async await no matter what we say.
So personally, I would like to provide them with the safest way we can for doing so, even if it is not the idiomatic way.
At least, they will get up and running and be able to adopt the framework.

I guess we have to brainstorm around this internally.
:thought_balloon: :thought_balloon: :thought_balloon: :thought_balloon:
:person_with_blond_hair: :boy: :man: :older_man:

[nvivo]

I think this is the purpose of this post, to start some discussion about this, not to implement it right away.

One final point I'd like to make is that Akka will not add anything to the framework either way. Akka will either allow people to use a very common pattern that is endorsed by the language or try to prevent it. And by trying, it will ultimately fail.

Non-blocking is about keeping threads free to work instead of awaiting while doing nothing, it shouldn't be about forcing actors to process messages even if they don't need to. Stashing is a solution, but it's more like a hack to fit in the current model.

As you said, reentrant scales better and should be the default path. But non-reentrant is an option too for some cases, and I found cases where that is what I want to do. I find it amusing that in Aaron's example, its acceptable to block a thread for 1 second waiting for some file to be read, but freeing the actor thread to do something else is unacceptable.

I hope I got some discussion started, and if some more use cases are needed, I'd be happy to provide.

[nvivo]

Just for reference, I'm providing contrib nuget package for the proof-of-concept implementation, just in case poeple want to try it out: https://www.nuget.org/packages/Akka.Contrib.AsyncActors/

[Aaronontheweb]

@rogeralsing and I discussed some ideas for this and here's what we came up with:

Here's what we propose:

1. Async / await is still considered possibly harmful within actors - it hides a lot of the details of the actor's processing model that it shouldn't and hides some of the powerful TPL patterns that can be combined with Akka.NET. Developers will still be encouraged to use PipeTo as the "correct" way of interacting with the TPL in Akka.NET.
2. That being said, we want to let developers who understand how the actor model and TPL work to make their code more concise with async / await. So we're going to expose a way of automatically enabling async / await in all actors under the /user guardian by default.
3. This will be done through configuration- haven't settled on the details yet but you'll be able to have all user-defined actors automatically created with async support turned on by enabling a setting inside your HOCON configuration. You'll also be able to enable this only for specifically deployed actors rather than all of them. This setting will remain off by default due to the 50% throughput penalty it introduces, although it should be noted that in real life this penalty will have way less impact on your app's responsiveness than the work your actors actually do for 99% of apps.

What do you think @nvivo ?

[nvivo]

I think this is great that you could discuss this in such a short notice. As I said before, I imagined this would be something for the 2.0 roadmap.

I'm intrigued on how this would be turned on by a setting or what affect it would have. One of the things I'm proposing is that "async/await" should be a method choice, not an actor choice. In my implementation, there are cases where I mix both in the same actor.

Per Roger's idea before, there would be a RunTask that would schedule any task to continue in the actor thread.

void Receive(object msg) {
    RunTask(async() => {
         // Receive ends immediatelly, but task continuation happens
         // in the actor thread after other messages are processed
    });
}  

To support an await without modifying the actor signature, it seems you are suggesting something like:

void Receive(object msg) {
    AwaitTask(async() => {
          // Receive ends immediatelly, but no new messages are
          // processed until this task finishes
    });
}  

Did I understand correctly? This looks like a nice solution that fits into any actor.

Some questions:

1. What happens if the setting is off? Would AwaitTask behave like RunTask? Wouldn't this create a kind on "inconsistency" where a method like "AwaitTask" does not wait for anything by default?
2. Or are thinking about changing RunTask behavior based on a setting per actor? If so, I don't think it's a good idea because that would force this behavior on the entire actor, while it would be more useful per method. I liked Roger's idea that you can run something in the actor context later without breaking consistency and still being reentrant.
3. What timeframe are you thinking for this? Something post 1.0?

[HCanber]

I started reading everything, but don't have time to read everything in detail, so please correct me if I'm wrong. And as I payback I write quite a lengthy one myself. :)

If I understand you @nvivo correctly you want to be able to turn this code ( a complete example would actually use the state variable for something):

``` C#
bool Receive(object message)
{
var result = DoSomeIOAsync().Result; //BAD idea, Do not block inside an actor
var transformed = TransformResult(result);
_previousTransformed = transformed; //Store state
Sender.Tell(transformed);
}

``` C#
bool Receive(object message) //Ignore the invalid signature of this function
{
  var result = await DoSomeIOAsync();
  var transformed = TransformResult(result);
  _previousTransformed = transformed; //Store state
  Sender.Tell(transformed);
}

with the same semantics, i.e. the actor should not process any messages while awaiting.

My opinion
I think that's very reasonable, and it would simplify code hugely. I don't think using async/await inside actors are harmful per se, it's just harmful right now since we don't support it correctly (i.e. we allow messages to be executed simultaneously).

I think it's more harmful that the two examples above behaves completely differently today.

I don't see any problems with having long running tasks inside an actor, as long as you're aware of that no messages are processed during that time, which might be exactly what you want, and really is in line with the synchronous version. For example, if you want to restrict usages of a resource you might want the actor to be blocked, but you don't want to block a thread just for this, and implementing this with PipeTo, ContinueWith or a state machine is not a pleasant option, although doable

So I think support for async/await should be the default, and if you want reentrant async actors, you would have to kick off tasks manually.

Implementation
This would be pretty simple to implement, if Akka would have been more functional. Us relying on the storing the current cell in a thread static field in InternalCurrentActorCellKeeper is the greatest flaw with Akka (for both JVM and .NET) today, in my opinion. If we can find a way to not rely on it (removing implicit senders for example and passing the context around), I think this can be achieved by doing this:

• Add a new interface IActor that has the function Task Receive(object message, IActorContext context)
• The semantics of Receive are:
  
  
  
  • returns null for synchronous handlers, the mailbox continues to process messages directly (as today).
  
  
  • returns a Task for async handlers. The mailbox remains Busy until the Task is completed. I.e no other messages can be processed until the current message has been completely handled.
  
  
  • we don't use the returned bool in the current version to anything useful other than logging if the message was handled or not. This functionality can be pushed down to the implementors of IActor.
  
  
• As the context is sent to Receive we don't need to use any funky thread statics to get hold of the context, so no thread context needs to be restored.
• ActorBase is changed so that it stores the context in the Context property so it will be available when returning after an await. It also provides backwards compatibility.
• As the core would depend on a small interface, instead of ActorBase other base classes that supports more functional programming style can be created.

If we can't get rid of InternalCurrentActorCellKeeper then we have to rely on restoring the context after awaiting, which I understand comes with a cost.

TImeframe and remove implicit sender for v1
I think it might be hard to get this in place any time soon, but I think we seriously should consider if we should remove implicit sender in v1, to prepare for a more functional core in the future. We can provide the method ActorBase.Tell(ICanTell receiver, object message, ActorRef sender=null) which defaults sender to this for a a nice api for the users, so instead of receiver.Tell(message) the user would use Tell(receiver, message).

[rogeralsing]

The reason we have the thread static context is not only for implicit sender.
It is also for initialization of member fields.
Eg

Class myactor{
  private ActorRef child = Context.ActorOf...

There are ofcourse better ways to init children, but as the original strategy was to stay as close as possible to the Java api. This was needed.

Im also in favor for the functional aporoach.
But that feels somewhat v2-3

[HCanber]

Im also in favor for the functional aporoach.
But that feels somewhat v2-3

Yes, agree. But the greatest change for user's then would be that someActor.Tell(message) would break. So I suggest we remove support for implicit sender, i.e. we remove public void Tell(object message) from ActorRef now before user's have a huge codebase they would have to go through when upgrading to v2.

[nvivo]

@HCanber, I think you got it perfectly! And you have more internal knowledge than me to know what it entails. Your solution is what I was trying to convey on my solution 3, where something internally should be changed to accommodate this correctly.

.NET is a different platform than JVM, and while it doesn't have all functional features from scala, it has somewhat better support for non-blocking IO than java has. IMHO this should be seen as a way to have a better Akka in C#, not a way to break Akka model.

One thing to note here is that there shouldn't be noticeable overhead by supporting async/await by default. I'm curious to see what would cause 50% even in worst cases in the scenarios you are thinking. This is because async/await by itself doesn't cause that. All hit is caused by code running in other threads, context switching but mostly OS thread scheduling. Any real overhead you can think of using async/await will impact scenarios using PipeTo as well, and will decrease message throughput in the same way.

To demonstrate that, see this sample benchmark: https://gist.github.com/nvivo/75b710b621ea2eb2b490

What I'm showing here that the state machine and additional checks added just by "supporting" async await and checking completed tasks are for any practical reasons unnoticeable. Any real code running in the CPU, including a simple log call will dwarf the overhead of the state machine by a long shot.

Either the solution proposed by @HCanber or returning a completed Task<bool> shouldn't affect Akka's performance that way. It will add a very small memory footprint by using tasks, but then it can be optimized to not even check for tasks in sync cases.

[rogeralsing]

@nvivo the cost comes from LogicalCallContext.get/set data.
As Håkan points out. We do have some implicit state, this needs to be set before any task is started.
We need this to
1 set the correct state of the actor when the continuations run (sender et al)
2 to make tasks pipe back to the owner actor so we dont break actor concurrency

[nvivo]

@rogeralsing, what you say implies that async/await causes continuations, which it doesn't. This seems to be the nature of all the confusion.

For all intents and purposes, async/await is 100% synchronous code. It doesn't schedule new tasks by itself.

That means that for Akka you never have to set LogicalCallContext unless you need to. And that's the beauty of the async/await mechanism. You will only pay the price if you actually have to, in other cases it's transparent.

That's why I say that any overhead caused by async/await will appear in PipeTo as well. Because it really is the same thing.

[rogeralsing]

Im not following.
I wrote the task scheduler, on every await, TPL schedules a task,when that completes we need to set our context

[rogeralsing]

There is a whole swamp of things in the details.
Eg, async await also captures logical call context, so if it is not present in advace. TPL will revert back to the empty context when it executes the block of code after await.
(Based on executioncontext.capture )

Even if we ever get a more functional approach, we still need a task scheduler to prevent concurrency from breaking.
(Unless we always suspend mailbox and dont support multiple call flows going on at once, which would make it impossible to do certain aggregations)

Take a look inside the ActorTaskScheduler

Feel free to modify it, if you get the AsyncAwaitSpec to pass w/o a custom scheduler or callcontext, Im buying you beer :-)

[nvivo]

I noticed that there is some confusion in what async/await really does.

There are 3 things to realize here:

1. tasks represent a promise that may or not be resolved, it doesn't necessarily represent a code that runs in another thread
2. await can be synchronous with Task.FromResult, TaskCompletionSource or when a task is already completed
3. async/await has only to do with how you check for tasks to complete, not how you start them

That means that:

var value = await task;

means something like this:

if (task.IsCompleted) {
    value = task.Result
}
else {
    task.ContinueWith(t => { 
        value = t.Result;
    });
}

Although it is really a state machine, and not a series of ContinueWith.

If a task is completed, doesn't matter if it was completed by another thread or was created with Task.FromResult, await shortcuts that with a synchronous code in the same thread, so there is never a context switch.

That means that context switch happens because some task was started in another thread and needs to sync back, not because it's awaited.

That also means that this code only has one context switch on v1, not 5:

var task = DoSomethingAsync();
var v1 = await task;
var v2 = await task;
var v3 = await task;
var v4 = await task;
var v5 = await task;

In short, you have context switches because you start tasks, not because you wait for them to complete. And async/await has only to do with waiting for them to complete. If you never asked to start something in another thread, you never have context switches. If you did, you will have them anyway, even with PipeTo.

Makes sense?

[rogeralsing]

Absolutely, but consider this:

Receive<Foo>(f =>
{
        // We have context here
        // Sender, CurrentMessage is set by the framework before we arrive here

        await Task.Delay(TimeSpan.FromSeconds(10);

        // we are now in a continuation, executing on another thread
        //Sender, CurrentMessage etc needs to be correct here

});

In order to make things right in the continuation here, we need to first capture the scheduled task that is the continuation.
We capture this in the ActorTaskScheduler, we can have breakpoints in there, so we can see that that is exactly what is happening.

So we capture the task that is the continuation.
Now we need to extract what actor that actually sent this task, we do this by reading the logicalcallcontext data that we have setup _before_ the Task.Delay.
Now we have the callcontext, but we are still in the wrong thread, we are in a completion port.
We don't know if the actor is processing other messages right now (Assuming we allow re-entrant messages)
So we need to get the continuation to execute within the context that we just extracted.
So we pack the continuation inside a message and pass the message to the actor from the context.
The actor can now process this message and execute the continuation within its own concurrency constraint, and with correct state.

[nvivo]

What seems to be missing here is that you don't capture the continuation because you have the await. You capture it because Task.Delay schedules a task. If it didn't, you'd never have to capture it.

For example:

Receive<Foo>(async f =>
{
    // context is captured here because delay(10) schedules a task
    await Task.Delay(10);
    // context needs to be restored here
});

But this is different:

Receive<Foo>(async f =>
{
    // context is not captured here because Task.Delay(0) doesn't schedule anything
    await Task.Delay(0);
    // even today with no changes to schedulers, Sender is still set because this never created a continuation
});

For all intents and purposes, this code above is executed exactly like an empty block.

So, I'm not saying that capturing context is not necessary, neither I'm saying that async/await is always as fast as synchronous code. What I'm saying is that the _cost of using async/await is the same you would have with ContinueWith in virtually any scenario, and possible less in some of them_. Because "await" is a shortcut for "ContinueWith", not a shortcut for "Task.Run", and by using "async" you allow the runtime to optimize scenarios you didn't predict.

This:

var foo = await Task.FromResult(0);
var bar = 1;

does not turn into this:

var tmp = Task.FromResult(0);
tmp.ContinueWith(t => {
    foo = t.Result;
    bar = 1;
});

It turns more into this:

var foo = 0;
var bar = 1;

If you turn this code into PipeTo inside an actor, you actually incur more overhead by passing a new message than using async/await. The idea of async/await is that you don't have to care about this. If a task is scheduled, you will need to capture the context, but if not it will run as synchronous code. That's why I'm saying that performance cannot decrease.

Also, you can do things like this:

var result = await _httpClient
    .GetAsync(imageUrl)
    .ContinueWith(_ =>{ }, TaskContinuationOptions.AttachedToParent & TaskContinuationOptions.ExecuteSynchronously);

Self.Tell(result);

A task is scheduled inside GetAsync, not in the await.

So, any fears you may have that async/await will incur 50% cost is based on some assumption that is probably wrong.

In practice, Akka supporting async/await in it's core should give the same performance numbers as it gives today if the code you execute in actors is synchronous.

[Aaronontheweb]

TImeframe and remove implicit sender for v1
I think it might be hard to get this in place any time soon, but I think we seriously should consider if we should remove implicit sender in v1,

We've already committed to a public timeframe and scope for V1 - it would look bad if we pushed the release back. I don't see an urgent need to do this now. Akka.NET's had a lot of time in beta status and its in great shape - there will always be things we can do better and some of those will require breaking changes in the future. Let's not move the goal posts this time - let's just ship it.

[Aaronontheweb]

@nvivo

So, any fears you may have that async/await will incur 50% cost is based on some assumption that is probably wrong.

This is based off of an actual benchmark @rogeralsing did.

[stefansedich]

I am with @Aaronontheweb on this one, I just did some async stuff and though crap I need to have awaitable receives.

But realized that this is the wrong way to do it, using some simple Task.Run with a continuation and PipeTo with message passing I was in a happy place and everything working as expected.

If you need to block while the async task runs then Become and Stashing solve that. I would hate to see the async cancer bleed into this as I think you can already achieve what you need.

Cheers

[nvivo]

@stefansedich

Until now, every argument I saw here against async/await boils down to lack of understanding of what it is.

[HCanber]

@nvivo I _think_ I understand what you're saying. :) Are you saying that the 50% cost @rogeralsing saw hadn't really anything to do with async/await. It comes down to what he was doing in what he was awaiting. Correct?

[rogeralsing]

Im starting to think we are all illeterates here.
The 50% overhead I saw was due to the use of LogicalCallContext, as it is way more expensive than ThreadStatic.
The 50% was present even w/o any async ops when that aporoach was enabled.
End of that 50% story....

It had nothing to do with tasks or await, it is for automatic marshalling state across threads in .net

Skickat från min iPhone

4 mar 2015 kl. 07:00 skrev Håkan Canberger [email protected]:

@nvivo I think I understand what you're saying. :) Are you saying that the 50% cost @rogeralsing saw hadn't really anything to do with async/await. It comes down to what he was doing in what he was awaiting. Correct?

—
Reply to this email directly or view it on GitHub.

[nvivo]

@HCanber, exactly.

@rogeralsing, If you had no async code running, you should never use LogicalCallContext, because async/await methods don't cause any change in execution. Scheduling tasks does, and ContinueWith does that.

This means that if you really had no async ops running, you would see this cost only if you use task.ContinueWith(...), but not with await task.

This is a key point in the discussion, because if there is no cost on supporting tasks correctly, you don't need to have settings, and you don't need warnings that performance will decrease because that setting is enabled, and then there is no fear that this is a bad choice for some reason.

I suspect that every misunderstanding of async/await and this "fear" of the "async cancer" has to do with people believing that:

• async - causes a method to execute asynchronously
• await - causes a task to be awaited asynchronously

If anyone here believes that, than it's wrong and all conclusions will be wrong from this point on.

[rogeralsing]

Im not sure at what end the communication problem resides now.

50% overhead:

_The 50% overhead problem and settings have been solved in my last PR._

So any discussion regarding those 50% and/or settings is just a waste of time.
That is done and delat with.

We can do receive actors that does sync processing with no overhead and we can do async processing where the logicalcallcontext is applied, because it has to.
Its used by TransactionScope, HttpCobtext.Current and by Orleans.
So it is the correct way to go about it here.

async

We are not scheduling any tasks onto new threads.
Tasks are wrapped in messages and piped to self.

This means that if the actor is currently running when the task is scheduled, it will run on the same thread as our mailbox does batch processing.
Tasks that rely on io completion will execute async for obvious reasons and thus, the pipeto self to execute the continuations(call them whatever you want but they are by definition continuations)

in short

We do not have settings
We do not have the 50% overhead
Tasks execute inline if possible (if you do not get why, see ActorTaskScheduler, mailbox and dispatchers)

There is room for improvements, if the actorcontext is the same as the logicalcallcontext , we do not need to go through the mailbox, we can tryexecute directly in the taskscheduler in that case.

Is anything of this still unclear?

If so, provide a concrete example where this approach would fail, pseudo code plus comments on what you think is going on/ will happen in that example and we take it from there.

Ok?

[nvivo]

@HCanber, again you are correct. Roger solves some of the problems related to async/await but not all of them, and introduces others.

@rogeralsing, most problems you are trying to solve with this implementation wouldn't even exist if proper task support was in place. Once you open your mind to the possibility that this implementation is not the best one, things will make more sense and we can discuss alternatives.

Im not sure at what end the communication problem resides now.

I think the problem here is that no questions are being asked. You are just assuming I'm wrong. I read your the implementation. I understand what it's doing and I'm posting my concerns and alternative solutions. But nothing is being asked about what I'm proposing.

I'm not proposing that Akka 1.0 is delayed, I'm not proposing that no temporary solutions can be developed, I'm not proposing that stashing/become is unfeasible.

All I'm only proposing to discuss "proper support for async/await inside actors", and this seems to requires more changes to the core of akka to support tasks correctly.

Your answer sounds like I'm asking for you to stop everything you are doing and do what I want right now. This is not the case.

[tired]So, in short....[/tired]

My concerns with Roger's solution are:

• Is a hack on top of the current architecture
• Requires a custom syntax of RunTask inside a "taskless" method, which is considered a bad practice and non-standard
• RunTask starts a task - it shouldn't, even in the same thread
• The RunTask doesn't add any more control or safety than Task.Run would provide
• Doesn't handle reentrancy by the possibility of awaiting for the task to complete
• Forces message passing when there is no need to

What I'm proposing is:

• Option to have non-blocking "Receive" methods by using Tasks
• Use standard .NET syntax to support tasks by exposing methods that return Tasks
• Use standard .NET behavior to opt out of actor context with ConfigureAwait(false) if needed
• Supports both reentrant and non-reentrant scenarios well
• Allows to use Task.Run and still change the state of the actor inside after await without fear of breaking the actor model
• Allows for Self.Tell(msg) that would work in every case after await, both reentrant and non-reentrant, making PipeTo not required and it's behavior more explicit without any change to what it does underneath
• Have zero overhead if you don't use async methods
• Have zero breaking changes to the API

I'd like to see this even as a long term goal. Even if it is Akka 2.0, even if it takes years. Is that unreasonable?

I can explain any point and how that would work if it's not clear.

[rogeralsing]

Oh drar lord. I dont even know where to begin

1. We already support non blocking receive.

Receive(async f => ....)

1. Yes it does offer more stafety for actor state.
   You can even use task.run inside it.

The runtask is for doing the same in the other actor types

1. I clearly expressed that we could get around the mailbox step in the cases the task can run within the actor.

[nvivo]

I honestly feel like talking to walls here. =)
Sorry to bring this up, let's move on.