scheduler: Tell v8 about the current RAIL mode

scheduler: Tell v8 about the current RAIL mode

This patch makes the Blink scheduler tell v8 about the current estimated
RAIL mode. This will allow v8 to choose different optimization
strategies based on what the user is doing. This patch implements
the default, response, animation and idle modes. A future patch will
add support for the load mode.

BUG=613520
Committed: https://crrev.com/a37c2a7427da99593f9358d18861b4d2343bcde2
Cr-Commit-Position: refs/heads/master@{#402852}

[Sami, 2016-06-24 17:55:00]

Description was changed from

==========
scheduler: Tell v8 about the current RAIL mode

This patch makes the Blink scheduler tell v8 about the current estimated
RAIL mode. This will allow v8 to choose different optimization
strategies based on what the user is doing. This patch implements
the default, response, animation and idle modes. A future patch will
add support for the load mode.

BUG=613520
==========

to

==========
scheduler: Tell v8 about the current RAIL mode

This patch makes the Blink scheduler tell v8 about the current estimated
RAIL mode. This will allow v8 to choose different optimization
strategies based on what the user is doing. This patch implements
the default, response, animation and idle modes. A future patch will
add support for the load mode.

BUG=613520
==========

[Sami, 2016-06-24 17:55:00]

skyostil@chromium.org changed reviewers:
+ alexclarke@chromium.org, hablich@chromium.org, hpayer@chromium.org,
ulan@chromium.org

[Sami, 2016-06-24 17:55:26]

PTAL. Doesn't do loading state yet but should still be good enough for testing
at least.

[alex clarke (OOO till 29th), 2016-06-24 20:26:37]

lgtm

[Sami, 2016-06-27 09:41:55]

skyostil@chromium.org changed reviewers:
+ jochen@chromium.org

[Sami, 2016-06-27 09:47:14]

Thanks. Jochen, PTAL at content/renderer/render_thread_impl.cc.

[jochen (gone - plz use gerrit), 2016-06-27 11:34:00]

why are you only telling the main thread isolate about the mode, but not the
background ones? (worker threads)

https://codereview.chromium.org/2093983002/diff/1/content/renderer/render_thr...
File content/renderer/render_thread_impl.cc (right):

https://codereview.chromium.org/2093983002/diff/1/content/renderer/render_thr...
content/renderer/render_thread_impl.cc:1164:
renderer_scheduler_->SetMainThreadIsolate(isolate);
when we shutdown webkit, the isolate is destroyed. should we tell the scheduler
about this as well?

[Sami, 2016-06-27 18:04:46]

> why are you only telling the main thread isolate about the mode, but not the
> background ones? (worker threads)

Fair point. Earlier I was thinking that workers aren't likely to participate in
the current RAIL mode directly, but your comment made me realize that it can
still be advantageous to make sure workers aren't stepping on the toes of the
main thread.

FWIW I chose a PostTask broadcast model because the RAIL mode changes relatively
rarely (once a frame at most, usually much less often than that). The
alternative pull model would have added overhead to each and every worker task.

https://codereview.chromium.org/2093983002/diff/1/content/renderer/render_thr...
File content/renderer/render_thread_impl.cc (right):

https://codereview.chromium.org/2093983002/diff/1/content/renderer/render_thr...
content/renderer/render_thread_impl.cc:1164:
renderer_scheduler_->SetMainThreadIsolate(isolate);
On 2016/06/27 11:34:00, jochen wrote:
> when we shutdown webkit, the isolate is destroyed. should we tell the
scheduler
> about this as well?

There is an explicit call to shut down the scheduler before that happens. The
scheduler won't touch anything blink-related after that, but I'll add code to
clear the isolate there to make it more explicit.

[nduca, 2016-06-27 22:11:35]

nduca@chromium.org changed reviewers:
+ nduca@chromium.org

[nduca, 2016-06-27 22:11:35]

+ben +fadi +andrey

[Hannes Payer (out of office), 2016-06-28 08:23:19]

Awesome Sami! LGTM

[ulan, 2016-06-28 13:09:08]

LGTM, thank you!

https://codereview.chromium.org/2093983002/diff/20001/components/scheduler/re...
File components/scheduler/renderer/renderer_scheduler_impl.cc (right):

https://codereview.chromium.org/2093983002/diff/20001/components/scheduler/re...
components/scheduler/renderer/renderer_scheduler_impl.cc:738:
new_policy.rail_mode = v8::PERFORMANCE_DEFAULT;
Looks like we will never get idle mode in foreground tab. Is it correct?
Would it make sense to set the mode to idle here? (I think telemetry defines
idle as not RAL)

https://codereview.chromium.org/2093983002/diff/20001/components/scheduler/re...
components/scheduler/renderer/renderer_scheduler_impl.cc:817:
new_policy.rail_mode = v8::PERFORMANCE_LOAD;
The CL description says "A future patch will add support for the load mode."
Is this path currently unreachable?

[rmcilroy, 2016-06-28 13:16:33]

rmcilroy@chromium.org changed reviewers:
+ rmcilroy@chromium.org

[rmcilroy, 2016-06-28 13:16:34]

https://codereview.chromium.org/2093983002/diff/20001/components/scheduler/re...
File components/scheduler/renderer/renderer_scheduler_impl.cc (right):

https://codereview.chromium.org/2093983002/diff/20001/components/scheduler/re...
components/scheduler/renderer/renderer_scheduler_impl.cc:738:
new_policy.rail_mode = v8::PERFORMANCE_DEFAULT;
On 2016/06/28 13:09:08, ulan wrote:
> Looks like we will never get idle mode in foreground tab. Is it correct?
> Would it make sense to set the mode to idle here? (I think telemetry defines
> idle as not RAL)

Sami and I chatted a bit about this. Are you sure you want Idle signal as
whenever not RAL - this will be most of the time, and I'm assuming you want to
use the Idle signal for something like the memory reducer (which currently
happens only when the page goes inactive - i.e., in the background or no frames
for a significant period of time).

[jochen (gone - plz use gerrit), 2016-06-28 13:20:58]

jochen@chromium.org changed reviewers:
- rmcilroy@chromium.org

[jochen (gone - plz use gerrit), 2016-06-28 13:20:58]

https://codereview.chromium.org/2093983002/diff/20001/content/renderer/render...
File content/renderer/render_thread_impl.cc (right):

https://codereview.chromium.org/2093983002/diff/20001/content/renderer/render...
content/renderer/render_thread_impl.cc:445: if (v8::Isolate* isolate =
v8::Isolate::GetCurrent())
the latest and greatest is introducing helper functions like
blink::MemoryPressureNotificationToWorkerThreadIsolates

[ulan, 2016-06-28 13:30:43]

https://codereview.chromium.org/2093983002/diff/20001/components/scheduler/re...
File components/scheduler/renderer/renderer_scheduler_impl.cc (right):

https://codereview.chromium.org/2093983002/diff/20001/components/scheduler/re...
components/scheduler/renderer/renderer_scheduler_impl.cc:738:
new_policy.rail_mode = v8::PERFORMANCE_DEFAULT;
On 2016/06/28 13:16:34, rmcilroy wrote:
> On 2016/06/28 13:09:08, ulan wrote:
> > Looks like we will never get idle mode in foreground tab. Is it correct?
> > Would it make sense to set the mode to idle here? (I think telemetry defines
> > idle as not RAL)
> 
> Sami and I chatted a bit about this. Are you sure you want Idle signal as
> whenever not RAL - this will be most of the time, and I'm assuming you want to
> use the Idle signal for something like the memory reducer (which currently
> happens only when the page goes inactive - i.e., in the background or no
frames
> for a significant period of time).

Indeed, idle defined as "page goes inactive" would be much more useful signal
for us than idle defined as "not RAL". I guess it is difficult to detect "page
goes inactive" for foreground tab?

[Sami, 2016-06-28 13:53:19]

https://codereview.chromium.org/2093983002/diff/20001/components/scheduler/re...
File components/scheduler/renderer/renderer_scheduler_impl.cc (right):

https://codereview.chromium.org/2093983002/diff/20001/components/scheduler/re...
components/scheduler/renderer/renderer_scheduler_impl.cc:738:
new_policy.rail_mode = v8::PERFORMANCE_DEFAULT;
On 2016/06/28 13:30:43, ulan wrote:
> On 2016/06/28 13:16:34, rmcilroy wrote:
> > On 2016/06/28 13:09:08, ulan wrote:
> > > Looks like we will never get idle mode in foreground tab. Is it correct?
> > > Would it make sense to set the mode to idle here? (I think telemetry
defines
> > > idle as not RAL)
> > 
> > Sami and I chatted a bit about this. Are you sure you want Idle signal as
> > whenever not RAL - this will be most of the time, and I'm assuming you want
to
> > use the Idle signal for something like the memory reducer (which currently
> > happens only when the page goes inactive - i.e., in the background or no
> frames
> > for a significant period of time).
> 
> Indeed, idle defined as "page goes inactive" would be much more useful signal
> for us than idle defined as "not RAL". I guess it is difficult to detect "page
> goes inactive" for foreground tab?

It shouldn't be too difficult, but we just have to define what "I" actually
means. I've added a TODO -- let's talk about what the most useful signal is
(both for v8 and speed-infra).

https://codereview.chromium.org/2093983002/diff/20001/components/scheduler/re...
components/scheduler/renderer/renderer_scheduler_impl.cc:817:
new_policy.rail_mode = v8::PERFORMANCE_LOAD;
On 2016/06/28 13:09:08, ulan wrote:
> The CL description says "A future patch will add support for the load mode."
> Is this path currently unreachable?

Yes, we currently don't enter the loading use case (there's a TODO elsewhere in
this file). My tentative plan is to stay in L until first contentful/meaningful
paint (There's a slight chicken and egg problem here since we want to
deprioritize rendering during loading, but you need to render to get *out* of L.
I think we just need to experiment a bit more.)

