What tasks should use what task runners

[Kentaro Hara]

Hi

I just discussed what tasks should use what task runners in the world of the per-frame scheduler with Sami and Alex.

Here is a summary of what we discussed:

- Per-frame scheduler

--- Specific task runners (<= encouraged)

------ Idle task runner

------ Timer task runner

------ Loading task runner

------ Rendering task runner

------ Input task runner

------ ...

--- Default task runner (<= not encouraged)

--- Unthrottled task runner (<= highly not encouraged)

- Per-thread scheduler

--- Idle task runner (<= not encouraged)

--- Default task runner (<= not encouraged)

The key points are as follows:

- We should use the per-frame scheduler as much as possible. The per-thread scheduler is not encouraged.

- We should use the specific task runners as much as possible (we can introduce more specific task runners as necessary). The default task runner is not encouraged.

- Regarding the per-frame scheduler, the specific task runners and the default task runner may be throttled on offscreen iframes. The unthrottled task runner can be used for tasks that shouldn't be throttled but the use case should be limited.

Does the above guideline make sense?

We need to convert 70 timers, 36 same-thread tasks and some of 54 cross-thread tasks posted to the main thread.

--

Kentaro Hara, Tokyo, Japan

[Ojan Vafai]

On Tue, Jul 26, 2016 at 6:28 AM Kentaro Hara <har...@chromium.org> wrote:

Hi

I just discussed what tasks should use what task runners in the world of the per-frame scheduler with Sami and Alex.

Here is a summary of what we discussed:

- Per-frame scheduler

--- Specific task runners (<= encouraged)

------ Idle task runner

------ Timer task runner

------ Loading task runner

------ Rendering task runner

------ Input task runner

What's the difference between the input task runner and the rendering task runner? The benefit of having different task runners is that we can schedule them separately, right? When would we want to schedule rendering and input tasks separately?

It would help to understand this if you had a couple examples of what would go in each of these task runners. If you don't know right now, I would say we should avoid having that task runner until we prove the need for it with specific examples.

 

------ ...

--- Default task runner (<= not encouraged)

Maybe we should avoid having the default task runner at all until we prove we need it for something. The problem with default is that it's where people will naturally put new ones and we'd have to catch it in codereview, which is always hard.

 

--- Unthrottled task runner (<= highly not encouraged)

- Per-thread scheduler

--- Idle task runner (<= not encouraged)

--- Default task runner (<= not encouraged)

The key points are as follows:

- We should use the per-frame scheduler as much as possible. The per-thread scheduler is not encouraged.

- We should use the specific task runners as much as possible (we can introduce more specific task runners as necessary). The default task runner is not encouraged.

- Regarding the per-frame scheduler, the specific task runners and the default task runner may be throttled on offscreen iframes. The unthrottled task runner can be used for tasks that shouldn't be throttled but the use case should be limited.

Does the above guideline make sense?

We need to convert 70 timers, 36 same-thread tasks and some of 54 cross-thread tasks posted to the main thread.

--

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/CABg10jy9b2z5BVYqV_YMHjo%2B4eVaqz0LrShaGnb_HhPbz22%2BRQ%40mail.gmail.com.

[Kentaro Hara]

On Tue, Jul 26, 2016 at 11:25 PM, Ojan Vafai <oj...@chromium.org> wrote:

On Tue, Jul 26, 2016 at 6:28 AM Kentaro Hara <har...@chromium.org> wrote:

Hi

I just discussed what tasks should use what task runners in the world of the per-frame scheduler with Sami and Alex.

Here is a summary of what we discussed:

- Per-frame scheduler

--- Specific task runners (<= encouraged)

------ Idle task runner

------ Timer task runner

------ Loading task runner

------ Rendering task runner

------ Input task runner

What's the difference between the input task runner and the rendering task runner? The benefit of having different task runners is that we can schedule them separately, right? When would we want to schedule rendering and input tasks separately?

It would help to understand this if you had a couple examples of what would go in each of these task runners. If you don't know right now, I would say we should avoid having that task runner until we prove the need for it with specific examples. 

Sami?

 

------ ...

--- Default task runner (<= not encouraged)

Maybe we should avoid having the default task runner at all until we prove we need it for something. The problem with default is that it's where people will naturally put new ones and we'd have to catch it in codereview, which is always hard. 

Yeah, it might be better not to introduce the default task runner than discouraging people from using it. I wanted to introduce the default task runner for WTF::Timers because WTF::Timers are used for many implementation hacks (e.g., delay destructors) and it's sometimes hard to associate the WTF::Timers with specific task runners. Maybe we can use the idle task runner for common WTF::Timers.

 

--- Unthrottled task runner (<= highly not encouraged)

- Per-thread scheduler

--- Idle task runner (<= not encouraged)

--- Default task runner (<= not encouraged)

The key points are as follows:

- We should use the per-frame scheduler as much as possible. The per-thread scheduler is not encouraged.

- We should use the specific task runners as much as possible (we can introduce more specific task runners as necessary). The default task runner is not encouraged.

- Regarding the per-frame scheduler, the specific task runners and the default task runner may be throttled on offscreen iframes. The unthrottled task runner can be used for tasks that shouldn't be throttled but the use case should be limited.

Does the above guideline make sense?

We need to convert 70 timers, 36 same-thread tasks and some of 54 cross-thread tasks posted to the main thread.

--

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/CABg10jy9b2z5BVYqV_YMHjo%2B4eVaqz0LrShaGnb_HhPbz22%2BRQ%40mail.gmail.com.

[Daniel Cheng]

Incidentally, I'd like to split blink::Timer into two subclasses:

