Retry tree activation if raster tasks finish during first half of vsync interval

Retry tree activation if raster tasks finish during first half of vsync interval

When we attempt pending tree activation in response to a vsync and it
fails, we currently have to wait a full vsync interval to try again.
This patch instead checks if all pending raster tasks complete within a
grace period (8 ms) from the previous vsync. If so, we retry tree
activation and render the new frame immediately if it succeeds. This
decreases the latency of Blink-driven animations.

BUG=161828

[Sami, 2013-04-18 10:50:57]

Straw man #2 for JSGameBench.

[Sami, 2013-04-18 10:59:58]

Here's the bug that might help us remove the magic constant:
http://crbug.com/224602.

[danakj, 2013-04-18 15:13:59]

On 2013/04/18 10:59:58, Sami wrote:
> Here's the bug that might help us remove the magic constant:
> http://crbug.com/224602.

What if we just always waited instead of trying twice?

[Sami, 2013-04-18 15:19:42]

On 2013/04/18 15:13:59, danakj wrote:
> On 2013/04/18 10:59:58, Sami wrote:
> > Here's the bug that might help us remove the magic constant:
> > http://crbug.com/224602.
> 
> What if we just always waited instead of trying twice?

Do you mean we'd only activate the tree after all raster tasks have completed?
One problem with that is that you wouldn't see any low res tiles.

[danakj, 2013-04-18 15:21:07]

On 2013/04/18 15:19:42, Sami wrote:
> On 2013/04/18 15:13:59, danakj wrote:
> > On 2013/04/18 10:59:58, Sami wrote:
> > > Here's the bug that might help us remove the magic constant:
> > > http://crbug.com/224602.
> > 
> > What if we just always waited instead of trying twice?
> 
> Do you mean we'd only activate the tree after all raster tasks have completed?
> One problem with that is that you wouldn't see any low res tiles.

I don't think I follow, can you explain?

[Sami, 2013-04-18 15:25:05]

On 2013/04/18 15:21:07, danakj wrote:
> On 2013/04/18 15:19:42, Sami wrote:
> > On 2013/04/18 15:13:59, danakj wrote:
> > > On 2013/04/18 10:59:58, Sami wrote:
> > > > Here's the bug that might help us remove the magic constant:
> > > > http://crbug.com/224602.
> > > 
> > > What if we just always waited instead of trying twice?
> > 
> > Do you mean we'd only activate the tree after all raster tasks have
completed?
> > One problem with that is that you wouldn't see any low res tiles.
> 
> I don't think I follow, can you explain?

I may have misunderstood your suggestion. It sounded like you meant that we
would always wait for raster tasks to complete instead of essentially polling
for tree activation at vsync like we do now. If we did that, then I think we'd
have much more checkerboarding than now because tree activation would require
all rasters to complete and not just some of them.

[danakj, 2013-04-18 15:27:57]

On 2013/04/18 15:25:05, Sami wrote:
> On 2013/04/18 15:21:07, danakj wrote:
> > On 2013/04/18 15:19:42, Sami wrote:
> > > On 2013/04/18 15:13:59, danakj wrote:
> > > > On 2013/04/18 10:59:58, Sami wrote:
> > > > > Here's the bug that might help us remove the magic constant:
> > > > > http://crbug.com/224602.
> > > > 
> > > > What if we just always waited instead of trying twice?
> > > 
> > > Do you mean we'd only activate the tree after all raster tasks have
> completed?
> > > One problem with that is that you wouldn't see any low res tiles.
> > 
> > I don't think I follow, can you explain?
> 
> I may have misunderstood your suggestion. It sounded like you meant that we
> would always wait for raster tasks to complete instead of essentially polling
> for tree activation at vsync like we do now. If we did that, then I think we'd
> have much more checkerboarding than now because tree activation would require
> all rasters to complete and not just some of them.

I was suggesting instead of vsync->try->wait 8ms->try to do vsync->wait
8ms->try. I'm not sure if that's a great idea or not, but trying twice seems a
bit silly if we can just reallign it.

[Sami, 2013-04-18 15:35:55]

> I was suggesting instead of vsync->try->wait 8ms->try to do vsync->wait
> 8ms->try. I'm not sure if that's a great idea or not, but trying twice seems a
> bit silly if we can just reallign it.

Ah, got it. We probably don't want to realign tree activation to ½ vsync all the
time because often activating at vsync is the right thing to do if all rasters
are done. Maybe we could check if there are pending rasters at vsync and only
then delay the activation 8 ms.

BTW, since I didn't mention it here yet, the reason for adding the 8 ms limit
was to keep the renderer from spamming too many frames to the browser. If we
could use swap acks to better throttle the renderer I think we could do without
the limit.

