New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 604280 link

Starred by 3 users
Status: Fixed
Owner:
loyso@chromium.org
Closed: Sep 2016
Cc:
█ vollick@chromium.org
ajuma@chromium.org
chrome-perf-bugs@google.com
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Feature



Sign in to add a comment

Introduce NeedsCommit dirty flag for AnimationHost

Project Member Reported by loyso@chromium.org, Apr 18 2016 
Introduce NeedsCommit dirty flag scoped for AnimationHost.
If no commit/PushProperties needed, just return.

In that case, OnAnimationWaitingForDeletion notification in element animations should invalidate that flag among many other invalidations.

Note that in old animation system we had no Layer::SetNeedsCommit call:

void Layer::OnAnimationWaitingForDeletion() {
  // Animations are only deleted during PushProperties.
  SetNeedsPushProperties();
}
void LayerImpl::OnAnimationWaitingForDeletion() {}

Comment 1 by loyso@chromium.org, Aug 22 2016

Status: Started (was: Assigned)
Project Member

Comment 2 by bugdroid1@chromium.org, Aug 29 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1ec9dd4962683e1fe12734a6756e825df2db10ca

commit 1ec9dd4962683e1fe12734a6756e825df2db10ca
Author: loyso <loyso@chromium.org>
Date: Mon Aug 29 07:09:16 2016

CC Animation: Introduce some dirty flags to optimize PushProperties on commit

The idea is that we invalidate needs_push_properties flag
from bottom-to-top in the hierarchy of ownership.

It helps us to isolate untouched groups of objects.

BUG= 604280 
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

Review-Url: https://codereview.chromium.org/2261113002
Cr-Commit-Position: refs/heads/master@{#414981}

[modify] https://crrev.com/1ec9dd4962683e1fe12734a6756e825df2db10ca/cc/animation/animation_host.cc
[modify] https://crrev.com/1ec9dd4962683e1fe12734a6756e825df2db10ca/cc/animation/animation_host.h
[modify] https://crrev.com/1ec9dd4962683e1fe12734a6756e825df2db10ca/cc/animation/animation_host_perftest.cc
[modify] https://crrev.com/1ec9dd4962683e1fe12734a6756e825df2db10ca/cc/animation/animation_player.cc
[modify] https://crrev.com/1ec9dd4962683e1fe12734a6756e825df2db10ca/cc/animation/animation_player.h
[modify] https://crrev.com/1ec9dd4962683e1fe12734a6756e825df2db10ca/cc/animation/animation_player_unittest.cc
[modify] https://crrev.com/1ec9dd4962683e1fe12734a6756e825df2db10ca/cc/animation/animation_timeline.cc
[modify] https://crrev.com/1ec9dd4962683e1fe12734a6756e825df2db10ca/cc/animation/animation_timeline.h
[modify] https://crrev.com/1ec9dd4962683e1fe12734a6756e825df2db10ca/cc/animation/element_animations.cc
[modify] https://crrev.com/1ec9dd4962683e1fe12734a6756e825df2db10ca/cc/animation/element_animations.h
[modify] https://crrev.com/1ec9dd4962683e1fe12734a6756e825df2db10ca/cc/animation/element_animations_unittest.cc
[modify] https://crrev.com/1ec9dd4962683e1fe12734a6756e825df2db10ca/cc/animation/scroll_offset_animations.cc
[modify] https://crrev.com/1ec9dd4962683e1fe12734a6756e825df2db10ca/cc/test/animation_timelines_test_common.cc
[modify] https://crrev.com/1ec9dd4962683e1fe12734a6756e825df2db10ca/cc/test/animation_timelines_test_common.h
Project Member

Comment 3 by bugdroid1@chromium.org, Aug 29 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/71b649adfb413b078a5fa6519627af90a203640e

commit 71b649adfb413b078a5fa6519627af90a203640e
Author: Istiaque Ahmed <lazyboy@chromium.org>
Date: Mon Aug 29 23:48:51 2016

Revert "CC Animation: Introduce some dirty flags to optimize PushProperties on commit"

This reverts commit 1ec9dd4962683e1fe12734a6756e825df2db10ca.

Reason for revert:
Since build
https://build.chromium.org/p/chromium.linux/builders/Linux%20Tests/builds/45513,
PluginPowerSaver.PosterTests and
PluginPowerSaver.SmallerThanPlayIcon
started flaking, timing out in VerifySnapshot: https://cs.chromium.org/chromium/src/chrome/browser/plugins/plugin_power_saver_browsertest.cc?sq=package:chromium&dr=CSs&rcl=1472469725&l=359

Speculatively reverting to see if that helps...

Original issue's description:
> CC Animation: Introduce some dirty flags to optimize PushProperties on commit
>
> The idea is that we invalidate needs_push_properties flag
> from bottom-to-top in the hierarchy of ownership.
>
> It helps us to isolate untouched groups of objects.
>
> BUG= 604280 
> CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
>
> Committed: https://crrev.com/1ec9dd4962683e1fe12734a6756e825df2db10ca
> Cr-Commit-Position: refs/heads/master@{#414981}

TBR=ajuma@chromium.org,danakj@chromium.org,loyso@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 604280 
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

Review URL: https://codereview.chromium.org/2291463003 .

Cr-Commit-Position: refs/heads/master@{#415040}

[modify] https://crrev.com/71b649adfb413b078a5fa6519627af90a203640e/cc/animation/animation_host.cc
[modify] https://crrev.com/71b649adfb413b078a5fa6519627af90a203640e/cc/animation/animation_host.h
[modify] https://crrev.com/71b649adfb413b078a5fa6519627af90a203640e/cc/animation/animation_host_perftest.cc
[modify] https://crrev.com/71b649adfb413b078a5fa6519627af90a203640e/cc/animation/animation_player.cc
[modify] https://crrev.com/71b649adfb413b078a5fa6519627af90a203640e/cc/animation/animation_player.h
[modify] https://crrev.com/71b649adfb413b078a5fa6519627af90a203640e/cc/animation/animation_player_unittest.cc
[modify] https://crrev.com/71b649adfb413b078a5fa6519627af90a203640e/cc/animation/animation_timeline.cc
[modify] https://crrev.com/71b649adfb413b078a5fa6519627af90a203640e/cc/animation/animation_timeline.h
[modify] https://crrev.com/71b649adfb413b078a5fa6519627af90a203640e/cc/animation/element_animations.cc
[modify] https://crrev.com/71b649adfb413b078a5fa6519627af90a203640e/cc/animation/element_animations.h
[modify] https://crrev.com/71b649adfb413b078a5fa6519627af90a203640e/cc/animation/element_animations_unittest.cc
[modify] https://crrev.com/71b649adfb413b078a5fa6519627af90a203640e/cc/animation/scroll_offset_animations.cc
[modify] https://crrev.com/71b649adfb413b078a5fa6519627af90a203640e/cc/test/animation_timelines_test_common.cc
[modify] https://crrev.com/71b649adfb413b078a5fa6519627af90a203640e/cc/test/animation_timelines_test_common.h
Project Member

Comment 4 by bugdroid1@chromium.org, Sep 8 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0bc588b1289ff9bda42a91ed8f57eaf0e36d7978

commit 0bc588b1289ff9bda42a91ed8f57eaf0e36d7978
Author: loyso <loyso@chromium.org>
Date: Thu Sep 08 00:33:45 2016

CC Animation: Introduce some dirty flags to optimize PushProperties on commit

The idea is that we invalidate needs_push_properties flag
from bottom-to-top in the hierarchy of ownership.

It helps us to isolate untouched groups of objects.

BUG= 604280 
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

Committed: https://crrev.com/1ec9dd4962683e1fe12734a6756e825df2db10ca
Review-Url: https://codereview.chromium.org/2261113002
Cr-Original-Commit-Position: refs/heads/master@{#414981}
Cr-Commit-Position: refs/heads/master@{#417132}

[modify] https://crrev.com/0bc588b1289ff9bda42a91ed8f57eaf0e36d7978/cc/animation/animation_host.cc
[modify] https://crrev.com/0bc588b1289ff9bda42a91ed8f57eaf0e36d7978/cc/animation/animation_host.h
[modify] https://crrev.com/0bc588b1289ff9bda42a91ed8f57eaf0e36d7978/cc/animation/animation_host_perftest.cc
[modify] https://crrev.com/0bc588b1289ff9bda42a91ed8f57eaf0e36d7978/cc/animation/animation_player.cc
[modify] https://crrev.com/0bc588b1289ff9bda42a91ed8f57eaf0e36d7978/cc/animation/animation_player.h
[modify] https://crrev.com/0bc588b1289ff9bda42a91ed8f57eaf0e36d7978/cc/animation/animation_player_unittest.cc
[modify] https://crrev.com/0bc588b1289ff9bda42a91ed8f57eaf0e36d7978/cc/animation/animation_timeline.cc
[modify] https://crrev.com/0bc588b1289ff9bda42a91ed8f57eaf0e36d7978/cc/animation/animation_timeline.h
[modify] https://crrev.com/0bc588b1289ff9bda42a91ed8f57eaf0e36d7978/cc/animation/element_animations.cc
[modify] https://crrev.com/0bc588b1289ff9bda42a91ed8f57eaf0e36d7978/cc/animation/element_animations.h
[modify] https://crrev.com/0bc588b1289ff9bda42a91ed8f57eaf0e36d7978/cc/animation/element_animations_unittest.cc
[modify] https://crrev.com/0bc588b1289ff9bda42a91ed8f57eaf0e36d7978/cc/animation/scroll_offset_animations.cc
[modify] https://crrev.com/0bc588b1289ff9bda42a91ed8f57eaf0e36d7978/cc/test/animation_timelines_test_common.cc
[modify] https://crrev.com/0bc588b1289ff9bda42a91ed8f57eaf0e36d7978/cc/test/animation_timelines_test_common.h

Comment 5 by loyso@chromium.org, Sep 8 2016

Status: Fixed (was: Started)
►

Sign in to add a comment