- blink::LegacyTimer (I can't really think of a good name for this, suggestions welcome): this just defaults to using Platform::current()->currentThread()->scheduler()->timerTaskRunner() as the task runner, so it's basically the current blink::Timer. Eventually, maybe we can remove this completely, but I'm not sure if we should: it seems like it might be convenient as a way to encourage people to use a blink::Timer with the appropriate task runner instead (and provides an easy way to grep for timers running on the default task queues).

- blink::Timer: like the current blink::Timer, but also takes a required TaskRunner argument (that should not be whatever LegacyTimer defaults to, we can DCHECK this)

In addition, this will help track the progress of the conversion. Thoughts?

Daniel

[Kentaro Hara]

On Wed, Jul 27, 2016 at 9:31 AM, Daniel Cheng <dch...@chromium.org> wrote:

Incidentally, I'd like to split blink::Timer into two subclasses:

- blink::LegacyTimer (I can't really think of a good name for this, suggestions welcome): this just defaults to using Platform::current()->currentThread()->scheduler()->timerTaskRunner() as the task runner, so it's basically the current blink::Timer. Eventually, maybe we can remove this completely, but I'm not sure if we should: it seems like it might be convenient as a way to encourage people to use a blink::Timer with the appropriate task runner instead (and provides an easy way to grep for timers running on the default task queues).

- blink::Timer: like the current blink::Timer, but also takes a required TaskRunner argument (that should not be whatever LegacyTimer defaults to, we can DCHECK this)

In addition, this will help track the progress of the conversion. Thoughts?

Sounds like a nice plan! Timers provide convenient functionalities like cancel(), stop(), startOneShot() etc, so we want to keep them.

Instead of introducing the LegacyTimer, we might want to provide two versions of constructors: One that takes the TaskRunner parameter, and the other that doesn't take the TaskRunner parameter (which should be deprecated).

[Kentaro Hara]

Or you could introduce IdleTimer, LoadingTimer, RenderingTimer etc as sub-classes of Timer. (I don't have any strong opinion.)

[Jochen Eisinger]

btw, v8 uses whatever base::ThreadTaskRunnerHandler::Get() returns at Isolate creation time for foreground tasks and idle tasks, and base::WorkerPool for background tasks.

It would be nice if we could replace base::WorkerPool with something a bit smarter in the future.

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

[Daniel Cheng]

On Wed, Jul 27, 2016 at 4:41 PM Kentaro Hara <har...@chromium.org> wrote:

Or you could introduce IdleTimer, LoadingTimer, RenderingTimer etc as sub-classes of Timer. (I don't have any strong opinion.)

I like this idea, but I think that we would need to put these subclasses in core/dom (so it can grab the corresponding timer from the LocalFrame). Does that sound reasonable? I still think we should give the default Timer class a longer name either way, so it doesn't become the go-to Timer class anyway because of it's short name =)

 

On Wed, Jul 27, 2016 at 9:39 AM, Kentaro Hara <har...@chromium.org> wrote:

On Wed, Jul 27, 2016 at 9:31 AM, Daniel Cheng <dch...@chromium.org> wrote:

Incidentally, I'd like to split blink::Timer into two subclasses:

- blink::LegacyTimer (I can't really think of a good name for this, suggestions welcome): this just defaults to using Platform::current()->currentThread()->scheduler()->timerTaskRunner() as the task runner, so it's basically the current blink::Timer. Eventually, maybe we can remove this completely, but I'm not sure if we should: it seems like it might be convenient as a way to encourage people to use a blink::Timer with the appropriate task runner instead (and provides an easy way to grep for timers running on the default task queues).

- blink::Timer: like the current blink::Timer, but also takes a required TaskRunner argument (that should not be whatever LegacyTimer defaults to, we can DCHECK this)

In addition, this will help track the progress of the conversion. Thoughts?

Sounds like a nice plan! Timers provide convenient functionalities like cancel(), stop(), startOneShot() etc, so we want to keep them.

Instead of introducing the LegacyTimer, we might want to provide two versions of constructors: One that takes the TaskRunner parameter, and the other that doesn't take the TaskRunner parameter (which should be deprecated).

This is my current approach but it's harder to track progress this way: I need to check codesearch for Timers to update, and the codesearch index is not updated in real time.

Daniel

[Kentaro Hara]

On Wed, Jul 27, 2016 at 9:56 AM, Daniel Cheng <dch...@chromium.org> wrote:

On Wed, Jul 27, 2016 at 4:41 PM Kentaro Hara <har...@chromium.org> wrote:

Or you could introduce IdleTimer, LoadingTimer, RenderingTimer etc as sub-classes of Timer. (I don't have any strong opinion.)

I like this idea, but I think that we would need to put these subclasses in core/dom (so it can grab the corresponding timer from the LocalFrame). Does that sound reasonable? I still think we should give the default Timer class a longer name either way, so it doesn't become the go-to Timer class anyway because of it's short name =)

I'm fine with having the sub-classed timers in core/dom/. That way we can avoid writing m_timer(..., TaskRunnerHelper::idleTaskRunner(frame)) everywhere we use Timers.

 

On Wed, Jul 27, 2016 at 9:39 AM, Kentaro Hara <har...@chromium.org> wrote:

On Wed, Jul 27, 2016 at 9:31 AM, Daniel Cheng <dch...@chromium.org> wrote:

Incidentally, I'd like to split blink::Timer into two subclasses:

- blink::LegacyTimer (I can't really think of a good name for this, suggestions welcome): this just defaults to using Platform::current()->currentThread()->scheduler()->timerTaskRunner() as the task runner, so it's basically the current blink::Timer. Eventually, maybe we can remove this completely, but I'm not sure if we should: it seems like it might be convenient as a way to encourage people to use a blink::Timer with the appropriate task runner instead (and provides an easy way to grep for timers running on the default task queues).

- blink::Timer: like the current blink::Timer, but also takes a required TaskRunner argument (that should not be whatever LegacyTimer defaults to, we can DCHECK this)

In addition, this will help track the progress of the conversion. Thoughts?

Sounds like a nice plan! Timers provide convenient functionalities like cancel(), stop(), startOneShot() etc, so we want to keep them.

Instead of introducing the LegacyTimer, we might want to provide two versions of constructors: One that takes the TaskRunner parameter, and the other that doesn't take the TaskRunner parameter (which should be deprecated).

This is my current approach but it's harder to track progress this way: I need to check codesearch for Timers to update, and the codesearch index is not updated in real time.

Makes sense.

[Kentaro Hara]

On Wed, Jul 27, 2016 at 9:47 AM, Jochen Eisinger <joc...@chromium.org> wrote:

btw, v8 uses whatever base::ThreadTaskRunnerHandler::Get() returns at Isolate creation time for foreground tasks and idle tasks, and base::WorkerPool for background tasks.

Yeah, Blink's worker thread can (should) do this. Regarding the main thread, the complexity comes from the fact that we want to use multiple specific task runners per frame...

It would be nice if we could replace base::WorkerPool with something a bit smarter in the future.

Agreed.

[Sami Kyostila]

ke 27. heinäkuuta 2016 klo 8.26 Kentaro Hara <har...@chromium.org> kirjoitti:

On Tue, Jul 26, 2016 at 11:25 PM, Ojan Vafai <oj...@chromium.org> wrote:

On Tue, Jul 26, 2016 at 6:28 AM Kentaro Hara <har...@chromium.org> wrote:

Hi

I just discussed what tasks should use what task runners in the world of the per-frame scheduler with Sami and Alex.

Here is a summary of what we discussed:

- Per-frame scheduler

--- Specific task runners (<= encouraged)

------ Idle task runner

------ Timer task runner

------ Loading task runner

------ Rendering task runner

------ Input task runner

What's the difference between the input task runner and the rendering task runner? The benefit of having different task runners is that we can schedule them separately, right? When would we want to schedule rendering and input tasks separately?

It would help to understand this if you had a couple examples of what would go in each of these task runners. If you don't know right now, I would say we should avoid having that task runner until we prove the need for it with specific examples. 

Sami?

Right now the Blink scheduler only has a compositor task runner, which combines both input and compositing/rendering tasks. We were thinking it might be clearer to have separate task runners for both so we more visibility into the semantic type of each task. The actual tasks would still be run the same as before (because they can't be reordered).

I don't feel too strongly about this one -- we could just as well continue funneling both types of tasks to one compositor queue.

 

 

------ ...

--- Default task runner (<= not encouraged)

Maybe we should avoid having the default task runner at all until we prove we need it for something. The problem with default is that it's where people will naturally put new ones and we'd have to catch it in codereview, which is always hard. 

Yeah, it might be better not to introduce the default task runner than discouraging people from using it. I wanted to introduce the default task runner for WTF::Timers because WTF::Timers are used for many implementation hacks (e.g., delay destructors) and it's sometimes hard to associate the WTF::Timers with specific task runners. Maybe we can use the idle task runner for common WTF::Timers.

Using idle tasks for these types of things is fine if you don't mind the task being arbitrarily delayed. You can also request an upper limit to the delay. 

 

--- Unthrottled task runner (<= highly not encouraged)

- Per-thread scheduler

--- Idle task runner (<= not encouraged)

--- Default task runner (<= not encouraged)

The key points are as follows:

- We should use the per-frame scheduler as much as possible. The per-thread scheduler is not encouraged.

- We should use the specific task runners as much as possible (we can introduce more specific task runners as necessary). The default task runner is not encouraged.

- Regarding the per-frame scheduler, the specific task runners and the default task runner may be throttled on offscreen iframes. The unthrottled task runner can be used for tasks that shouldn't be throttled but the use case should be limited.

Does the above guideline make sense?

We need to convert 70 timers, 36 same-thread tasks and some of 54 cross-thread tasks posted to the main thread.

--

Kentaro Hara, Tokyo, Japan

- Sami

[Sami Kyostila]

On the subject of timers, it seems like a vast majority of them are using a delay of zero, i.e., they're not really timers at all but more like cancellable tasks. Would it make sense to steer the new non-legacy Timer class to that direction?

- Sami

[Daniel Cheng]

What's the advantage of using CancellableTaskFactory over Timer? One nice thing about Timer is that the task factory to use can be implicitly bound into the Timer. It's also not without precedent to use a timer for these sort of tasks: it's quite common in Chromium as well (see base::OneShotTimer).

Daniel

[Kentaro Hara]

Yeah. What we really need is just a cancellable task. However, if we want to support convenient methods like startOneShot, startRepeating, isActive etc, we need to create a class that wraps the cancellable task. That is Timer :)

[Sami Kyostila]

Is there a lot of usage startRepeating()? My thought was that if folks just want a one-shot task, then we should give them that interface instead of something more complicated :)

I'm not sure CancellableTaskFactory is that thing either -- seems like it could be simplified to be more of a drop-in replacement for one shot timers.

- Sami

[Alex Clarke]

On 28 July 2016 at 15:03, Daniel Cheng <dch...@chromium.org> wrote:

What's the advantage of using CancellableTaskFactory over Timer?

It's API is simpler, and it's obviously a one shot thing.

I actually think we might consider splitting it into two, one that's threadsafe (the current one) and a non-thread safe version which doesn't use weak pointers.

[Kentaro Hara]

There're only 10 users of startRepeating(). But there're a lot of users of isActive(). The common pattern is that we kick a timer only when it's not yet active. Also there're a couple of users who want to mocks the behavior in tests by inheriting Timer's methods.

I agree that we can replace X% (X >= 50) of the existing Timers with CancellableTasks, but a certain amount of Timers will remain.

[Alex Clarke]

On 28 July 2016 at 16:06, Kentaro Hara <har...@chromium.org> wrote:

There're only 10 users of startRepeating(). But there're a lot of users of isActive().

For completeness CancellableTaskFactory supports this with isPending()

[Kentaro Hara]

To sum up the discussion so far, our preference is something like this?

- Per-frame scheduler

---- Idle task runner

---- Timer task runner

---- Loading task runner

---- Rendering task runner

---- ... (<= We may add more specific task runners as necessary.)

---- Unthrottled task runner (<= not encouraged)

- Per-thread scheduler

--- Idle task runner (<= not encouraged)

--- Default task runner (<= not encouraged)

(Nit: Maybe we might want to rename Timer task runner to JSTimer task runner, since it's confusing with other WTF::Timers. The WTF::Timers should use one of Loading, Rendering and Idle task runners.)

[Ojan Vafai]

At a high-level, lgtm. But, I want to repeat that the most helpful thing in understanding/evaluating this proposal is to have a couple examples of things that would go in each of these queues.

Just to give a bit of color here to what I was saying. I'm a bit worried that as we add all these task runners it will be very hard for chrome devs to reason about which task runner they should put their task in and that we'll need to rely on experts to review every patch that adds a new task. So, the simpler we can keep things the better. If we get concrete wins from creating a new task runner, then I think it's worth it. But, I'd err on the side of not adding speculative ones.

I'm also a bit confused about how using different task runners doesn't affect task running order. If you throttle one task runner, but not another, then the order of the tasks relative to each other changes, right? Am I misunderstanding how the system works? The end result would be that you can't assume a consistent interleaving of rendering and input events if we had different task runners for them, which I think would probably not be what we want. But again, I'm making theoretical/conceptual arguments here. It will be easier to reason about when we actually have lists of tasks that would go in each task runner.

Also, yay! Can't wait for this work to complete. It's gonna be great. :)

[Kentaro Hara]

On Thu, Jul 28, 2016 at 8:58 PM, Ojan Vafai <oj...@chromium.org> wrote:

At a high-level, lgtm. But, I want to repeat that the most helpful thing in understanding/evaluating this proposal is to have a couple examples of things that would go in each of these queues.

Just to give a bit of color here to what I was saying. I'm a bit worried that as we add all these task runners it will be very hard for chrome devs to reason about which task runner they should put their task in and that we'll need to rely on experts to review every patch that adds a new task. So, the simpler we can keep things the better. If we get concrete wins from creating a new task runner, then I think it's worth it. But, I'd err on the side of not adding speculative ones.

I understand your concern. However, if we want to avoid adding a new task runner until we're confident that it's needed, we'll need the default task runner. For example, if we only have Idle, Loading and Timer task runners, it's not clear where HTMLCanvasElement's tasks should go. In other words, if we want to avoid the default task runner, we cannot avoid adding a couple of specific task runners speculatively, I guess.

[Ojan Vafai]

On Thu, Jul 28, 2016 at 12:56 PM Kentaro Hara <har...@chromium.org> wrote:

On Thu, Jul 28, 2016 at 8:58 PM, Ojan Vafai <oj...@chromium.org> wrote:

At a high-level, lgtm. But, I want to repeat that the most helpful thing in understanding/evaluating this proposal is to have a couple examples of things that would go in each of these queues.

Just to give a bit of color here to what I was saying. I'm a bit worried that as we add all these task runners it will be very hard for chrome devs to reason about which task runner they should put their task in and that we'll need to rely on experts to review every patch that adds a new task. So, the simpler we can keep things the better. If we get concrete wins from creating a new task runner, then I think it's worth it. But, I'd err on the side of not adding speculative ones.

I understand your concern. However, if we want to avoid adding a new task runner until we're confident that it's needed, we'll need the default task runner. For example, if we only have Idle, Loading and Timer task runners, it's not clear where HTMLCanvasElement's tasks should go. In other words, if we want to avoid the default task runner, we cannot avoid adding a couple of specific task runners speculatively, I guess.

Yup. That is exactly in line with the spirit of what I was trying to say. I think we are well aligned here.

This why I was asking for examples. Canvas seems like a clear justification for adding a Rendering task runner and I can see that even today, we would want to throttle the Rendering task runner separately from the Loading and Timer ones (e.g. when the finger is down, we don't want to throttle rendering, but we might want to throttle loading). So I don't have any concerns or objections to adding a Rendering task runner.

I was just trying to clarify the principles I have in mind for when to add a new one or not. The specific list of new task runners you wrote down in your 7/28 email all make sense to me.

[Sami Kyostila]

to 28. heinäkuuta 2016 klo 19.58 Ojan Vafai <oj...@chromium.org> kirjoitti:

At a high-level, lgtm. But, I want to repeat that the most helpful thing in understanding/evaluating this proposal is to have a couple examples of things that would go in each of these queues.

Just to give a bit of color here to what I was saying. I'm a bit worried that as we add all these task runners it will be very hard for chrome devs to reason about which task runner they should put their task in and that we'll need to rely on experts to review every patch that adds a new task. So, the simpler we can keep things the better. If we get concrete wins from creating a new task runner, then I think it's worth it. But, I'd err on the side of not adding speculative ones.

I agree that developer ergonomics is a big factor here.

 

I'm also a bit confused about how using different task runners doesn't affect task running order. If you throttle one task runner, but not another, then the order of the tasks relative to each other changes, right? Am I misunderstanding how the system works? The end result would be that you can't assume a consistent interleaving of rendering and input events if we had different task runners for them, which I think would probably not be what we want. But again, I'm making theoretical/conceptual arguments here. It will be easier to reason about when we actually have lists of tasks that would go in each task runner.

Right, if you prioritize task runners differently then they will no longer run tasks in order. If you prioritize a bunch of task runners as a unit, then task ordering will be preserved -- but I admit that is somewhat of a fragile setup and we should only do it if it has other clear benefits.

 

Also, yay! Can't wait for this work to complete. It's gonna be great. :)

+1 :)

- Sami

[Kentaro Hara]

On Fri, Jul 29, 2016 at 12:48 PM, Sami Kyostila <skyo...@google.com> wrote:

to 28. heinäkuuta 2016 klo 19.58 Ojan Vafai <oj...@chromium.org> kirjoitti:

At a high-level, lgtm. But, I want to repeat that the most helpful thing in understanding/evaluating this proposal is to have a couple examples of things that would go in each of these queues.

Just to give a bit of color here to what I was saying. I'm a bit worried that as we add all these task runners it will be very hard for chrome devs to reason about which task runner they should put their task in and that we'll need to rely on experts to review every patch that adds a new task. So, the simpler we can keep things the better. If we get concrete wins from creating a new task runner, then I think it's worth it. But, I'd err on the side of not adding speculative ones.

I agree that developer ergonomics is a big factor here.

 

I'm also a bit confused about how using different task runners doesn't affect task running order. If you throttle one task runner, but not another, then the order of the tasks relative to each other changes, right? Am I misunderstanding how the system works? The end result would be that you can't assume a consistent interleaving of rendering and input events if we had different task runners for them, which I think would probably not be what we want. But again, I'm making theoretical/conceptual arguments here. It will be easier to reason about when we actually have lists of tasks that would go in each task runner.

Right, if you prioritize task runners differently then they will no longer run tasks in order. If you prioritize a bunch of task runners as a unit, then task ordering will be preserved -- but I admit that is somewhat of a fragile setup and we should only do it if it has other clear benefits. 

I'm just curious but today is it guaranteed that tasks posted by Blink are fired in the posted order (even if the tasks go to different task runners)?

[Alex Clarke]

On 29 July 2016 at 12:25, Kentaro Hara <har...@chromium.org> wrote:

On Fri, Jul 29, 2016 at 12:48 PM, Sami Kyostila <skyo...@google.com> wrote:

to 28. heinäkuuta 2016 klo 19.58 Ojan Vafai <oj...@chromium.org> kirjoitti:

At a high-level, lgtm. But, I want to repeat that the most helpful thing in understanding/evaluating this proposal is to have a couple examples of things that would go in each of these queues.

Just to give a bit of color here to what I was saying. I'm a bit worried that as we add all these task runners it will be very hard for chrome devs to reason about which task runner they should put their task in and that we'll need to rely on experts to review every patch that adds a new task. So, the simpler we can keep things the better. If we get concrete wins from creating a new task runner, then I think it's worth it. But, I'd err on the side of not adding speculative ones.

I agree that developer ergonomics is a big factor here.

 

I'm also a bit confused about how using different task runners doesn't affect task running order. If you throttle one task runner, but not another, then the order of the tasks relative to each other changes, right? Am I misunderstanding how the system works? The end result would be that you can't assume a consistent interleaving of rendering and input events if we had different task runners for them, which I think would probably not be what we want. But again, I'm making theoretical/conceptual arguments here. It will be easier to reason about when we actually have lists of tasks that would go in each task runner.

Right, if you prioritize task runners differently then they will no longer run tasks in order. If you prioritize a bunch of task runners as a unit, then task ordering will be preserved -- but I admit that is somewhat of a fragile setup and we should only do it if it has other clear benefits. 

I'm just curious but today is it guaranteed that tasks posted by Blink are fired in the posted order (even if the tasks go to different task runners)?

If they have the same priority & throttling status then yes.

[Sami Kyostila]

To expand on that, for example timer tasks are not guaranteed to run in the same order as unthrottled tasks. Also currently all timer tasks run in order, but when we start throttling timers in out-of-view frames then this will no longer be true.

- Sami

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

[Kentaro Hara]

On Fri, Jul 29, 2016 at 1:30 PM, Sami Kyostila <skyo...@google.com> wrote:

To expand on that, for example timer tasks are not guaranteed to run in the same order as unthrottled tasks.

Yeah, that's why I asked the question. Even if we don't throttle any task runner, we're already breaking the task ordering by having multiple task runners.

The per-frame scheduler will make the breakage a bit worse by introducing more task runners. But my understanding is that it would be okay because the ordering wouldn't really matter in practice. Am I understanding things correctly?

[Sami Kyostila]

pe 29. heinäkuuta 2016 klo 12.43 Kentaro Hara <har...@chromium.org> kirjoitti:

On Fri, Jul 29, 2016 at 1:30 PM, Sami Kyostila <skyo...@google.com> wrote:

To expand on that, for example timer tasks are not guaranteed to run in the same order as unthrottled tasks.

Yeah, that's why I asked the question. Even if we don't throttle any task runner, we're already breaking the task ordering by having multiple task runners.

That's not true: distinct task runners with equal priority (and time domain) will always run tasks in posted order.

 

The per-frame scheduler will make the breakage a bit worse by introducing more task runners. But my understanding is that it would be okay because the ordering wouldn't really matter in practice. Am I understanding things correctly?

Just adding new task runners doesn't necessary change any ordering. Sometimes ordering does matter, especially for web-exposed things, so we need to be a bit careful here. For example we're doing the out-of-view iframe throttling only for cross origin frames to make the changed ordering w.r.t. the main frame less likely to be a problem. (However Safari doesn't make this distinction and seems to get away with it.)

- Sami

[Kentaro Hara]

Thanks, I got your point :)

[Kentaro Hara]

Sami or Alex: Is there any document or comment that describes what task runners have the same priority and time domain?

I'm wondering what level of task ordering is guaranteed with the existing task runners. For example, do the following task runners have the same priority?

- frameScheduler()->timerTaskRunner()

- frameScheduler()->loadingTaskRunner()

- frameScheduler()->unthrottledTaskRunner()

- currentThread()->scheduler()->timerTaskRunner()

- currentThread()->scheduler()->loadingTaskRunner() 

- currentThread()->getWebTaskRunner()

[Sami Kyostila]

In general we shouldn't assume any ordering between different task runners. Otherwise we tie our hands from actually setting different priorities for different task runners and the whole thing becomes kind of pointless :)

That said, you can check the priority settings from here: https://cs.chromium.org/chromium/src/components/scheduler/renderer/renderer_scheduler_impl.cc?rcl=0&l=768

Right now in your list the default, loading and unthrottled task runners run in order w.r.t. one another, but timers may run out of order. But to reiterate, let's not rely on this.

- Sami

[Kentaro Hara]

Today Elliott, Ojan and I discussed in an arch-leads meeting what task ordering is guaranteed today and how the per-frame scheduler is going to break it. I'm not intending to rely on the priorities; just wanted to clarify :)

I'm confused.

In the above of this thread, you were saying:

>> Yeah, that's why I asked the question. Even if we don't throttle any task runner, we're already breaking the task ordering by having multiple task runners.

>>

> That's not true: distinct task runners with equal priority (and time domain) will always run tasks in posted order.

but now you're saying:

> In general we shouldn't assume any ordering between different task runners. Otherwise we tie our hands from actually setting different priorities for different task runners and the whole thing becomes kind of pointless :)
>

> Right now in your list the default, loading and unthrottled task runners run in order w.r.t. one another, but timers may run out of order. But to reiterate, let's not rely on this.

If we shouldn't assume any ordering between different task runners, (as I mentioned above) doesn't it mean that we're already breaking the task ordering by having multiple task runners (e.g., timer and loading)?

[Sami Kyostila]

In my defense both are technically true :) Just adding another task runner doesn't cause things to run out of order, but as soon as someone sets a different priority on that task runner (e.g., just by disabling it), the global task ordering will change. That's why I think as we move things to distinct task runners, we should assume that they can run out of order w.r.t. other task runners. Does that make sense?

- Sami 

[Alex Clarke]

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

Today Elliott, Ojan and I discussed in an arch-leads meeting what task ordering is guaranteed today and how the per-frame scheduler is going to break it. I'm not intending to rely on the priorities; just wanted to clarify :)

I'm confused.

In the above of this thread, you were saying:

>> Yeah, that's why I asked the question. Even if we don't throttle any task runner, we're already breaking the task ordering by having multiple task runners.

>>

> That's not true: distinct task runners with equal priority (and time domain) will always run tasks in posted order.

but now you're saying:

> In general we shouldn't assume any ordering between different task runners. Otherwise we tie our hands from actually setting different priorities for different task runners and the whole thing becomes kind of pointless :)
>
> Right now in your list the default, loading and unthrottled task runners run in order w.r.t. one another, but timers may run out of order. But to reiterate, let's not rely on this.

Perhaps a quick discussion of how TaskQueues works would be helpful:

A scheduler task queue contains four queues:

• std::unique_ptr<WorkQueue> immediate_work_queue
• Contains non-delayed tasks that are ready to run
• std::unique_ptr<WorkQueue> delayed_work_queue 
  
• Contains delayed tasks whose delay has expired and are ready to run
• std::queue<Task> immediate_incoming_queue
• PostTask inserts tasks in this queue, and when immediate_work_queue becomes empty the scheduler swaps them
• std::priority_queue<Task> delayed_incoming_queue
  
• PostDelayedTask inserts tasks in this queue, and when the delay expires, tasks are moved into the delayed_work_queue

Selecting which task to run next

Each task queue can be enabled or disabled.

  virtual void SetQueueEnabled(bool enabled) = 0;
And each task queue has a priority:
 enum QueuePriority {    CONTROL_PRIORITY,     -- Highest priority, for internal use only, will starve out other work    HIGH_PRIORITY,        -- For every 5 high priority tasks, one lower priority one will run    NORMAL_PRIORITY,    BEST_EFFORT_PRIORITY, -- Only runs if there's no other work to do  }  virtual void SetQueuePriority(QueuePriority priority) = 0;

When selecting which task to run next, the scheduler uses this algorithm:
Run any CONTROL_PRIORITY tasks
If starvation counter < 5 then run any HIGH_PRIORITY tasks and increment starvation counter
Run any NORMAL_PRIORITY tasks and set starvation counter to zero 
Run any BEST_EFFORT_PRIORITY tasks and set starvation counter to zero 

To make that fast, for each priority the scheduler maintains a map of queueing order to task queues.  The queueing order is a 64 bit int set whenever
a task becomes ready to run (e.g. when you PostTask or when the delay has expired for a PostDelayedTask) and the smaller the number the older the
task is.

If there is more than one task queue with a given priority, the one with the lowest queueing order is selected.  This is why we say that if there are several
task queues with the same priority tasks will run in the order they are posted in.

Most scheduling decisions are implemented by manipulating the priority of compositor_task_queue, and temporarily enabling or disabling other queues.

By doing so we change the order in which tasks get run, and we have to be very careful about that or we risk breaking people's expectations.
Some notes:
The loading_tq can't be prioritized over the default_tq or IPC code starts to break.
The loading_tq can often be throttled without ill effect (bar increased loading times).
The timer_tq can often be throttled without ill effect (bar lower timer throughput).
The compositor_tq can safely have any priority - although getting it wrong causes jank
Generally the loading_tq, timer_tq and default_tq all have NORMAL_PRIORITY

[Kentaro Hara]

Thanks a lot for the details, Alex and Sami!

In my defense both are technically true :) Just adding another task runner doesn't cause things to run out of order, but as soon as someone sets a different priority on that task runner (e.g., just by disabling it), the global task ordering will change. That's why I think as we move things to distinct task runners, we should assume that they can run out of order w.r.t. other task runners. Does that make sense?

The idea of the per-frame scheduler is, in the first place, that we want to deprioritize tasks in some task runners of third-party iframes. This means that we *must not* assume that tasks in distinct task runners are ordered, even if the distinct task runners happen to have the same priority. Am I understanding correctly?

If that is the case, I begin to think that it would be risky to introduce more specific runners such as a Rendering task runner, a Worker task runner etc, because having more task runners increases a risk of violating the task ordering (because we shouldn't assume that those task runners have the same priority by design). Then wouldn't it make more sense to put as many tasks as possible to one specific task runner (e.g., a Timer task runner) until we really want to implement a different scheduling policy for the tasks in the task runner?

[Alex Clarke]

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

Thanks a lot for the details, Alex and Sami!

In my defense both are technically true :) Just adding another task runner doesn't cause things to run out of order, but as soon as someone sets a different priority on that task runner (e.g., just by disabling it), the global task ordering will change. That's why I think as we move things to distinct task runners, we should assume that they can run out of order w.r.t. other task runners. Does that make sense?

The idea of the per-frame scheduler is, in the first place, that we want to deprioritize tasks in some task runners of third-party iframes. This means that we *must not* assume that tasks in distinct task runners are ordered, even if the distinct task runners happen to have the same priority. Am I understanding correctly?

If that is the case, I begin to think that it would be risky

 

That depends what you do with them :)

Introducing a new task runner which inherits the settings from another task queue for FrameBlamr isn't inherently risky because this isn't changing the order of execution.

If we want to introduce something new (and the spec gives us some latitude to do this) and change it's priority, that is risky.  Stuff will break, but fixing that might be worth it should be benefit be significant.

[Kentaro Hara]

On Tue, Aug 2, 2016 at 11:34 PM, 'Alex Clarke' via platform-architecture-dev <platform-arc...@chromium.org> wrote:

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

Thanks a lot for the details, Alex and Sami!

In my defense both are technically true :) Just adding another task runner doesn't cause things to run out of order, but as soon as someone sets a different priority on that task runner (e.g., just by disabling it), the global task ordering will change. That's why I think as we move things to distinct task runners, we should assume that they can run out of order w.r.t. other task runners. Does that make sense?