https://codereview.chromium.org/2093983002/diff/20001/content/renderer/render...
File content/renderer/render_thread_impl.cc (right):

https://codereview.chromium.org/2093983002/diff/20001/content/renderer/render...
content/renderer/render_thread_impl.cc:445: if (v8::Isolate* isolate =
v8::Isolate::GetCurrent())
On 2016/06/28 13:20:58, jochen wrote:
> the latest and greatest is introducing helper functions like
> blink::MemoryPressureNotificationToWorkerThreadIsolates

I am now aligned with the latest and greatest trends.

With this change I'm now setting the RAIL mode from a thread different from the
one owning the isolate. Hannes/Ulan/Michael, is that okay?

[jochen (gone - plz use gerrit), 2016-06-28 14:09:38]

lgtm

[Hannes Payer (out of office), 2016-06-28 14:18:12]

On 2016/06/28 13:30:43, ulan wrote:
>
https://codereview.chromium.org/2093983002/diff/20001/components/scheduler/re...
> File components/scheduler/renderer/renderer_scheduler_impl.cc (right):
> 
>
https://codereview.chromium.org/2093983002/diff/20001/components/scheduler/re...
> components/scheduler/renderer/renderer_scheduler_impl.cc:738:
> new_policy.rail_mode = v8::PERFORMANCE_DEFAULT;
> On 2016/06/28 13:16:34, rmcilroy wrote:
> > On 2016/06/28 13:09:08, ulan wrote:
> > > Looks like we will never get idle mode in foreground tab. Is it correct?
> > > Would it make sense to set the mode to idle here? (I think telemetry
defines
> > > idle as not RAL)
> > 
> > Sami and I chatted a bit about this. Are you sure you want Idle signal as
> > whenever not RAL - this will be most of the time, and I'm assuming you want
to
> > use the Idle signal for something like the memory reducer (which currently
> > happens only when the page goes inactive - i.e., in the background or no
> frames
> > for a significant period of time).
> 
> Indeed, idle defined as "page goes inactive" would be much more useful signal
> for us than idle defined as "not RAL". I guess it is difficult to detect "page
> goes inactive" for foreground tab?

