API for creating cancellable tasks

[Kentaro Hara]

Hi

Currently we have multiple APIs to create cancellable tasks. We have inconsistently mixed CancellableTaskFactory and Timer. We recently introduced makeCancellable. It would be important to reach consensus about the design/APIs to move more things to the per-frame scheduler.

A good start point would be to identify issues of CancellableTaskFactory and figure out how to make it better (so that we can use CancellableTaskFactory in common cases). As far as I chatted with tzik@ and hiroshige@, we see the following issues in CancellableTaskFactory:

a) CancellableTaskFactory::create() cannot take function parameters. Hence, the caller site needs to store all parameters on |this| object before creating a task. This is not nice.

b) CancellableTaskFactory doesn't guarantee one-shot-ness of the underlying WTF::Closure (i.e., the WTF::Closure can be posted multiple times). If one-shot-ness is not guaranteed, we need to keep alive the function parameters while CancellableTaskFactory is alive. This can unnecessarily extend the lifetime of the function parameters. Ideally we want to clear/std::move the parameters when the WTF::Closure is fired. Also, not having the one-shot-ness makes it hard to support cancellable tasks in task queues.

c) CancellableTaskFactory should return a WTF::Closure instead of a Task so that we can always pass in WTF::Closure to postTask APIs.

Are there any other issues you're aware of?

a) and c) would be easy to fix but I'm not sure about b).

--

Kentaro Hara, Tokyo, Japan

[Alex Clarke]

Yes it should return a WTF::Closure and I think we should try to get rid of WebTaskRunner::Task.

Also I'm planning on introducing a new API to WebTaskRunner:

TaskHandle postCancellableTask(const WebTraceLocation&, std::unique_ptr<WTF::Closure>);

DelayedTaskHandle postDelayedTask(const WebTraceLocation&, std::unique_ptr<WTF::Closure>, double delayMs);

// Will remove the task from the queue

bool CancelTask(TaskHandle);

bool CancelTask(DelayedTaskHandle);

It might make sense to add something similar to CancellableTaskFactory which holds the TaskHandle/DelayedTaskHandle or even replace CancellableTaskFactory entirely.

[Kentaro Hara]

On Fri, Aug 12, 2016 at 6:28 PM, Alex Clarke <alexc...@google.com> wrote:

On 12 August 2016 at 10:20, Kentaro Hara <har...@chromium.org> wrote:

Hi

Currently we have multiple APIs to create cancellable tasks. We have inconsistently mixed CancellableTaskFactory and Timer. We recently introduced makeCancellable. It would be important to reach consensus about the design/APIs to move more things to the per-frame scheduler.

A good start point would be to identify issues of CancellableTaskFactory and figure out how to make it better (so that we can use CancellableTaskFactory in common cases). As far as I chatted with tzik@ and hiroshige@, we see the following issues in CancellableTaskFactory:

a) CancellableTaskFactory::create() cannot take function parameters. Hence, the caller site needs to store all parameters on |this| object before creating a task. This is not nice.

b) CancellableTaskFactory doesn't guarantee one-shot-ness of the underlying WTF::Closure (i.e., the WTF::Closure can be posted multiple times). If one-shot-ness is not guaranteed, we need to keep alive the function parameters while CancellableTaskFactory is alive. This can unnecessarily extend the lifetime of the function parameters. Ideally we want to clear/std::move the parameters when the WTF::Closure is fired. Also, not having the one-shot-ness makes it hard to support cancellable tasks in task queues.

c) CancellableTaskFactory should return a WTF::Closure instead of a Task so that we can always pass in WTF::Closure to postTask APIs.

 

Yes it should return a WTF::Closure and I think we should try to get rid of WebTaskRunner::Task.

Also I'm planning on introducing a new API to WebTaskRunner:

TaskHandle postCancellableTask(const WebTraceLocation&, std::unique_ptr<WTF::Closure>);

DelayedTaskHandle postDelayedTask(const WebTraceLocation&, std::unique_ptr<WTF::Closure>, double delayMs);

// Will remove the task from the queue

bool CancelTask(TaskHandle);

bool CancelTask(DelayedTaskHandle);

Oh, this design makes a lot of more sense! It will resolve all of a), b) and c).

 

It might make sense to add something similar to CancellableTaskFactory which holds the TaskHandle/DelayedTaskHandle or even replace CancellableTaskFactory entirely.

Yeah, if we have postCancellableTask on WebTaskRunner, we no longer need CancellableTaskFactory.

 

 

Are there any other issues you're aware of?

a) and c) would be easy to fix but I'm not sure about b).

--

Kentaro Hara, Tokyo, Japan

[Sami Kyostila]

I think something like this would be great to have. We chatted about a few things with Alex:

- It might be nice to combine TaskHandle and DelayedTaskHandle into a single opaque object if it doesn't add too much overhead.

- CancelTask() should only be called on the target thread. We *could* make it work from other threads, but I feel this would encourage writing accidentally racy code.