The idea of the per-frame scheduler is, in the first place, that we want to deprioritize tasks in some task runners of third-party iframes. This means that we *must not* assume that tasks in distinct task runners are ordered, even if the distinct task runners happen to have the same priority. Am I understanding correctly?

If that is the case, I begin to think that it would be risky

 

That depends what you do with them :)

Introducing a new task runner which inherits the settings from another task queue for FrameBlamr isn't inherently risky because this isn't changing the order of execution.

If we want to introduce something new (and the spec gives us some latitude to do this) and change it's priority, that is risky.  Stuff will break, but fixing that might be worth it should be benefit be significant.

Hmm. But what still confuses me is that it sounds like "You must not assume that tasks in distinct task runners are orderly executed. However, regarding task runners that you know have the same priority, you can assume that the tasks are orderly executed".

(Sorry if I'm bothering you with nit-picky questions. I hope Ojan or Elliott will chime in here to settle the discussion :-)

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

[Alex Clarke]

On 2 August 2016 at 16:25, Kentaro Hara <har...@chromium.org> wrote:

On Tue, Aug 2, 2016 at 11:34 PM, 'Alex Clarke' via platform-architecture-dev <platform-arc...@chromium.org> wrote:

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

Thanks a lot for the details, Alex and Sami!

In my defense both are technically true :) Just adding another task runner doesn't cause things to run out of order, but as soon as someone sets a different priority on that task runner (e.g., just by disabling it), the global task ordering will change. That's why I think as we move things to distinct task runners, we should assume that they can run out of order w.r.t. other task runners. Does that make sense?