not RAL would mean that the tab is potentially inactive. If the tab is not RAL
for
quite some time, we could assume that it is indeed inactive. Maybe we should
notify I then.

[Hannes Payer (out of office), 2016-06-28 14:39:40]

On 2016/06/28 13:53:19, Sami wrote:
>
https://codereview.chromium.org/2093983002/diff/20001/components/scheduler/re...
> File components/scheduler/renderer/renderer_scheduler_impl.cc (right):
> 
>
https://codereview.chromium.org/2093983002/diff/20001/components/scheduler/re...
> components/scheduler/renderer/renderer_scheduler_impl.cc:738:
> new_policy.rail_mode = v8::PERFORMANCE_DEFAULT;
> On 2016/06/28 13:30:43, ulan wrote:
> > On 2016/06/28 13:16:34, rmcilroy wrote:
> > > On 2016/06/28 13:09:08, ulan wrote:
> > > > Looks like we will never get idle mode in foreground tab. Is it correct?
> > > > Would it make sense to set the mode to idle here? (I think telemetry
> defines
> > > > idle as not RAL)
> > > 
> > > Sami and I chatted a bit about this. Are you sure you want Idle signal as
> > > whenever not RAL - this will be most of the time, and I'm assuming you
want
> to
> > > use the Idle signal for something like the memory reducer (which currently
> > > happens only when the page goes inactive - i.e., in the background or no
> > frames
> > > for a significant period of time).
> > 
> > Indeed, idle defined as "page goes inactive" would be much more useful
signal
> > for us than idle defined as "not RAL". I guess it is difficult to detect
"page
> > goes inactive" for foreground tab?
> 
> It shouldn't be too difficult, but we just have to define what "I" actually
> means. I've added a TODO -- let's talk about what the most useful signal is
> (both for v8 and speed-infra).
> 
>
https://codereview.chromium.org/2093983002/diff/20001/components/scheduler/re...
> components/scheduler/renderer/renderer_scheduler_impl.cc:817:
> new_policy.rail_mode = v8::PERFORMANCE_LOAD;
> On 2016/06/28 13:09:08, ulan wrote:
> > The CL description says "A future patch will add support for the load mode."
> > Is this path currently unreachable?
> 
> Yes, we currently don't enter the loading use case (there's a TODO elsewhere
in
> this file). My tentative plan is to stay in L until first
contentful/meaningful
> paint (There's a slight chicken and egg problem here since we want to
> deprioritize rendering during loading, but you need to render to get *out* of
L.
> I think we just need to experiment a bit more.)
> 
>
https://codereview.chromium.org/2093983002/diff/20001/content/renderer/render...
> File content/renderer/render_thread_impl.cc (right):
> 
>
https://codereview.chromium.org/2093983002/diff/20001/content/renderer/render...
> content/renderer/render_thread_impl.cc:445: if (v8::Isolate* isolate =
> v8::Isolate::GetCurrent())
> On 2016/06/28 13:20:58, jochen wrote:
> > the latest and greatest is introducing helper functions like
> > blink::MemoryPressureNotificationToWorkerThreadIsolates
> 
> I am now aligned with the latest and greatest trends.
> 
> With this change I'm now setting the RAIL mode from a thread different from
the
> one owning the isolate. Hannes/Ulan/Michael, is that okay?