[brianderson, 2013-04-18 22:24:43]

I like where this patch is going.

I would vote for using swap acks to throttle everything rather than the 1/2
vsync timer.

How expensive is the check to see if we can activate? Can we make the check
cheap enough so that we can check for it after each rasterization on the worker
threads?

Currently, it looks like this patch only notifies the main thread to check for
activation once all rasterizations are done. Activation could occur once a
subset (visible and high res) of the rasterizations are done though.

[danakj, 2013-04-18 22:39:34]

On Thu, Apr 18, 2013 at 6:24 PM, <brianderson@chromium.org> wrote:

> I like where this patch is going.
>
> I would vote for using swap acks to throttle everything rather than the 1/2
> vsync timer.
>
> How expensive is the check to see if we can activate? Can we make the check
> cheap enough so that we can check for it after each rasterization on the
> worker
> threads?
>

Currently it's a tree walk - expensive.

To do better would be something like... keep a list of all the tiles that
need to be activated by doing a single tree walk (maybe collect them in
UpdateTilePriorities). Remove them from the list as they get rastered. When
list.empty() you can activate.


>
> Currently, it looks like this patch only notifies the main thread to check
> for
> activation once all rasterizations are done. Activation could occur once a
> subset (visible and high res) of the rasterizations are done though.
>
>
>
https://codereview.chromium.**org/13947038/<https://codereview.chromium.org/1...
>

[Tom Hudson, 2013-04-19 09:55:37]

On 2013/04/18 22:39:34, danakj wrote:
> On Thu, Apr 18, 2013 at 6:24 PM, <mailto:brianderson@chromium.org> wrote:
> > How expensive is the check to see if we can activate? Can we make the check
> > cheap enough so that we can check for it after each rasterization on the
> > worker
> > threads?
> >
> 
> Currently it's a tree walk - expensive.
> 
> To do better would be something like... keep a list of all the tiles that
> need to be activated by doing a single tree walk (maybe collect them in
> UpdateTilePriorities). Remove them from the list as they get rastered. When
> list.empty() you can activate.

Even cheaper, keep just a count? Or are there error cases for that?

[danakj, 2013-04-19 16:48:09]

On Fri, Apr 19, 2013 at 5:55 AM, <tomhudson@chromium.org> wrote:

> On 2013/04/18 22:39:34, danakj wrote:
>
>  On Thu, Apr 18, 2013 at 6:24 PM,
<mailto:brianderson@chromium.**org<brianderson@chromium.org>>
>> wrote:
>> > How expensive is the check to see if we can activate? Can we make the
>> check
>> > cheap enough so that we can check for it after each rasterization on the
>> > worker
>> > threads?
>> >
>>
>
>  Currently it's a tree walk - expensive.
>>
>
>  To do better would be something like... keep a list of all the tiles that
>> need to be activated by doing a single tree walk (maybe collect them in
>> UpdateTilePriorities). Remove them from the list as they get rastered.
>> When
>> list.empty() you can activate.
>>
>
> Even cheaper, keep just a count? Or are there error cases for that?
>

Ya I think that could probably work.


>
>
>
https://codereview.chromium.**org/13947038/<https://codereview.chromium.org/1...
>

[enne (OOO), 2013-04-19 17:09:22]

I think we're all on a similar page here.  To recap an offline discussion with
danakj from yesterday:

Right now activation is expensive because it's a tree walk and then tiling walks
underneath that.  We could pre-calculate the tiles required for activation
during UpdateTilePriorities, essentially by moving the areVisibleResourceReady
work into UpdateTilesPriorities.  I think we could do this by adding a "required
for activation" flag to TilePriority, and sorting priorities marked as such as
being higher than tiles not marked.  We could also use this flag to maintain a
count somewhere (in TileManager?) and cheaply ask about whether activation is
ok.  We'd have to be a little bit careful about counting tiles "not assigned GPU
memory" as complete, but that's a minor wrinkle.

I think an approach like this would be plausible and would more elegantly reduce
activation latency without magic wait times or worrying about "trying to
activate" too much.

[Sami, 2013-04-19 17:20:18]

On 2013/04/19 17:09:22, enne wrote:
> I think we're all on a similar page here.  To recap an offline discussion with
> danakj from yesterday:
> 
> Right now activation is expensive because it's a tree walk and then tiling
walks
> underneath that.  We could pre-calculate the tiles required for activation
> during UpdateTilePriorities, essentially by moving the areVisibleResourceReady
> work into UpdateTilesPriorities.  I think we could do this by adding a
"required
> for activation" flag to TilePriority, and sorting priorities marked as such as
> being higher than tiles not marked.  We could also use this flag to maintain a
> count somewhere (in TileManager?) and cheaply ask about whether activation is
> ok.  We'd have to be a little bit careful about counting tiles "not assigned
GPU
> memory" as complete, but that's a minor wrinkle.
> 
> I think an approach like this would be plausible and would more elegantly
reduce
> activation latency without magic wait times or worrying about "trying to
> activate" too much.

