New issue
Advanced search Search tips

Issue 850746 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocked on: View detail
issue 858875
issue 884876



Sign in to add a comment

Youtube ads triggers 180 scheduler::BeginFrame per second even invisible

Project Member Reported by zmo@chromium.org, Jun 7 2018

Issue description

This is ToT. I recorded the tracing on Windows desktop.

Go to any youtube video, go to fullscreen.

As can seen from the trace, renderer (id: 13960) with googlesyndication.com triggered Scheduler::BeginFrame() at 180 fps? (or three independent items each triggering at 60 fps)

This should be invisible, so in theory compositor should just be idle. So we are wasting CPU time here.

 
trace_youtube_fullscreen_no_video_surface_desktop.json.gz
6.0 MB Download

Comment 1 by zmo@chromium.org, Jun 7 2018

I think it doesn't matter which video, but just in case, the specific video I used to record the tracing is https://www.youtube.com/watch?v=9A0h9XB_0ik
Cc: sadrul@chromium.org fsam...@chromium.org creis@chromium.org
Although there are 180 BeginFrames per second, it looks like this is because 3 instances are sharing the same Renderer process. 3 sources ticking at 60fps without submitting frames still isn't good though.

We run AnimationHost::TickAnimations every BeginFrame but it doesn't look like that ever causes damage, which is strange:
 * Maybe there's a bug in our animation logic?
 * Maybe the ad logic (I'm assuming they are ads) have a buggy animation?

This might have a similar root cause as  issue 834421 , where OOPIF's submit undisplayed frames causing LatencyInfos to pile up. +Charlie,  Sadrul, Fady in case this is a known issue.
 * If the 3 OOPIF's are offscreen or not visible, maybe we need to detect that tell them they aren't visible.

If this isn't a known issue, I'll spend some time narrowing down the root cause tomorrow.






Comment 3 by creis@chromium.org, Jun 8 2018

Cc: kenrb@chromium.org
Components: Internals>Sandbox>SiteIsolation
I'm not aware of a known issue for this, but CC'ing kenrb@ in case this sounds familiar to him.  Narrowing it down further sounds good to me-- thanks!
Owner: zmo@chromium.org
Back over to Mo. Sorry I didn't actually get to narrowing it down.
Blockedon: 858875
[GPU Triage Council]

Is this still an issue?
Cc: rjkroege@chromium.org
No, it's definitely not.

Robert, now viz has launched, who might be the right person to look into this?
kylechar@ would be the logical owner. But: you said "it's definitely not" in response to "is this still an issue". Maybe you meant: "it's still an issue"?
Cc: zmo@chromium.org
Owner: kylec...@chromium.org
Right, what I mean is it's still an issue (and pretty bad perf-wise)

kylechar@: can you take a shot at fixing this?
I can confirm the OOPIF renderer is still getting begin frames and triggering 
Schduler::BeginFrame even when a full screen video is playing. We probably want a similar solution to https://crbug.com/884876 where we throttle begin frames to offscreen clients?
Kyle: thanks for taking a look. I see https://crbug.com/884876 is a P3, so I am not sure if that will gets fixed soon. The reason I set this to P1 is we are optimizing video stack on Windows right now and we are hoping to get to a point where renderer is not activated at all for most frames (through batch decoding and batch frame presenting). Having this bug basically defeat that purpose. Your input to get this fixed will be really appreciated.
Cc: -fsam...@chromium.org
Let me talk with sadrul and see what the status of crbug.com/884876 is.
Cc: kylec...@chromium.org
Owner: sgilhuly@chromium.org
I talked to sadrul@ and tried the current version. It doesn't hit the throttling logic for static display adds in an OOPIF because the last submitted CompositorFrame was drawn.

sgilhuly@ can you take a look? The latest CL from sadrul is in https://crrev.com/c/1320199. If we can get that to throttle begin frames for Youtube ads then we can look at solving the test failures that block landing it.
Blockedon: 884876
Cc: sunn...@chromium.org
Okay so it sounds like https://crrev.com/c/1320199 is fixed and should be landable again soon. It throttles BeginFrames to offscreen OOPIFs that are submitting CompositorFrames by throttling if the last CompositorFrame the client submitted hasn't been drawn (eg. used during surface aggregation to build the display frame).

For this bug we want to also throttle BeginFrames to offscreen OOPIFs that aren't submitting CompositorFrames. The logic in https://crrev.com/c/1320199 misses this case because it's a static display ad and if the ad gets drawn once then Surface::HasUndrawnActiveFrame() will always be false. It should be possible to extend the logic so that instead of just checking for an undrawn active frame in CompositorFrameSinkSupport::ShouldSendBeginFrame(), we also add logic so that when an OOPIF is no longer embedded (eg. no longer used during surface aggregation when it was previously used) we also throttle BeginFrames.

sadrul/piman/sunnyps: Any concerns here?
If the client is not submitting CFs, why is it still requesting BF? The cc logic should stop doing that (regardless of BF throttling in Display), shouldn't it?
That seems like it should be the case and potentially is. I took another trace on Linux with a video playing on youtube. I switch to the video tab and then go fullscreen in the trace.

There is an OOPIF renderer for googlesyndication.com and there is one ad showing above the "Up Next" label before I go fullscreen. I thought that OOPIF was submitting the ad content but it's not. A FrameSinkId is registered for the OOPIF renderer but there is no surface created. The ad shows up, so it's being put there by the main renderer and not the OOPIF renderer.

In the trace it looks like the googlesyndication.com SchedulerStateMachine always has needs_redraw=true and visible=true. It never actually draws anything but it gets a begin frame each frame?
trace_youtube_fullscreen_linux.json.gz
7.1 MB Download
I think if the page has a rAF callback, but the callback doesn't actually change any visuals, then we would have this scenario of the page always requesting begin-frames, but not submitting CFs.

Sign in to add a comment