OK, we will change the state variable v8 internally to atomic.

[rmcilroy, 2016-06-28 14:42:50]

On 2016/06/28 14:18:12, Hannes Payer (slow) wrote:
> On 2016/06/28 13:30:43, ulan wrote:
> >
>
https://codereview.chromium.org/2093983002/diff/20001/components/scheduler/re...
> > File components/scheduler/renderer/renderer_scheduler_impl.cc (right):
> > 
> >
>
https://codereview.chromium.org/2093983002/diff/20001/components/scheduler/re...
> > components/scheduler/renderer/renderer_scheduler_impl.cc:738:
> > new_policy.rail_mode = v8::PERFORMANCE_DEFAULT;
> > On 2016/06/28 13:16:34, rmcilroy wrote:
> > > On 2016/06/28 13:09:08, ulan wrote:
> > > > Looks like we will never get idle mode in foreground tab. Is it correct?
> > > > Would it make sense to set the mode to idle here? (I think telemetry
> defines
> > > > idle as not RAL)
> > > 
> > > Sami and I chatted a bit about this. Are you sure you want Idle signal as
> > > whenever not RAL - this will be most of the time, and I'm assuming you
want
> to
> > > use the Idle signal for something like the memory reducer (which currently
> > > happens only when the page goes inactive - i.e., in the background or no
> > frames
> > > for a significant period of time).
> > 
> > Indeed, idle defined as "page goes inactive" would be much more useful
signal
> > for us than idle defined as "not RAL". I guess it is difficult to detect
"page
> > goes inactive" for foreground tab?
> 
> not RAL would mean that the tab is potentially inactive. If the tab is not RAL
> for
> quite some time, we could assume that it is indeed inactive. Maybe we should
> notify I then.

Should we call the mode v8::PERFORMANCE_INACTIVE in that case, to avoid
confusing things with idle-time / idle-tasks?

[ulan, 2016-06-28 14:47:25]

https://codereview.chromium.org/2093983002/diff/20001/components/scheduler/re...
File components/scheduler/renderer/renderer_scheduler_impl.cc (right):