This sounds awesome. On top of that I think we'd also need the bit from this
patch that turns a successful activation into a vsync so that we can draw sooner
than the next real vsync.

sievers@ was going to try delaying swap acks to the renderer so it wouldn't
grind to a halt if it tried to draw too much. Avoiding that was the reason for
the magical 8 ms.

[nduca, 2013-04-22 17:28:35]

Oh please dont go delaying swap acks. Figure out why the scheduler is doing the
wrong thing and change the scheduler. The more I look at things, I think
rewriting the scheduler to just do a fixed set of steps on vsync tick would be a
nice change from our current code.

[nduca, 2013-04-22 17:32:23]

Actually, wait. Folks, if the scheduler was right aligned, then wouldn't this
fix things?

vsync is on left side of frame. bfac is sent here.

draw is as far to the right as possible, e.g. next vsync - 4ms [which by the way
will probably be 8ms].

when you draw, check for uploads, try to activate, draw.

[Sami, 2013-04-22 18:42:07]

On 2013/04/22 17:32:23, nduca wrote:
> Actually, wait. Folks, if the scheduler was right aligned, then wouldn't this
> fix things?
>
> vsync is on left side of frame. bfac is sent here.
> 
> draw is as far to the right as possible, e.g. next vsync - 4ms [which by the
way
> will probably be 8ms].
> 
> when you draw, check for uploads, try to activate, draw.

Yeah, in a way this is right-aligning the renderer, but not to an arbitrary
fixed offset w.r.t. vsync but to when we know for a fact that we can activate
and draw. This is similar to how the browser compositor is scheduled on Android
-- if there's a new frame pending from the renderer, draw immediately, otherwise
wait for vsync.

Daniel pointed out that since both rAF and tree activation are (indirectly)
triggered on vsync, we're effectively aligning one frame to two vsyncs. Doing
everything based on a single vsync seems better to me.

[nduca, 2013-04-22 22:54:10]

Well put! I think maybe a gvc with some whiteboardses would be good soon.

[Tom Hudson, 2013-04-23 08:43:27]

On 2013/04/22 22:54:10, nduca wrote:
> Well put! I think maybe a gvc with some whiteboardses would be good soon.

ABSOLUTELY.

[reveman, 2013-04-23 18:32:55]

The early activation idea is useful on its own and can be done separately from
any vsync changes, right?

[enne (OOO), 2013-04-24 14:36:21]

On 2013/04/23 18:32:55, David Reveman wrote:
> The early activation idea is useful on its own and can be done separately from
> any vsync changes, right?

I think the two needs overlap.  If we moved to a right-aligned scheduler, then
there's no need for early activation.  We would attempt to draw at the last
second that we could draw.  If you can't activate at that point, then it's not
like you would or oculd activate later before the next vsync and draw again. 
So, in my mind, the core issue is that we are trying to activate too early.

So, although I think an O(1) activation check is a good idea in theory, it's
also an unneeded complication if we only need to call it once.

[danakj, 2013-04-24 14:45:44]

On Wed, Apr 24, 2013 at 10:36 AM, <enne@chromium.org> wrote:

> On 2013/04/23 18:32:55, David Reveman wrote:
>
>> The early activation idea is useful on its own and can be done separately
>> from
>> any vsync changes, right?
>>
>
> I think the two needs overlap.  If we moved to a right-aligned scheduler,
> then
> there's no need for early activation.  We would attempt to draw at the last
> second that we could draw.  If you can't activate at that point, then it's
> not
> like you would or oculd activate later before the next vsync and draw
> again.
> So, in my mind, the core issue is that we are trying to activate too early.
>
> So, although I think an O(1) activation check is a good idea in theory,
> it's
> also an unneeded complication if we only need to call it once.
>

It also lets us push the draw further to the right if it is not slow, as
treewalks are (see the verge.com)?


>
>
https://codereview.chromium.**org/13947038/<https://codereview.chromium.org/1...
>

[Tom Hudson, 2013-04-29 10:40:48]

Did the evening meeting on the 25th conclude that we're going to set this aside
for a larger project?

[Sami, 2013-04-29 10:45:07]

Yes, I believe this is deprecated in favor of
https://docs.google.com/a/chromium.org/document/d/1LUFA8MDpJcDHE0_L2EHvrcwqOM...
:)