The idea of the per-frame scheduler is, in the first place, that we want to deprioritize tasks in some task runners of third-party iframes. This means that we *must not* assume that tasks in distinct task runners are ordered, even if the distinct task runners happen to have the same priority. Am I understanding correctly?

If that is the case, I begin to think that it would be risky

 

That depends what you do with them :)

Introducing a new task runner which inherits the settings from another task queue for FrameBlamr isn't inherently risky because this isn't changing the order of execution.

If we want to introduce something new (and the spec gives us some latitude to do this) and change it's priority, that is risky.  Stuff will break, but fixing that might be worth it should be benefit be significant.

Hmm. But what still confuses me is that it sounds like "You must not assume that tasks in distinct task runners are orderly executed. However, regarding task runners that you know have the same priority, you can assume that the tasks are orderly executed".

Does thinking like this help?

If tasks 1, 2, & 3 must run in order then they must be posted on the same task queue.

If tasks 1, 2, & 3 are unrelated it doesn't matter which task queue they are posted on, although they will often run in order.

[Kentaro Hara]

On Wed, Aug 3, 2016 at 12:34 AM, Alex Clarke <alexc...@google.com> wrote:

On 2 August 2016 at 16:25, Kentaro Hara <har...@chromium.org> wrote:

On Tue, Aug 2, 2016 at 11:34 PM, 'Alex Clarke' via platform-architecture-dev <platform-arc...@chromium.org> wrote:

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