https://codereview.chromium.org/2093983002/diff/20001/components/scheduler/re...
components/scheduler/renderer/renderer_scheduler_impl.cc:738:
new_policy.rail_mode = v8::PERFORMANCE_DEFAULT;
On 2016/06/28 13:53:18, Sami wrote:
> On 2016/06/28 13:30:43, ulan wrote:
> > On 2016/06/28 13:16:34, rmcilroy wrote:
> > > On 2016/06/28 13:09:08, ulan wrote:
> > > > Looks like we will never get idle mode in foreground tab. Is it correct?
> > > > Would it make sense to set the mode to idle here? (I think telemetry
> defines
> > > > idle as not RAL)
> > > 
> > > Sami and I chatted a bit about this. Are you sure you want Idle signal as
> > > whenever not RAL - this will be most of the time, and I'm assuming you
want
> to
> > > use the Idle signal for something like the memory reducer (which currently
> > > happens only when the page goes inactive - i.e., in the background or no
> > frames
> > > for a significant period of time).
> > 
> > Indeed, idle defined as "page goes inactive" would be much more useful
signal
> > for us than idle defined as "not RAL". I guess it is difficult to detect
"page
> > goes inactive" for foreground tab?
> 
> It shouldn't be too difficult, but we just have to define what "I" actually
> means. I've added a TODO -- let's talk about what the most useful signal is
> (both for v8 and speed-infra).

Thank you! Related note: we discussed DEFAULT mode with Hannes and Michael. The
transition from RAIL to DEFAULT is problematic and adds unnecessary complexity.
Effectively we will have RAILD instead of RAIL, but our heuristics for DEFAULT
will probably be the same as for A.

We are going to remove DEFAULT mode and make A the default.

[Sami, 2016-06-28 15:01:37]

https://codereview.chromium.org/2093983002/diff/20001/components/scheduler/re...
File components/scheduler/renderer/renderer_scheduler_impl.cc (right):

https://codereview.chromium.org/2093983002/diff/20001/components/scheduler/re...
components/scheduler/renderer/renderer_scheduler_impl.cc:738:
new_policy.rail_mode = v8::PERFORMANCE_DEFAULT;
On 2016/06/28 14:47:25, ulan wrote:
> On 2016/06/28 13:53:18, Sami wrote:
> > On 2016/06/28 13:30:43, ulan wrote:
> > > On 2016/06/28 13:16:34, rmcilroy wrote:
> > > > On 2016/06/28 13:09:08, ulan wrote:
> > > > > Looks like we will never get idle mode in foreground tab. Is it
correct?
> > > > > Would it make sense to set the mode to idle here? (I think telemetry
> > defines
> > > > > idle as not RAL)
> > > > 
> > > > Sami and I chatted a bit about this. Are you sure you want Idle signal
as
> > > > whenever not RAL - this will be most of the time, and I'm assuming you
> want
> > to
> > > > use the Idle signal for something like the memory reducer (which
currently
> > > > happens only when the page goes inactive - i.e., in the background or no
> > > frames
> > > > for a significant period of time).
> > > 
> > > Indeed, idle defined as "page goes inactive" would be much more useful
> signal
> > > for us than idle defined as "not RAL". I guess it is difficult to detect
> "page
> > > goes inactive" for foreground tab?
> > 
> > It shouldn't be too difficult, but we just have to define what "I" actually
> > means. I've added a TODO -- let's talk about what the most useful signal is
> > (both for v8 and speed-infra).
> 
> Thank you! Related note: we discussed DEFAULT mode with Hannes and Michael.
The
> transition from RAIL to DEFAULT is problematic and adds unnecessary
complexity.
> Effectively we will have RAILD instead of RAIL, but our heuristics for DEFAULT
> will probably be the same as for A.
> 
> We are going to remove DEFAULT mode and make A the default.

Right, I was wondering about the difference between those two. I can make the
same change in my patch. It does, however, mean that we're most of the time in
ANIMATE mode.  Would it be worth switching to IDLE when we're not actively
drawing and after a delay to INACTIVE that Ross suggested? Or should we just
stay in ANIMATE and go to IDLE after a longer delay?

[ulan, 2016-06-28 15:31:49]

