Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2470333002:
Move PendingTree's UpdateDrawProperties to the back of SubmitCompositorFrame. (Closed)
|
Created:
4 years, 1 month ago by Yupei Wang Modified:
4 years, 1 month ago CC:
boliu, cc-bugs_chromium.org, chromium-reviews Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionMove PendingTree's UpdateDrawProperties to the back of SubmitCompositorFrame.
PendingTree's UpdateDrawProperties is unnecessary for submitting current compositorFrame.
In order to submit current compositorFrame as soon as possible，move PendingTree's UpdateDrawProperties to the back of it.
BUG=662311
R=enne@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
Patch Set 1 #
Messages
Total messages: 20 (10 generated)
Description was changed from ========== Move PendingTree's UpdateDrawProperties to the back of SubmitCompositorFrame. PendingTree's UpdateDrawProperties is unnecessary for submitting current compositorFrame. In order to submit current compositorFrame as soon as possible，move PendingTree's UpdateDrawProperties to the back of it. BUG=nobug R=enne@chromium.org ========== to ========== Move PendingTree's UpdateDrawProperties to the back of SubmitCompositorFrame. PendingTree's UpdateDrawProperties is unnecessary for submitting current compositorFrame. In order to submit current compositorFrame as soon as possible，move PendingTree's UpdateDrawProperties to the back of it. BUG=nobug R=enne@chromium.org CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel ==========
Description was changed from ========== Move PendingTree's UpdateDrawProperties to the back of SubmitCompositorFrame. PendingTree's UpdateDrawProperties is unnecessary for submitting current compositorFrame. In order to submit current compositorFrame as soon as possible，move PendingTree's UpdateDrawProperties to the back of it. BUG=nobug R=enne@chromium.org CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel ========== to ========== Move PendingTree's UpdateDrawProperties to the back of SubmitCompositorFrame. PendingTree's UpdateDrawProperties is unnecessary for submitting current compositorFrame. In order to submit current compositorFrame as soon as possible，move PendingTree's UpdateDrawProperties to the back of it. BUG=nobug R=enne@chromium.org CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel ==========
perryuwang@tencent.com changed reviewers: + aelias@chromium.org
I don't think we should land behavioral changes that don't solve any bug. Please file a bug describing what problem you're aiming to fix (e.g. a trace with a performance problem observed), and link it here in BUG=.
On 2016/11/04 01:22:40, aelias wrote: > I don't think we should land behavioral changes that don't solve any bug. > Please file a bug describing what problem you're aiming to fix (e.g. a trace > with a performance problem observed), and link it here in BUG=. When a page has a lot of layers，pendingTree's UpdateDrawProperties is a time-consuming task. This's more obvious for low-end devices. And it's unnecessary for submitting current compositorFrame.
Please file a bug, attach an example HTML page (or link URL) demonstrating the problem, and attach a video and a chrome://tracing trace.
The CQ bit was checked by aelias@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by perryuwang@tencent.com
The CQ bit was checked by perryuwang@tencent.com
The CQ bit was unchecked by perryuwang@tencent.com
On 2016/11/04 03:12:09, aelias wrote: > Please file a bug, attach an example HTML page (or link URL) demonstrating the > problem, and attach a video and a chrome://tracing trace. Ok. This is a bug: https://bugs.chromium.org/p/chromium/issues/detail?id=662311. I attach a trace.
Description was changed from ========== Move PendingTree's UpdateDrawProperties to the back of SubmitCompositorFrame. PendingTree's UpdateDrawProperties is unnecessary for submitting current compositorFrame. In order to submit current compositorFrame as soon as possible，move PendingTree's UpdateDrawProperties to the back of it. BUG=nobug R=enne@chromium.org CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel ========== to ========== Move PendingTree's UpdateDrawProperties to the back of SubmitCompositorFrame. PendingTree's UpdateDrawProperties is unnecessary for submitting current compositorFrame. In order to submit current compositorFrame as soon as possible，move PendingTree's UpdateDrawProperties to the back of it. BUG=662311 R=enne@chromium.org CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel ==========
OK, according to boliu@ on the bug, we shouldn't take any action here, so closing this codereview.
Message was sent while issue was closed.
boliu@chromium.org changed reviewers: + boliu@chromium.org
Message was sent while issue was closed.
oh I didn't see this from the trace since not every draw had a pending tree, but knowing where it is, I did find it. so in the trace, this takes ~2ms. guess either it's a very complex page or a really slow device, anyway So yes, if there's only one webview drawing per frame, then moving this to after SubmitFrame would move it out of the critical path of the frame, for both webview and chrome. Won't help as much if there are multiple webviews though. I have no idea if it's a safe change though. So ehh... feel free to keep discussing correctness if you want?
Message was sent while issue was closed.
OK. Judging by the name of the trace file, the repro page is http://sina.cn/. Maybe there is some kind of problem here (although perhaps not -- I'm still not sure if this actually is *mandatory* to have in the critical path for some reason). But it still doesn't seem worth the risk to take this dangerous-looking ordering change for the sake of improving a general web browsing case (relative nongoal for WebView, I'm more interested in native app performance cases) by 2ms on certain frames on low-end devices. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