Thanks a lot for the details, Alex and Sami!

In my defense both are technically true :) Just adding another task runner doesn't cause things to run out of order, but as soon as someone sets a different priority on that task runner (e.g., just by disabling it), the global task ordering will change. That's why I think as we move things to distinct task runners, we should assume that they can run out of order w.r.t. other task runners. Does that make sense?

The idea of the per-frame scheduler is, in the first place, that we want to deprioritize tasks in some task runners of third-party iframes. This means that we *must not* assume that tasks in distinct task runners are ordered, even if the distinct task runners happen to have the same priority. Am I understanding correctly?

If that is the case, I begin to think that it would be risky

 

That depends what you do with them :)

Introducing a new task runner which inherits the settings from another task queue for FrameBlamr isn't inherently risky because this isn't changing the order of execution.

If we want to introduce something new (and the spec gives us some latitude to do this) and change it's priority, that is risky.  Stuff will break, but fixing that might be worth it should be benefit be significant.

Hmm. But what still confuses me is that it sounds like "You must not assume that tasks in distinct task runners are orderly executed. However, regarding task runners that you know have the same priority, you can assume that the tasks are orderly executed".

Does thinking like this help?

If tasks 1, 2, & 3 must run in order then they must be posted on the same task queue.

If tasks 1, 2, & 3 are unrelated it doesn't matter which task queue they are posted on, although they will often run in order. 