On 2016/06/28 15:01:37, Sami wrote:
>
https://codereview.chromium.org/2093983002/diff/20001/components/scheduler/re...
> File components/scheduler/renderer/renderer_scheduler_impl.cc (right):
> 
>
https://codereview.chromium.org/2093983002/diff/20001/components/scheduler/re...
> components/scheduler/renderer/renderer_scheduler_impl.cc:738:
> new_policy.rail_mode = v8::PERFORMANCE_DEFAULT;
> On 2016/06/28 14:47:25, ulan wrote:
> > On 2016/06/28 13:53:18, Sami wrote:
> > > On 2016/06/28 13:30:43, ulan wrote:
> > > > On 2016/06/28 13:16:34, rmcilroy wrote:
> > > > > On 2016/06/28 13:09:08, ulan wrote:
> > > > > > Looks like we will never get idle mode in foreground tab. Is it
> correct?
> > > > > > Would it make sense to set the mode to idle here? (I think telemetry
> > > defines
> > > > > > idle as not RAL)
> > > > > 
> > > > > Sami and I chatted a bit about this. Are you sure you want Idle signal
> as
> > > > > whenever not RAL - this will be most of the time, and I'm assuming you
> > want
> > > to
> > > > > use the Idle signal for something like the memory reducer (which
> currently
> > > > > happens only when the page goes inactive - i.e., in the background or
no
> > > > frames
> > > > > for a significant period of time).
> > > > 
> > > > Indeed, idle defined as "page goes inactive" would be much more useful
> > signal
> > > > for us than idle defined as "not RAL". I guess it is difficult to detect
> > "page
> > > > goes inactive" for foreground tab?
> > > 
> > > It shouldn't be too difficult, but we just have to define what "I"
actually
> > > means. I've added a TODO -- let's talk about what the most useful signal
is
> > > (both for v8 and speed-infra).
> > 
> > Thank you! Related note: we discussed DEFAULT mode with Hannes and Michael.
> The
> > transition from RAIL to DEFAULT is problematic and adds unnecessary
> complexity.
> > Effectively we will have RAILD instead of RAIL, but our heuristics for
DEFAULT
> > will probably be the same as for A.
> > 
> > We are going to remove DEFAULT mode and make A the default.
> 
> Right, I was wondering about the difference between those two. I can make the
> same change in my patch.
That would be great. I uploaded V8 patch:
https://codereview.chromium.org/2108503003/

> It does, however, mean that we're most of the time in
> ANIMATE mode.  Would it be worth switching to IDLE when we're not actively
> drawing and after a delay to INACTIVE that Ross suggested? Or should we just
> stay in ANIMATE and go to IDLE after a longer delay?
I don't think we can make good use of intermediate state between ANIMATE and
IDLE.
So the latter (ANIMATE -> IDLE) would be better for us. 

Btw, maybe I misunderstood, but I thought Ross was suggesting to rename
PERFORMANCE_IDLE to PERFORMANCE_INACTIVE (without adding another mode).

[Hannes Payer (out of office), 2016-06-28 16:55:18]

On 2016/06/28 15:31:49, ulan wrote:
> On 2016/06/28 15:01:37, Sami wrote:
> >
>
https://codereview.chromium.org/2093983002/diff/20001/components/scheduler/re...
> > File components/scheduler/renderer/renderer_scheduler_impl.cc (right):
> > 
> >
>
https://codereview.chromium.org/2093983002/diff/20001/components/scheduler/re...
> > components/scheduler/renderer/renderer_scheduler_impl.cc:738:
> > new_policy.rail_mode = v8::PERFORMANCE_DEFAULT;
> > On 2016/06/28 14:47:25, ulan wrote:
> > > On 2016/06/28 13:53:18, Sami wrote:
> > > > On 2016/06/28 13:30:43, ulan wrote:
> > > > > On 2016/06/28 13:16:34, rmcilroy wrote:
> > > > > > On 2016/06/28 13:09:08, ulan wrote:
> > > > > > > Looks like we will never get idle mode in foreground tab. Is it
> > correct?
> > > > > > > Would it make sense to set the mode to idle here? (I think
telemetry
> > > > defines
> > > > > > > idle as not RAL)
> > > > > > 
> > > > > > Sami and I chatted a bit about this. Are you sure you want Idle
signal
> > as
> > > > > > whenever not RAL - this will be most of the time, and I'm assuming
you
> > > want
> > > > to
> > > > > > use the Idle signal for something like the memory reducer (which
> > currently
> > > > > > happens only when the page goes inactive - i.e., in the background
or
> no
> > > > > frames
> > > > > > for a significant period of time).
> > > > > 
> > > > > Indeed, idle defined as "page goes inactive" would be much more useful
> > > signal
> > > > > for us than idle defined as "not RAL". I guess it is difficult to
detect
> > > "page
> > > > > goes inactive" for foreground tab?
> > > > 
> > > > It shouldn't be too difficult, but we just have to define what "I"
> actually
> > > > means. I've added a TODO -- let's talk about what the most useful signal
> is
> > > > (both for v8 and speed-infra).
> > > 
> > > Thank you! Related note: we discussed DEFAULT mode with Hannes and
Michael.
> > The
> > > transition from RAIL to DEFAULT is problematic and adds unnecessary
> > complexity.
> > > Effectively we will have RAILD instead of RAIL, but our heuristics for
> DEFAULT
> > > will probably be the same as for A.
> > > 
> > > We are going to remove DEFAULT mode and make A the default.
> > 
> > Right, I was wondering about the difference between those two. I can make
the
> > same change in my patch.
> That would be great. I uploaded V8 patch:
> https://codereview.chromium.org/2108503003/
> 
> > It does, however, mean that we're most of the time in
> > ANIMATE mode.  Would it be worth switching to IDLE when we're not actively
> > drawing and after a delay to INACTIVE that Ross suggested? Or should we just
> > stay in ANIMATE and go to IDLE after a longer delay?
> I don't think we can make good use of intermediate state between ANIMATE and
> IDLE.
> So the latter (ANIMATE -> IDLE) would be better for us. 
> 
> Btw, maybe I misunderstood, but I thought Ross was suggesting to rename
> PERFORMANCE_IDLE to PERFORMANCE_INACTIVE (without adding another mode).

I think we just need the idle aka inaktive state for memory reducing GCs. If we
see that we can take advantage of an intermediate idle state, we can always add
it later.

[Sami, 2016-06-28 17:03:58]

On 2016/06/28 16:55:18, Hannes Payer (slow) wrote:
> I think we just need the idle aka inaktive state for memory reducing GCs. If
we
> see that we can take advantage of an intermediate idle state, we can always
add
> it later.

That sounds good to me. Let's stick to RAIL for now.

[rmcilroy, 2016-06-28 17:17:05]

On 2016/06/28 15:31:49, ulan wrote:
> On 2016/06/28 15:01:37, Sami wrote:
> >
>
https://codereview.chromium.org/2093983002/diff/20001/components/scheduler/re...
> > File components/scheduler/renderer/renderer_scheduler_impl.cc (right):
> > 
> >
>
https://codereview.chromium.org/2093983002/diff/20001/components/scheduler/re...
> > components/scheduler/renderer/renderer_scheduler_impl.cc:738:
> > new_policy.rail_mode = v8::PERFORMANCE_DEFAULT;
> > On 2016/06/28 14:47:25, ulan wrote:
> > > On 2016/06/28 13:53:18, Sami wrote:
> > > > On 2016/06/28 13:30:43, ulan wrote:
> > > > > On 2016/06/28 13:16:34, rmcilroy wrote:
> > > > > > On 2016/06/28 13:09:08, ulan wrote:
> > > > > > > Looks like we will never get idle mode in foreground tab. Is it
> > correct?
> > > > > > > Would it make sense to set the mode to idle here? (I think
telemetry
> > > > defines
> > > > > > > idle as not RAL)
> > > > > > 
> > > > > > Sami and I chatted a bit about this. Are you sure you want Idle
signal
> > as
> > > > > > whenever not RAL - this will be most of the time, and I'm assuming
you
> > > want
> > > > to
> > > > > > use the Idle signal for something like the memory reducer (which
> > currently
> > > > > > happens only when the page goes inactive - i.e., in the background
or
> no
> > > > > frames
> > > > > > for a significant period of time).
> > > > > 
> > > > > Indeed, idle defined as "page goes inactive" would be much more useful
> > > signal
> > > > > for us than idle defined as "not RAL". I guess it is difficult to
detect
> > > "page
> > > > > goes inactive" for foreground tab?
> > > > 
> > > > It shouldn't be too difficult, but we just have to define what "I"
> actually
> > > > means. I've added a TODO -- let's talk about what the most useful signal
> is
> > > > (both for v8 and speed-infra).
> > > 
> > > Thank you! Related note: we discussed DEFAULT mode with Hannes and
Michael.
> > The
> > > transition from RAIL to DEFAULT is problematic and adds unnecessary
> > complexity.
> > > Effectively we will have RAILD instead of RAIL, but our heuristics for
> DEFAULT
> > > will probably be the same as for A.
> > > 
> > > We are going to remove DEFAULT mode and make A the default.
> > 
> > Right, I was wondering about the difference between those two. I can make
the
> > same change in my patch.
> That would be great. I uploaded V8 patch:
> https://codereview.chromium.org/2108503003/
> 
> > It does, however, mean that we're most of the time in
> > ANIMATE mode.  Would it be worth switching to IDLE when we're not actively
> > drawing and after a delay to INACTIVE that Ross suggested? Or should we just
> > stay in ANIMATE and go to IDLE after a longer delay?
> I don't think we can make good use of intermediate state between ANIMATE and
> IDLE.
> So the latter (ANIMATE -> IDLE) would be better for us. 
> 
> Btw, maybe I misunderstood, but I thought Ross was suggesting to rename
> PERFORMANCE_IDLE to PERFORMANCE_INACTIVE (without adding another mode).

