cc: Limit ManageTiles calls to once per-frame on average

cc: Limit ManageTiles calls to once per-frame on average

This makes sure LTHI notifies the Scheduler of all ManageTiles calls
and then avoids a scheduled ManageTiles if we are already averaging
one or more ManageTiles per BeginImplFrame.

Limiting the scheduled ManageTiles to occur only if a ManageTiles
hasn't happened in the current frame can still result in two
ManageTiles per frame since we might schedule a ManageTiles at the
begining of a frame and then call explicitly when the commit finishes
if the main thread is in a high latency mode.

BUG=334749
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=245388

[enne (OOO), 2014-01-15 19:26:43]

https://codereview.chromium.org/130483010/diff/1/cc/scheduler/scheduler_state...
File cc/scheduler/scheduler_state_machine.cc (right):

https://codereview.chromium.org/130483010/diff/1/cc/scheduler/scheduler_state...
cc/scheduler/scheduler_state_machine.cc:510: // and then periodically after
that.  Limiting to every other frame prevents
Maybe I'm misunderstanding, but I don't think this comment is correct.

If you have a main thread-driven app that's hitting 60hz, then this will prevent
ManageTiles from ever being scheduled because the commit-based one every frame
will keep pushing this number forward.

[brianderson, 2014-01-15 19:39:08]

https://codereview.chromium.org/130483010/diff/1/cc/scheduler/scheduler_state...
File cc/scheduler/scheduler_state_machine.cc (right):

https://codereview.chromium.org/130483010/diff/1/cc/scheduler/scheduler_state...
cc/scheduler/scheduler_state_machine.cc:510: // and then periodically after
that.  Limiting to every other frame prevents
On 2014/01/15 19:26:44, enne wrote:
> Maybe I'm misunderstanding, but I don't think this comment is correct.
> 
> If you have a main thread-driven app that's hitting 60hz, then this will
prevent
> ManageTiles from ever being scheduled because the commit-based one every frame
> will keep pushing this number forward.

It's mainly a problem when the main thread is in the "high latency mode". In
that mode, we increment the current_frame_number_, draw a previous commit, and
then start the next commit. In comparison, the draw and commit are swapped in
the low latency mode.

In the low latency mode, what you say is true. We will ManageTiles after the
commit, which will prevent the subsequent draw from triggering a scheduled
ManageTiles.

In the high latency mode, we will run a scheduled ManageTiles after the draw but
can have the commit arrive afterwards, triggering an explicit ManageTiles within
the same frame.

[brianderson, 2014-01-16 02:07:13]

I initially tried keeping track of the number of ManageTiles calls in the
previous frame and only skipping the scheduled ManageTiles if that number was 2
or greater, but that resulted in alternating between 1 and 2 ManageTiles in the
high latency mode.

Instead of keeping a count per-frame, I've implemented a "funnel" variable.
Hopefully the comments make understanding how the funnel works intuitive.

Should pass existing tests. Will add a test that fails without this patch.

[epennerAtGoogle, 2014-01-16 03:01:24]

On 2014/01/16 02:07:13, Brian Anderson wrote:
> I initially tried keeping track of the number of ManageTiles calls in the
> previous frame and only skipping the scheduled ManageTiles if that number was
2
> or greater, but that resulted in alternating between 1 and 2 ManageTiles in
the
> high latency mode.
> 
> Instead of keeping a count per-frame, I've implemented a "funnel" variable.
> Hopefully the comments make understanding how the funnel works intuitive.
> 
> Should pass existing tests. Will add a test that fails without this patch.

Cool! Yeah I totally hadn't thought of 1-2-1-2 problem. Actually you want
2-1-1-1-1 which will end up being pretty darn close to optimal!

[brianderson, 2014-01-16 04:00:54]

Tests added. PTAL.

[enne (OOO), 2014-01-16 20:32:38]

lgtm

I think a maybe better but more complicated solution might be to delay the
scheduled manage tiles for high-latency mode until later in the frame in case a
commit comes in and removes the need for it.  However, I think the funnel is a
lot simpler and is a pretty elegant way to have better average performance over
time.

I think the worst this could do would be to delay the manage tiles caused by a
compositor-side scroll by a frame, since there's now a chance that you get no
manage tiles on a single frame (assuming that we get at most one explicit and
one scheduled manage tiles in the same frame).  I think that's probably ok,
though.  If there was work to do, the TileManager would already have the tiles
and would be working on prepainting.  It may slightly misprioritize them for a
frame, but I don't think that is likely to cause anything noticable.

[commit-bot: I haz the power, 2014-01-16 22:17:03]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/brianderson@chromium.org/130483010/190001

[commit-bot: I haz the power, 2014-01-17 00:12:59]

Change committed as 245388