- It's safe to call CancelTask() on the wrong task queue or after the task has run (it'll be a no-op), but we could add DCHECKs as a guard.

- Once we prove this pattern in Blink it might be nice to have it in base too.

(Small naming nit: postDelayedTask => postCancellableDelayedTask)

- Sami

[Kentaro Hara]

I chatted with tzik@ and hiroshige@ offline.

The postCancellableTask API looks great to us. All one-shot tasks in Blink can use it. Then we can remove CancellableTaskRunner, makeCancellable and one-shot Timers :)

[Hiroshige Hayashizaki]

> bool CancelTask(TaskHandle);

Can we cancel a task only by TaskHandle?

If we need a pointer to WebTaskRunner to use CancelTask(), we might have to retain a reference to a TaskRunner because TaskRunnerHelper::get() might return different TaskRunners when called for postCancellableTask() and when called for CancelTask().

[Alex Clarke]

On 12 August 2016 at 11:50, Hiroshige Hayashizaki <hiro...@chromium.org> wrote:

> bool CancelTask(TaskHandle);

Can we cancel a task only by TaskHandle?

If we need a pointer to WebTaskRunner to use CancelTask(), we might have to retain a reference to a TaskRunner because TaskRunnerHelper::get() might return different TaskRunners when called for postCancellableTask() and when called for CancelTask().

The handles should be globally unique so we can probably add a cancel on the scheduler method too.  The scheduler internally needs to figure out which task queue it was posted on, using a raw pointer might not be too bad since TaskQueueManager contains std::set<scoped_refptr<internal::TaskQueueImpl>> queues_  and I think we can efficiently check if a raw pointer to a TaskQueue in in that.

[Sami Kyostila]

Maybe Hiroshige meant that TaskHandle should have a cancel() method on it? Cancelling via the scheduler would work too, but then we'd need to plumb information about the scheduler everywhere, which somewhat defeats the purpose of a task runner object IMO.

Should the task handles also be on the oilpan heap? That way they could keep a reference to the task queue in order to implement cancellation.

- Sami

[dan...@chromium.org]

Are y'all aware of base::CancelableCallback? It provides cancelable tasks without requiring any extra post-task APIs. I'm not sure if something like that won't work or has other downsides for you, but want to make sure you've seen it.

--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to platform-architecture-dev+unsubsc...@chromium.org.
To post to this group, send email to platform-architecture-dev@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CABg10jzppS1Q%3D809bxyURFT5kUDFnGC0taGh16EhBXGzt-e6Sg%40mail.gmail.com.

[Sami Kyostila]

Yep, CancellableTaskFactory was kinda sorta modeled after it. The downside is that task executor won't know about the cancellation so it still needs to do a bunch of extra work to run a no-op task.

- Sami

pe 12. elokuuta 2016 klo 19.10 dan...@chromium.org kirjoitti:

Are y'all aware of base::CancelableCallback? It provides cancelable tasks without requiring any extra post-task APIs. I'm not sure if something like that won't work or has other downsides for you, but want to make sure you've seen it.

On Fri, Aug 12, 2016 at 2:20 AM, Kentaro Hara <har...@chromium.org> wrote:

Hi

Currently we have multiple APIs to create cancellable tasks. We have inconsistently mixed CancellableTaskFactory and Timer. We recently introduced makeCancellable. It would be important to reach consensus about the design/APIs to move more things to the per-frame scheduler.

A good start point would be to identify issues of CancellableTaskFactory and figure out how to make it better (so that we can use CancellableTaskFactory in common cases). As far as I chatted with tzik@ and hiroshige@, we see the following issues in CancellableTaskFactory:

a) CancellableTaskFactory::create() cannot take function parameters. Hence, the caller site needs to store all parameters on |this| object before creating a task. This is not nice.

b) CancellableTaskFactory doesn't guarantee one-shot-ness of the underlying WTF::Closure (i.e., the WTF::Closure can be posted multiple times). If one-shot-ness is not guaranteed, we need to keep alive the function parameters while CancellableTaskFactory is alive. This can unnecessarily extend the lifetime of the function parameters. Ideally we want to clear/std::move the parameters when the WTF::Closure is fired. Also, not having the one-shot-ness makes it hard to support cancellable tasks in task queues.

c) CancellableTaskFactory should return a WTF::Closure instead of a Task so that we can always pass in WTF::Closure to postTask APIs.

Are there any other issues you're aware of?

a) and c) would be easy to fix but I'm not sure about b).

--

Kentaro Hara, Tokyo, Japan

--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to platform-architect...@chromium.org.
To post to this group, send email to platform-arc...@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CABg10jzppS1Q%3D809bxyURFT5kUDFnGC0taGh16EhBXGzt-e6Sg%40mail.gmail.com.

--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to platform-architect...@chromium.org.
To post to this group, send email to platform-arc...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAHtyhaSX8p0gXMjAf1ULzrcXij%3DEPDtzaH9t%2BrVGKQYUi39XVg%40mail.gmail.com.