New issue
Advanced search Search tips

Issue 682032 link

Starred by 1 user
Status: Fixed
Owner:
chrishtr@chromium.org
Closed: Jan 2018
Cc:
vmp...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

[SPv2] UpdateAuxiliaryObjectProperties introduced a O(n^2) to the PrePaint tree walk

Project Member Reported by pdr@chromium.org, Jan 17 2017 
updateAuxiliaryObjectProperties calls enclosingLayer which walks back up the ancestor chain to the nearest layer. One way around this would be to cache enclosingLayer on the way down, but I'd like to avoid bloating PrePaintTreeWalkContext more than we have to.
Project Member

Comment 1 by sheriffbot@chromium.org, Jan 19 2018

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available. If you change it back, also remove the "Hotlist-Recharge-Cold" label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 2 by schenney@chromium.org, Jan 19 2018

Owner: wangxianzhu@chromium.org
Status: Assigned (was: Untriaged)
Is this still an issue?

Comment 3 by schenney@chromium.org, Jan 19 2018

Labels: -Hotlist-Recharge-Cold

Comment 4 by wangxianzhu@chromium.org, Jan 19 2018

Summary: [SPv2] UpdateAuxiliaryObjectProperties introduced a O(n^2) to the PrePaint tree walk (was: updateAuxiliaryObjectProperties introduced a O(n^2) to the PrePaint tree walk)
This bug is SPv2 only. The fix seems simple: just let EnclosingLayer() return the parent layer if the object has layer.

Comment 5 by chrishtr@chromium.org, Jan 19 2018 

It's actually just replacing it with:

ToLayoutBoxModelObject(current)->Layer()

since the code early-outs if there is no PaintLayer.

Comment 6 by chrishtr@chromium.org, Jan 19 2018

Owner: chrishtr@chromium.org
I have a quick fix in case you haven't already got one...
Project Member

Comment 7 by bugdroid1@chromium.org, Jan 19 2018 

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

commit 0d801437338e54b9f79640f53e6e9c695ec82b4c
Author: Chris Harrelson <chrishtr@chromium.org>
Date: Fri Jan 19 21:28:05 2018

[CI] Replace enclosingLayer with explicit lookup of layer.

This emphasizes the fact that the enclosingLayer call is actually cheap because
the object in question is guaranteed to have one, by the logic earlier in the
method.

Bug:682032

Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: Ifaba3ba951acf52380a52fe1111f20d5ad2d61cd
Reviewed-on: https://chromium-review.googlesource.com/876731
Reviewed-by: Stephen Chenney <schenney@chromium.org>
Commit-Queue: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#530615}
[modify] https://crrev.com/0d801437338e54b9f79640f53e6e9c695ec82b4c/third_party/WebKit/Source/core/paint/PrePaintTreeWalk.cpp

Comment 8 by chrishtr@chromium.org, Jan 19 2018

Status: Fixed (was: Assigned)
►

Sign in to add a comment