Slow activation - Id leak in PropertyTrees::always_use_active_tree_opacity_effect_ids |
|||||||
Issue descriptionVersion: r398134 OS: Android (others unconfirmed for now) Issue 617111 showed up cases of activation taking over 10ms on Android. Investigation shows that PropertyTrees::PushOpacityIfNeeded times are ballooning as PropertyTrees::always_use_active_tree_opacity_effect_ids size keeps growing. We add ids (https://cs.chromium.org/chromium/src/cc/trees/property_tree_builder.cc?q=AddEffectNodeIfNeeded&sq=package:chromium&dr=CSs&l=856) but do we ever remove them?
,
Jun 7 2016
This page does constant invalidation and shows the issue: http://vmiura.github.io/svg-anim/svg-anim.html?1,tiger
,
Jun 7 2016
,
Jun 7 2016
>Investigation shows that PropertyTrees::PushOpacityIfNeeded times are ballooning as PropertyTrees::always_use_active_tree_opacity_effect_ids size keeps growing. Thanks for the investigation. As you rightly pointed out, the bug is that we are not clearing it. Its size should be fixed and not be growing. sizeof(PropertyTrees::always_use_active_tree_opacity_effect_ids) = number of solid_color_scrollbar layers + number of overlay painted_scrollbar layers We should be clearing it in PropertyTreeBuilder:: BuildPropertyTreesTopLevelInternal. Will land a fix soon.
,
Jun 7 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8013559b9c20153c8ed18f24179841025c6587f6 commit 8013559b9c20153c8ed18f24179841025c6587f6 Author: jaydasika <jaydasika@chromium.org> Date: Tue Jun 07 18:18:37 2016 cc: Clear always_use_active_tree_opacity_effect_ids When we rebuild property trees. BUG= 617826 CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel Review-Url: https://codereview.chromium.org/2044803002 Cr-Commit-Position: refs/heads/master@{#398344} [modify] https://crrev.com/8013559b9c20153c8ed18f24179841025c6587f6/cc/layers/scrollbar_layer_unittest.cc [modify] https://crrev.com/8013559b9c20153c8ed18f24179841025c6587f6/cc/trees/property_tree_builder.cc
,
Jun 7 2016
I believe the original issue landed just before M52 branch. Let's request merge to M52 after 1 day on Canary.
,
Jun 7 2016
Issue 613433 has been merged into this issue.
,
Jun 9 2016
,
Jun 9 2016
Your change meets the bar and is auto-approved for M52 (branch: 2743)
,
Jun 9 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/77e55266aa9dcfb528d201fcb602ebbd01ab32ab commit 77e55266aa9dcfb528d201fcb602ebbd01ab32ab Author: Jayadev Dasika <jaydasika@google.com> Date: Thu Jun 09 19:18:49 2016 cc: Clear always_use_active_tree_opacity_effect_ids When we rebuild property trees. BUG= 617826 CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel Review-Url: https://codereview.chromium.org/2044803002 Cr-Commit-Position: refs/heads/master@{#398344} (cherry picked from commit 8013559b9c20153c8ed18f24179841025c6587f6) Review URL: https://codereview.chromium.org/2050623006 . Cr-Commit-Position: refs/branch-heads/2743@{#296} Cr-Branched-From: 2b3ae3b8090361f8af5a611712fc1a5ab2de53cb-refs/heads/master@{#394939} [modify] https://crrev.com/77e55266aa9dcfb528d201fcb602ebbd01ab32ab/cc/trees/property_tree_builder.cc
,
Jun 9 2016
The change that I landed on m52 doesn't have the unit test change ( cc/layers/scrollbar_layer_unittest.cc). I removed it as it had merge conflicts.
,
Jun 9 2016
,
Jun 15 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/77e55266aa9dcfb528d201fcb602ebbd01ab32ab commit 77e55266aa9dcfb528d201fcb602ebbd01ab32ab Author: Jayadev Dasika <jaydasika@google.com> Date: Thu Jun 09 19:18:49 2016 cc: Clear always_use_active_tree_opacity_effect_ids When we rebuild property trees. BUG= 617826 CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel Review-Url: https://codereview.chromium.org/2044803002 Cr-Commit-Position: refs/heads/master@{#398344} (cherry picked from commit 8013559b9c20153c8ed18f24179841025c6587f6) Review URL: https://codereview.chromium.org/2050623006 . Cr-Commit-Position: refs/branch-heads/2743@{#296} Cr-Branched-From: 2b3ae3b8090361f8af5a611712fc1a5ab2de53cb-refs/heads/master@{#394939} [modify] https://crrev.com/77e55266aa9dcfb528d201fcb602ebbd01ab32ab/cc/trees/property_tree_builder.cc |
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by vmi...@chromium.org
, Jun 7 2016