Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Task.forkAndForget #520

Closed
alexandru opened this issue Jan 13, 2018 · 5 comments
Closed

Add Task.forkAndForget #520

alexandru opened this issue Jan 13, 2018 · 5 comments
Milestone

Comments

@alexandru
Copy link
Member

alexandru commented Jan 13, 2018

For Task we now have .start and .fork with the same semantics, except that fork also forces a logical thread fork:

sealed abstract class Task[+A] {
  // ...
  def start: Task[Task[A]]
  // ...
  def fork: Task[Task[A]] =
    executeAsync.start
}

Now we need something like this:

sealed abstract class Task[+A] {
  // ...
  def forkAndForget: Task[Unit]
}

Following law should apply, but the implementation doesn't need to be this:

task.forkAndForget <-> task.fork.map(_ => ())

N.B. this law probably can't be tested since forkAndForget is all about triggering side effects.

@alexandru alexandru added this to the 3.0.0 milestone Jan 13, 2018
@Avasil
Copy link
Collaborator

Avasil commented Jan 15, 2018

I would love to attempt this with other implementation, I assume it has some benefits and it would be something similar to Task.start implementation? I have never played with a "lower level" Task functions before but it seems like a good way to start.

@alexandru
Copy link
Member Author

Sure, go for it.

Just FYI — it should do an automatic fork, so in the implementation you should use Task.unsafeStartAsync.

@Avasil
Copy link
Collaborator

Avasil commented Jan 15, 2018

@alexandru
Could you give some pointers about implementing more tests / desired behavior?

My naive implementation:

 def apply[A](fa: Task[A]): Task[Unit] =
    Task.Async[Unit] { (ctx, cb) =>
      // Light async boundary to avoid stack overflows
      ctx.scheduler.execute(new TrampolinedRunnable {
        def run(): Unit = {
          implicit val sc = ctx.scheduler
          // Standard Scala promise gets used for storing or waiting
          // for the final result
          val p = Promise[Unit]()
          // Building the Task to signal, linked to the above Promise.
          // It needs its own context, its own cancelable
          val ctx2 = Task.Context(ctx.scheduler, ctx.options)
          TaskFromFuture.build(p.future, ctx2.connection)
          // Starting actual execution of our newly created task
          Task.unsafeStartAsync(fa.map(_ => ()), ctx2, Callback.fromPromise(p))
          // Signal the created Task reference
          cb.onSuccess(())
        }
      })
    }

Seems to do the trick (it passes the law) but I'm not confident that it is entirely correct.

I don't fully understand it and have handful of questions related to it:

Why is Future/Promise used versus Task.create ? To pass StackedCancelable from Context?

How does Context work? Should every Task have different Context? It looks like we can use connection which is StackedCancelable so each Task created this way can be cancelled on its own. Is this correct?

Is Callback at the end used to signal that Task finished its processing or that it has been created?

I will really appreciate any explanation! I wandered into TaskRunLoop and I'm trying to grasp it but it will take some time. :D

@alexandru
Copy link
Member Author

You don't need a Future / Promise here. You don't need Callback.fromPromise(p), you can just use Callback.empty.

Why is Future/Promise used versus Task.create ?

Because we need to do memoization. We're creating a Task value that corresponds to an already running background process and because of this the created Task value must behave much like a Future - i.e. on one hand you've got the producer that has already started and that needs to store the generated value somewhere and on the other hand you've got a way for listeners to register for receiving a call when that generated value will be ready.

And we already have Future / Promise that do that, no need to reinvent the wheel when we can just wrap those.

But all this is relevant for start and not for startAndForget.

does Context work? Should every Task have different Context?

Every Task needs a different context, because every task needs its own StackedCancelable.
The Context gets injected when Task.Async gets executed, being handled by the internal run-loop, but you've got cases where you need to build your own context, like in this case.

Is Callback at the end used to signal that Task finished its processing or that it has been created?

The callback is signaling that processing is finished, where processing represents just the start of the child task. In the case the parent is done processing since you've started the child, with the actual execution being represented by the signaled child in start and in startAndForget you just leave the background process running with no further ties to it.

NOTE — this is the current implementation of start, but because you're using unsafeStartAsync you don't need to wrap that in an additional ctx.scheduler.execute(new TrampolinedRunnable, the goal being to minimize the async boundaries needed.

@Avasil
Copy link
Collaborator

Avasil commented Jan 15, 2018

@alexandru Thanks that makes sense and clear some things for me, hopefully I'm pretty close in my implementation in #530.

@alexandru alexandru changed the title Add Task.startAndForget Add Task.forkAndForget Jan 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants