Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(600)

Issue 429223002: Reduced iterations for group-id check in MarkAnimationsForDeletions (Closed)

Created:
6 years, 4 months ago by shreyas.g
Modified:
6 years, 4 months ago
Reviewers:
vivekg, ajuma, Sikugu_, MuVen
CC:
chromium-reviews, cc-bugs_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Project:
chromium
Visibility:
Public.

Description

cc: Optimize group id check in LayerAnimationController::MarkAnimationsForDeletion MarkAnimationsForDeletion first checks if all the animations with the same group ID as the current animation is finished or not. If no, nothing is done. If yes, then in another iteration it again finds all the animations after the current animation in the list with the same group ID and marks it for WaitingForDeletion. This patch executes the MarkAnimationsForDeletion for animations with minimum iterations. While checking if all the animations with same ID is finished also mark the animations which satisfy this condition. Later use these marked animations to reduce the iterations. BUG=396562 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=287103

Patch Set 1 : Reduced iterations for group-id check in MarkAnimationsForDeleteion #

Total comments: 4

Patch Set 2 : Added the clear vector instead of maintaining separate logic #

Total comments: 4

Patch Set 3 : Added small nit changes #

Unified diffs Side-by-side diffs Delta from patch set Stats (+33 lines, -18 lines) Patch
M cc/animation/layer_animation_controller.cc View 1 2 3 chunks +33 lines, -18 lines 0 comments Download

Messages

Total messages: 16 (0 generated)
shreyas.g
PTAL.
6 years, 4 months ago (2014-07-30 13:17:42 UTC) #1
shreyas.g
++Reviewers Vivek, Siva, Muven
6 years, 4 months ago (2014-07-30 13:19:20 UTC) #2
shreyas.g
Also added data with examples and trace information to the bug 396562. PTAL.
6 years, 4 months ago (2014-07-30 14:24:25 UTC) #3
ajuma
https://codereview.chromium.org/429223002/diff/20001/cc/animation/layer_animation_controller.cc File cc/animation/layer_animation_controller.cc (right): https://codereview.chromium.org/429223002/diff/20001/cc/animation/layer_animation_controller.cc#newcode762 cc/animation/layer_animation_controller.cc:762: animation_i_will_send_or_has_received_finish_event) { What if you just create the animations_with_same_group_id ...
6 years, 4 months ago (2014-07-30 15:22:47 UTC) #4
shreyas.g
https://codereview.chromium.org/429223002/diff/20001/cc/animation/layer_animation_controller.cc File cc/animation/layer_animation_controller.cc (right): https://codereview.chromium.org/429223002/diff/20001/cc/animation/layer_animation_controller.cc#newcode762 cc/animation/layer_animation_controller.cc:762: animation_i_will_send_or_has_received_finish_event) { On 2014/07/30 15:22:47, ajuma wrote: > What ...
6 years, 4 months ago (2014-07-30 15:53:00 UTC) #5
ajuma
https://codereview.chromium.org/429223002/diff/20001/cc/animation/layer_animation_controller.cc File cc/animation/layer_animation_controller.cc (right): https://codereview.chromium.org/429223002/diff/20001/cc/animation/layer_animation_controller.cc#newcode762 cc/animation/layer_animation_controller.cc:762: animation_i_will_send_or_has_received_finish_event) { On 2014/07/30 15:52:59, shreyas.g wrote: > On ...
6 years, 4 months ago (2014-07-30 16:00:00 UTC) #6
shreyas.g
https://codereview.chromium.org/429223002/diff/20001/cc/animation/layer_animation_controller.cc File cc/animation/layer_animation_controller.cc (right): https://codereview.chromium.org/429223002/diff/20001/cc/animation/layer_animation_controller.cc#newcode762 cc/animation/layer_animation_controller.cc:762: animation_i_will_send_or_has_received_finish_event) { On 2014/07/30 16:00:00, ajuma wrote: > On ...
6 years, 4 months ago (2014-08-01 08:44:05 UTC) #7
ajuma
On 2014/08/01 08:44:05, shreyas.g wrote: > https://codereview.chromium.org/429223002/diff/20001/cc/animation/layer_animation_controller.cc > File cc/animation/layer_animation_controller.cc (right): > > https://codereview.chromium.org/429223002/diff/20001/cc/animation/layer_animation_controller.cc#newcode762 > ...
6 years, 4 months ago (2014-08-01 14:27:04 UTC) #8
shreyas.g
On 2014/08/01 14:27:04, ajuma wrote: > On 2014/08/01 08:44:05, shreyas.g wrote: > > > https://codereview.chromium.org/429223002/diff/20001/cc/animation/layer_animation_controller.cc ...
6 years, 4 months ago (2014-08-01 14:39:05 UTC) #9
ajuma
On 2014/08/01 14:39:05, shreyas.g wrote: > Thanks Ajuma. Thats a very good point you make ...
6 years, 4 months ago (2014-08-01 14:42:02 UTC) #10
shreyas.g
On 2014/08/01 14:39:05, shreyas.g wrote: > On 2014/08/01 14:27:04, ajuma wrote: > > On 2014/08/01 ...
6 years, 4 months ago (2014-08-01 15:13:09 UTC) #11
ajuma
lgtm with a couple nits: https://codereview.chromium.org/429223002/diff/40001/cc/animation/layer_animation_controller.cc File cc/animation/layer_animation_controller.cc (right): https://codereview.chromium.org/429223002/diff/40001/cc/animation/layer_animation_controller.cc#newcode781 cc/animation/layer_animation_controller.cc:781: // animations below will ...
6 years, 4 months ago (2014-08-01 15:26:48 UTC) #12
shreyas.g
https://codereview.chromium.org/429223002/diff/40001/cc/animation/layer_animation_controller.cc File cc/animation/layer_animation_controller.cc (right): https://codereview.chromium.org/429223002/diff/40001/cc/animation/layer_animation_controller.cc#newcode781 cc/animation/layer_animation_controller.cc:781: // animations below will be set to waitingForDeletion in ...
6 years, 4 months ago (2014-08-01 15:43:39 UTC) #13
shreyas.g
The CQ bit was checked by shreyas.g@samsung.com
6 years, 4 months ago (2014-08-01 15:43:46 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/shreyas.g@samsung.com/429223002/60001
6 years, 4 months ago (2014-08-01 15:45:03 UTC) #15
commit-bot: I haz the power
6 years, 4 months ago (2014-08-01 22:15:27 UTC) #16
Message was sent while issue was closed.
Change committed as 287103

Powered by Google App Engine
This is Rietveld 408576698