Yes, I was just suggesting renaming, not adding another state.

[Sami, 2016-06-29 12:36:11]

Thanks all. I'll land this for experimentation and find out how we want to deal
with L and I as a follow-up.

[Sami, 2016-06-29 12:36:21]

The CQ bit was checked by skyostil@chromium.org

[Sami, 2016-06-29 12:36:21]

The patchset sent to the CQ was uploaded after l-g-t-m from
alexclarke@chromium.org, hpayer@chromium.org, ulan@chromium.org,
jochen@chromium.org
Link to the patchset: https://codereview.chromium.org/2093983002/#ps80001
(title: "Update test expectations")

[commit-bot: I haz the power, 2016-06-29 12:36:32]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-06-29 13:08:24]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-29 13:08:26]

Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Sami, 2016-06-29 16:07:05]

The CQ bit was checked by skyostil@chromium.org

[Sami, 2016-06-29 16:07:07]

The patchset sent to the CQ was uploaded after l-g-t-m from jochen@chromium.org,
hpayer@chromium.org, ulan@chromium.org, alexclarke@chromium.org
Link to the patchset: https://codereview.chromium.org/2093983002/#ps100001
(title: "Add missing gyp dependencies")

[commit-bot: I haz the power, 2016-06-29 16:07:36]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-06-29 17:30:20]

Description was changed from

==========
scheduler: Tell v8 about the current RAIL mode

This patch makes the Blink scheduler tell v8 about the current estimated
RAIL mode. This will allow v8 to choose different optimization
strategies based on what the user is doing. This patch implements
the default, response, animation and idle modes. A future patch will
add support for the load mode.

BUG=613520
==========

to

==========
scheduler: Tell v8 about the current RAIL mode

This patch makes the Blink scheduler tell v8 about the current estimated
RAIL mode. This will allow v8 to choose different optimization
strategies based on what the user is doing. This patch implements
the default, response, animation and idle modes. A future patch will
add support for the load mode.

BUG=613520
==========

[commit-bot: I haz the power, 2016-06-29 17:30:23]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-06-29 17:30:52]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-06-29 17:32:14]

Description was changed from

==========
scheduler: Tell v8 about the current RAIL mode

This patch makes the Blink scheduler tell v8 about the current estimated
RAIL mode. This will allow v8 to choose different optimization
strategies based on what the user is doing. This patch implements
the default, response, animation and idle modes. A future patch will
add support for the load mode.

BUG=613520
==========

to

==========
scheduler: Tell v8 about the current RAIL mode

This patch makes the Blink scheduler tell v8 about the current estimated
RAIL mode. This will allow v8 to choose different optimization
strategies based on what the user is doing. This patch implements
the default, response, animation and idle modes. A future patch will
add support for the load mode.

BUG=613520

Committed: https://crrev.com/a37c2a7427da99593f9358d18861b4d2343bcde2
Cr-Commit-Position: refs/heads/master@{#402852}
==========

[commit-bot: I haz the power, 2016-06-29 17:32:27]

Patchset 6 (id:??) landed as
https://crrev.com/a37c2a7427da99593f9358d18861b4d2343bcde2
Cr-Commit-Position: refs/heads/master@{#402852}