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

Issue 875255 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 27
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

20.2%-29319.5% regression in browser_tests at 583892:583919

Project Member Reported by niklase@chromium.org, Aug 17

Issue description

Tons of latency regressions
 
All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=875255

(For debugging:) Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?sid=681f9cb04e3b5083fa13f7700e4698220459f79d6f3bbc20bea4c13810bb373c


Bot(s) for this bug's original alert(s):

chromium-webrtc-rel-7
chromium-webrtc-rel-linux
chromium-webrtc-rel-win10
Cc: emir...@chromium.org
Owner: enne@chromium.org
enne@ can you PTAL? This CL seems to be related.
https://chromium.googlesource.com/chromium/src/+/430c8fbe06f29f492df7f212a9cc83d272d7634b

What we measure in this graph is time between these two calls. I assume GetCurrentFrame() are later with your CL. Can you explain why they are coming later.
"WebMediaPlayerMSCompositor::SetCurrentFrame": Driven by real-time frames received from network and decoded. On "Disable smoothness" graph, we set it asap. In the other graph, it goes through media::VideoRendererAlgorithm.
"WebMediaPlayerMSCompositor::GetCurrentFrame": Driven by compositor.
https://chromeperf.appspot.com/report?sid=0674731133488daa861de93649d718be44d716e3691080239f0d4b22d1ced2c7&rev=583911
https://chromeperf.appspot.com/report?sid=45981c2db8321e747879df400de12c76d012ff210703b12d727e585be2aaa132&rev=583911
Cc: mlamouri@chromium.org liber...@chromium.org samans@chromium.org sunn...@chromium.org lethalantidote@chromium.org
If we activate deadlines immediately, then any frames that come in after that and before the next frame have to wait for the frame after.  In this case, redraws driven by video layers, but I think this is also true for commits driven by the main thread outside of raf.

There are a number of other regressions here ( issue 875323 ,  issue 875328 ), so likely this isn't going to be able to stick and we're going to need to fix this in the DisplayScheduler.

Also, it looks like UseSurfaceLayerforVideoMS is not turned on for the bots, so this is running without surface layer for video.

I'm going to leave this on so that we can at least get some data out of the bots as to whether this fixes the 30fps video issue, but this is going to require a bunch more work if this is the approach we want to use to fix this.
Cc: rmccool@chromium.org
> Also, it looks like UseSurfaceLayerforVideoMS is not turned on for the bots, so this is running without surface layer for video.
I think this feature is still WIP.

rmccool@, FYI in case you observe regressions in Canary. 
Also for clarification, I'm going to leave this on <1 week and then will likely revert.  I just want to get some signal out of it.
Project Member

Comment 7 by bugdroid1@chromium.org, Aug 27

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

commit 2e5401607fcfe8d434f2f91ae37a76222d9df322
Author: enne <enne@chromium.org>
Date: Mon Aug 27 20:41:09 2018

Revert "cc: Finish frame early if main thread is idle regardless of redraw state"

This reverts commit 430c8fbe06f29f492df7f212a9cc83d272d7634b.

Reason for revert: various perf regressions.
This should instead be solved via issue 874680.

Original change's description:
> cc: Finish frame early if main thread is idle regardless of redraw state
>
> This condition was added originally in this patch:
> https://codereview.chromium.org/27200003
>
> That patch was trying to trigger early deadlines when the compositor
> thread had work to do but the main thread had aborted.
>
> In a world where we have begin frame acks even when frames aren't
> produced, it's worth triggering early deadlines even when the compositor
> thread doesn't have any work to do either so that the display compositor
> can go ahead and do work earlier.
>
> This is intended as a short-term fix to solve smoothness issues with
> surface for video with 30fps videos.  Ideally the DisplayCompositor will
> have some followups as well so that one client that is slow to respond
> will not cause smoothness issues with another client.
>
> Bug:  874676 
> Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
> Change-Id: I8b6663570aba7d135c1f211a21c37b1e17d19577
> Reviewed-on: https://chromium-review.googlesource.com/1176559
> Commit-Queue: enne <enne@chromium.org>
> Reviewed-by: Sunny Sachanandani <sunnyps@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#583908}

TBR=enne@chromium.org,sunnyps@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  874676 ,  875255 , 875316,  875323 ,  875328 ,  875424 ,  876099 
Change-Id: I4fd531fe07396ec2275bfd9109c8d86c6b4778f5
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Reviewed-on: https://chromium-review.googlesource.com/1191326
Commit-Queue: enne <enne@chromium.org>
Reviewed-by: enne <enne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#586402}
[modify] https://crrev.com/2e5401607fcfe8d434f2f91ae37a76222d9df322/cc/scheduler/scheduler_state_machine.cc

Status: Fixed (was: Assigned)

Sign in to add a comment