Yes, I like this rule. But if we follow the rule, having more task runners has a risk of violating necessary task ordering, right? (Or, as the number of task runners increases, the complexity of considering what tasks should go to what task runners increases.) 

That's why I'm thinking that it would make more sense to post as many tasks to one specific task runner (e.g., Timer task runner) as possible instead of introducing Rendering task runner, Worker task runner etc until we really want to have a different scheduling policy.

[Alex Clarke]

On 2 August 2016 at 16:44, Kentaro Hara <har...@chromium.org> wrote:

On Wed, Aug 3, 2016 at 12:34 AM, Alex Clarke <alexc...@google.com> wrote:

On 2 August 2016 at 16:25, Kentaro Hara <har...@chromium.org> wrote:

On Tue, Aug 2, 2016 at 11:34 PM, 'Alex Clarke' via platform-architecture-dev <platform-arc...@chromium.org> wrote:

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

Thanks a lot for the details, Alex and Sami!

In my defense both are technically true :) Just adding another task runner doesn't cause things to run out of order, but as soon as someone sets a different priority on that task runner (e.g., just by disabling it), the global task ordering will change. That's why I think as we move things to distinct task runners, we should assume that they can run out of order w.r.t. other task runners. Does that make sense?

The idea of the per-frame scheduler is, in the first place, that we want to deprioritize tasks in some task runners of third-party iframes. This means that we *must not* assume that tasks in distinct task runners are ordered, even if the distinct task runners happen to have the same priority. Am I understanding correctly?

If that is the case, I begin to think that it would be risky

 

That depends what you do with them :)

Introducing a new task runner which inherits the settings from another task queue for FrameBlamr isn't inherently risky because this isn't changing the order of execution.

If we want to introduce something new (and the spec gives us some latitude to do this) and change it's priority, that is risky.  Stuff will break, but fixing that might be worth it should be benefit be significant.

Hmm. But what still confuses me is that it sounds like "You must not assume that tasks in distinct task runners are orderly executed. However, regarding task runners that you know have the same priority, you can assume that the tasks are orderly executed".

Does thinking like this help?

If tasks 1, 2, & 3 must run in order then they must be posted on the same task queue.

If tasks 1, 2, & 3 are unrelated it doesn't matter which task queue they are posted on, although they will often run in order. 

Yes, I like this rule. But if we follow the rule, having more task runners has a risk of violating necessary task ordering, right?

(Or, as the number of task runners increases, the complexity of considering what tasks should go to what task runners increases.) 

 

That's why I'm thinking that it would make more sense to post as many tasks to one specific task runner (e.g., Timer task runner) as possible instead of introducing Rendering task runner, Worker task runner etc until we really want to have a different scheduling policy.

I think I see what you mean, and it might be the right call to avoid adding new task runners unless we intend to for them to have different scheduling policies.

[Kentaro Hara]

On Wed, Aug 3, 2016 at 12:55 AM, Alex Clarke <alexc...@google.com> wrote:

On 2 August 2016 at 16:44, Kentaro Hara <har...@chromium.org> wrote:

On Wed, Aug 3, 2016 at 12:34 AM, Alex Clarke <alexc...@google.com> wrote:

On 2 August 2016 at 16:25, Kentaro Hara <har...@chromium.org> wrote:

On Tue, Aug 2, 2016 at 11:34 PM, 'Alex Clarke' via platform-architecture-dev <platform-arc...@chromium.org> wrote:

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

Thanks a lot for the details, Alex and Sami!

In my defense both are technically true :) Just adding another task runner doesn't cause things to run out of order, but as soon as someone sets a different priority on that task runner (e.g., just by disabling it), the global task ordering will change. That's why I think as we move things to distinct task runners, we should assume that they can run out of order w.r.t. other task runners. Does that make sense?

The idea of the per-frame scheduler is, in the first place, that we want to deprioritize tasks in some task runners of third-party iframes. This means that we *must not* assume that tasks in distinct task runners are ordered, even if the distinct task runners happen to have the same priority. Am I understanding correctly?

If that is the case, I begin to think that it would be risky

 

That depends what you do with them :)

Introducing a new task runner which inherits the settings from another task queue for FrameBlamr isn't inherently risky because this isn't changing the order of execution.

If we want to introduce something new (and the spec gives us some latitude to do this) and change it's priority, that is risky.  Stuff will break, but fixing that might be worth it should be benefit be significant.

Hmm. But what still confuses me is that it sounds like "You must not assume that tasks in distinct task runners are orderly executed. However, regarding task runners that you know have the same priority, you can assume that the tasks are orderly executed".

Does thinking like this help?

If tasks 1, 2, & 3 must run in order then they must be posted on the same task queue.

If tasks 1, 2, & 3 are unrelated it doesn't matter which task queue they are posted on, although they will often run in order. 

Yes, I like this rule. But if we follow the rule, having more task runners has a risk of violating necessary task ordering, right?

(Or, as the number of task runners increases, the complexity of considering what tasks should go to what task runners increases.) 

 

That's why I'm thinking that it would make more sense to post as many tasks to one specific task runner (e.g., Timer task runner) as possible instead of introducing Rendering task runner, Worker task runner etc until we really want to have a different scheduling policy.

I think I see what you mean, and it might be the right call to avoid adding new task runners unless we intend to for them to have different scheduling policies.

If that's the way to go, would it be a right thing to use the Timer task runner to post tasks that aren't related to loading or unthrottled?

[Sami Kyostila]

One option here would be to mirror the spec's various task sources: https://html.spec.whatwg.org/multipage/webappapis.html#generic-task-sources -- for example there is one (browser context -global) task source for timers. There are also a lot of special purpose task sources which we don't currently have.

Of course there are cases where we want more fine grained control. For example we want per-frame timer task queues so that we can throttle iframes independently as an intervention.

In general I think I'm agreeing that adding a new task runner should be done with caution. (There is one more benefit to a separate task runner however: it can get a more specific blame context. We could invent a new object to represent this distinction though -- a sub task runner perhaps?)

- Sami

[Alex Clarke]

On 2 August 2016 at 17:07, Kentaro Hara <har...@chromium.org> wrote:

On Wed, Aug 3, 2016 at 12:55 AM, Alex Clarke <alexc...@google.com> wrote:

On 2 August 2016 at 16:44, Kentaro Hara <har...@chromium.org> wrote:

On Wed, Aug 3, 2016 at 12:34 AM, Alex Clarke <alexc...@google.com> wrote:

On 2 August 2016 at 16:25, Kentaro Hara <har...@chromium.org> wrote:

On Tue, Aug 2, 2016 at 11:34 PM, 'Alex Clarke' via platform-architecture-dev <platform-arc...@chromium.org> wrote:

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

Thanks a lot for the details, Alex and Sami!

In my defense both are technically true :) Just adding another task runner doesn't cause things to run out of order, but as soon as someone sets a different priority on that task runner (e.g., just by disabling it), the global task ordering will change. That's why I think as we move things to distinct task runners, we should assume that they can run out of order w.r.t. other task runners. Does that make sense?

The idea of the per-frame scheduler is, in the first place, that we want to deprioritize tasks in some task runners of third-party iframes. This means that we *must not* assume that tasks in distinct task runners are ordered, even if the distinct task runners happen to have the same priority. Am I understanding correctly?

If that is the case, I begin to think that it would be risky

 

That depends what you do with them :)

Introducing a new task runner which inherits the settings from another task queue for FrameBlamr isn't inherently risky because this isn't changing the order of execution.

If we want to introduce something new (and the spec gives us some latitude to do this) and change it's priority, that is risky.  Stuff will break, but fixing that might be worth it should be benefit be significant.

Hmm. But what still confuses me is that it sounds like "You must not assume that tasks in distinct task runners are orderly executed. However, regarding task runners that you know have the same priority, you can assume that the tasks are orderly executed".

Does thinking like this help?

If tasks 1, 2, & 3 must run in order then they must be posted on the same task queue.

If tasks 1, 2, & 3 are unrelated it doesn't matter which task queue they are posted on, although they will often run in order. 

Yes, I like this rule. But if we follow the rule, having more task runners has a risk of violating necessary task ordering, right?

(Or, as the number of task runners increases, the complexity of considering what tasks should go to what task runners increases.) 

 

That's why I'm thinking that it would make more sense to post as many tasks to one specific task runner (e.g., Timer task runner) as possible instead of introducing Rendering task runner, Worker task runner etc until we really want to have a different scheduling policy.

I think I see what you mean, and it might be the right call to avoid adding new task runners unless we intend to for them to have different scheduling policies.

If that's the way to go, would it be a right thing to use the Timer task runner to post tasks that aren't related to loading or unthrottled?

Depends what they are.  We assume it's OK to temporally starve timer task runner to improve smoothness.  We also assume it's OK to throttle timer tasks associated with off-screen iframes.  If that's fine then it's probably OK.

I think the sub-task runner idea might be useful here, the idea is a sub timer-task runner will inherit all the scheduling decisions of it's parent but it shows up differently in tracing and FrameBlamr.

[Kentaro Hara]

+1 to the idea of the sub task runners. It would be a good idea to separate a notion of task classification from a notion of task ordering.

To sum up the discussion so far, here is my proposal:

- Introduce task runners per the spec. Specifically, introduce DOMManipulation/UserInteraction/Networking/HistoryTraversal task runners. Also introduce an Unthrottled task runner for limited internal use.

- Convert existing postTasks/timers/EventSenders to one of the task runners.

- The existing Timer task runner is replaced with DOMManipulation task runner. The existing Loading task runner is replaced with Networking task runner.

What do you think?

[Alex Clarke]

SGTM

[Sami Kyostila]

There's actually a dedicated timer task runner in the spec so I think we can keep that. It might be worth grepping to see what others are there -- the link I posted earlier only lists the generic ones.

It *seems* like it should be safe to throttle all of the task runners you listed aside from the unthrottled one.

- Sami

[Kentaro Hara]

There's actually a dedicated timer task runner in the spec so I think we can keep that. It might be worth grepping to see what others are there -- the link I posted earlier only lists the generic ones. 

 

It *seems* like it should be safe to throttle all of the task runners you listed aside from the unthrottled one.

Thanks!

I scanned the whatwg spec and found the following task sources:

- DOM manipulation task source

- user interaction task source

- networking task source

- history traversal task source

- embed task source

- media element event task source

- canvas blob serialisation task source

- microtask task source

- timer task source

- remote event task source

- WebSocket task source

- posted message task source

- unshipped port message task source

That said, I don't think we want to introduce so many task runners to src/components/scheduler/renderer/.

Then my proposal would be:

- Keep the current task runners (loading, timer, unthrottled) implemented in src/components/scheduler/renderer/ as is.

- TaskRunnerHelper provides a helper method for each task runner defined in the spec. For example, TaskRunnerHelper provides TaskRunnerHelper::getTaskRunner(TaskRunnerType, ...), where TaskRunnerType is an enum of DOMManipulation, UserInteration, Networking etc.

- TaskRunnerHelper::getTaskRunner(TaskRunnerType, ...) maps the TaskRunnerType to the task runners implemented in src/components/scheduler/renderer/. We can change the mapping in the future as necessary.

What do you think?

The remaining issue is what task runner we should use for "internal" tasks (e.g., WTF::Timers) and tasks that are not explicitly speced. Maybe do we want to introduce TaskRunnerType=Default? I guess it would be better to introduce a default task runner than reusing random task runners, but I'm not sure.

 

[Sami Kyostila]

That sounds good. (I'll leave it up to you whether you prefer getTaskRunner(TaskRunner::DOMManipulation) vs. getDOMManipulationTaskRunner()). Would these still be associated with frames/documents too?

I guess some kind of default or catch-all task runner is inevitable regardless of what taxonomy we come up with. I'm fine with calling it default as long as there's a big comment above it explaining how and when to use it.

- Sami

[Kentaro Hara]

I landed the change in https://codereview.chromium.org/2209023002/.

Let's move existing Timers/postTasks to the per-frame scheduler with the following APIs and see how it goes :)

- TaskRunnerHelper::get(TaskType, LocalFrame*)

- TaskRunnerHelper::get(TaskType, Document*)

- TaskRunnerHelper::get(TaskType, ExecutionContext*)

- TaskRunnerHelper::get(TaskType, ScriptState*)

[Ojan Vafai]

Sorry I didn't respond here sooner.

I don't see any correctness issues with using subtask runners to have our task runners actually match the spec, but it's not obvious to me that the benefit of better blaming is worth the cognitive overhead of having to understand which task runner to put your task in. I worry it might also slow you down in getting tasks onto the right task runner because you'll have to spend time figuring out the specs for each task and in some cases the spec won't outline which task runner it should run in.

Please don't block any of this work on my comments above though. I just wanted to share this in case you start down this route and realize that there's extra work to do trying to figure out the sub task runners. If this slows you down on getting everything in per-frame task runners, then it might not be worth it.

[Daniel Cheng]

Yeah, this was one of my concerns about adding more task runners. It doesn't seem to be a problem... yet. If it comes down to it and it's too confusing, it should be straightforward to remove some of the task runner types down the road.

Daniel

[Kentaro Hara]

We have a couple of options here:

a) Convert tasks/timers to the per-frame scheduler keeping the existing behavior. We can just move most of the tasks/timers to the unthrottled task runner. It's mostly mechanical work.

b) Introduce task runners defined in the spec and convert tasks/timers as conformant to the spec as possible.

c) In addition to b), introduce sub task runners.

I think b) would be a good balance point and that's what we're doing now.

[Ojan Vafai]

Oh, I see. I misunderstood the proposal. With b, there's some compatibility risk if we were to separately throttle the task runners. In theory it should be OK, but I think it's extremely unlikely that the spec matches real-world web compatibility since that's not what any browser implements AFAIK.

I agree that a) is a bad option. There's no benefit to moving to the unthrottled task runner. But there's a version of a) that makes more sense to me: Convert tasks/timers to the per-frame scheduler keeping the existing behavior. We can just move most of the tasks/timers to the timer task runner.

Putting all the tasks into the timer task runner minimizes compatibility risk, but still lets us get the benefits of per-frame scheduling, i.e. being able to separately throttle different frames.

[Kentaro Hara]

Oh, I see. I misunderstood the proposal. With b, there's some compatibility risk if we were to separately throttle the task runners. In theory it should be OK, but I think it's extremely unlikely that the spec matches real-world web compatibility since that's not what any browser implements AFAIK.

Sorry for not being clear. I'm planning to mitigate the compatibility risk as follows:

1) When Blink posts tasks/timers, use TaskType::XXX as conformant to the spec as possible (instead of using TaskType::Timer unconditionally).

2) The TaskType::XXX is mapped to the underlying task runners (timer/loading/unthrottled). The mapping is encapsulated in TaskRunnerHelper::get(). At the moment, almost all TaskTypes are mapped to the timer task runner (and I think this is what you want :) At this point we don't need to worry about compatibility too much. This is the per-frame scheduler project.

3) In middle term, we want to introduce more task runners and throttle some of them. At that point, we can just adjust the mapping in TaskRunnerHelper::get(). At this point we need to worry about compatibility.

Does it make sense? 

[Ojan Vafai]

On Wed, Aug 17, 2016 at 7:18 PM Kentaro Hara <har...@chromium.org> wrote:

Oh, I see. I misunderstood the proposal. With b, there's some compatibility risk if we were to separately throttle the task runners. In theory it should be OK, but I think it's extremely unlikely that the spec matches real-world web compatibility since that's not what any browser implements AFAIK.

Sorry for not being clear. I'm planning to mitigate the compatibility risk as follows:

1) When Blink posts tasks/timers, use TaskType::XXX as conformant to the spec as possible (instead of using TaskType::Timer unconditionally).

2) The TaskType::XXX is mapped to the underlying task runners (timer/loading/unthrottled). The mapping is encapsulated in TaskRunnerHelper::get(). At the moment, almost all TaskTypes are mapped to the timer task runner (and I think this is what you want :) At this point we don't need to worry about compatibility too much. This is the per-frame scheduler project.

3) In middle term, we want to introduce more task runners and throttle some of them. At that point, we can just adjust the mapping in TaskRunnerHelper::get(). At this point we need to worry about compatibility.

Does it make sense? 

Ah. Yes, that makes sense. I'm skeptical we'll want to do #3, but I agree with Daniel that it's very easy to merge task runners if we change our minds later on, so the only potential cost is that it slows you down now. Of course, you are the best judge of that. So, SGTM. :)

[Kentaro Hara]

FYI, Sami has landed a CL to enable the offscreen throttling by default. This means that the timer task runner on offscreen frames is throttled. Also we're currently mapping almost all TaskTypes (except Networking and Unthrottled) to the timer task runner. This means that as we move more tasks to the per-frame scheduler, we'll throttle more tasks.

[Ojan Vafai]

<3