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

Issue 636644 link

Starred by 23 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug



Sign in to add a comment

SGI_video_sync causes browser window to stop updating

Project Member Reported by sunn...@chromium.org, Aug 11 2016

Issue description

With nvidia driver version 361.42 (goobuntu latest)

Repro:
1. Use a non-compositing window manager (e.g. i3)
2. Start chrome
3. Observe that the main window is stuck although it still receives input events and issues swap buffers calls

The above also happens intermittently in a long-running chrome window especially when switching to it (expose).

This does not happen with the gpu sandbox disabled or with SGI_video_sync disabled. It also does not happen with a compositing manager (I tried cinnamon, compton).
 
Summary: SGI_video_sync causes browser window to stop updating (was: SGI_video_sync causes causes browser window to stop updating)

Comment 2 by piman@chromium.org, Aug 11 2016

Cc: rickyz@chromium.org
Labels: -Pri-3 Pri-1
Status: Started (was: Assigned)
Cc: thomasanderson@chromium.org
 Issue 636013  has been merged into this issue.
This appears to be fixed on the latest nvidia driver.

I guess if anyone is looking for a quick fix, you can try "apt-get install nvidia-367".  Note that this will uninstall the goobuntu driver, and I accept no responsibility for damages :)
Issue 635391 has been merged into this issue.

Comment 8 by derat@chromium.org, Aug 23 2016

I filed http://b/31039175 to track this internally.
@sunnyps

Would it be possible to create a workaround that checks if 
1. the driver is 361.42
2. the user is running a non-compositing WM
and if those conditions are true, disable SGI_video_sync?


Project Member

Comment 10 by bugdroid1@chromium.org, Aug 25 2016

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

commit 22774dfbaceb5f616183b3d6135264b458c9ed0a
Author: brianderson <brianderson@chromium.org>
Date: Thu Aug 25 06:35:19 2016

gl: Disable SGI_video_sync by default.

Disables the SGI_video_sync extension and assumes a
vsync interval of 59.9Hz on Linux.

Also adds an --enable-sgi-video-sync command line flag to
re-enable the extension.

Use of the extension on a thread other than the main GPU
thread is encountering issues with Chrome's sandbox for
certain combinations of Linux window managers and NVIDIA
drivers.

Before this patch, we were assuming a frequency of 60Hz
on Linux and re-aligning the phase every 5 seconds.
Since we can no longer align the phase, we assume a
frequency of 59.9 Hz. On a 60Hz monitor, this avoids
driver backpressure on the GPU thread at the cost of
skipping a frame every 10 seconds.

BUG= 636644 

Review-Url: https://codereview.chromium.org/2277883003
Cr-Commit-Position: refs/heads/master@{#414342}

[modify] https://crrev.com/22774dfbaceb5f616183b3d6135264b458c9ed0a/ui/gfx/BUILD.gn
[add] https://crrev.com/22774dfbaceb5f616183b3d6135264b458c9ed0a/ui/gfx/vsync_provider.cc
[modify] https://crrev.com/22774dfbaceb5f616183b3d6135264b458c9ed0a/ui/gfx/vsync_provider.h
[modify] https://crrev.com/22774dfbaceb5f616183b3d6135264b458c9ed0a/ui/gl/gl_surface_glx.cc
[modify] https://crrev.com/22774dfbaceb5f616183b3d6135264b458c9ed0a/ui/gl/gl_switches.cc
[modify] https://crrev.com/22774dfbaceb5f616183b3d6135264b458c9ed0a/ui/gl/gl_switches.h

Labels: Merge-Request-53 Merge-Request-54
brianderson@
So I'm guessing the change in #10 fixed this?
If so, could we get this merged into M53 and M54?

Comment 12 by dimu@chromium.org, Sep 6 2016

Labels: -Merge-Request-53 Merge-Review-53 Hotlist-Merge-Review
[Automated comment] Request affecting a post-stable build (M53), manual review required.

Comment 13 by dimu@chromium.org, Sep 6 2016

Labels: -Merge-Request-54 Merge-Review-54
[Automated comment] Commit may have occurred before M54 branch point (8/25/2016), needs manual review.

Comment 14 by dimu@chromium.org, Sep 6 2016

Labels: -Merge-Request-53 Merge-Review-53 Hotlist-Merge-Review
[Automated comment] Request affecting a post-stable build (M53), manual review required.
Also, it's not urgent that this needs to be merged today, but can be done for next Tuesday's release.
Ok, per comment #15, it will miss this week Stable release and we can take it in for next stable refresh release if any in future (hopefully by then it will be baked in M54 beta as well)
Labels: -Merge-Review-54 Merge-Approved-54
This change meets the bar and is approved for merge into M54
Could you please confirm whether this change is baked/verified in Canary and safe to merge?If yes, merge your change to M54 (branch: 2840) so that we could take this for next Beta Release.

Project Member

Comment 19 by bugdroid1@chromium.org, Sep 9 2016

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

commit ffc0e467329470df9c2da085d133d59cc8ae2414
Author: sunnyps <sunnyps@chromium.org>
Date: Fri Sep 09 22:51:57 2016

Fix SGI_video_sync cpu usage and rendering issues with Nvidia driver.

Using the GLX_SGI_video_sync extension has caused high cpu usage and
rendering problems. The cpu usage was worked around by throttling the
use of the extension to once every 5 seconds. The rendering problems
started happening with Nvidia driver 361.42 and was being worked around
by not using the extension.

This CL fixes both problems by doing the following:
1. Using a separate unmapped child window (and GLX drawable) for use
   with SGI_video_sync.
2. Create a dummy unmapped window on both the main X Display and the
   video sync Display before the sandbox is set up.
3. Whitelist the Nvidia device files that are opened after the sandbox
   is set up.

This CL also makes all vsync providers use a single GLXContext because
there's only one thread that's used for all of them.

R=piman@chromium.org,rickyz@chromium.org
BUG= 636644 , 483721 
TEST=manual testing with i3 wm (no compositing)

Review-Url: https://codereview.chromium.org/2291373002
Cr-Commit-Position: refs/heads/master@{#417747}

[modify] https://crrev.com/ffc0e467329470df9c2da085d133d59cc8ae2414/content/common/sandbox_linux/bpf_gpu_policy_linux.cc
[modify] https://crrev.com/ffc0e467329470df9c2da085d133d59cc8ae2414/ui/gl/gl_surface_glx.cc

CL listed @ #19 is not landed/baked in Canary yet and both changes (#10 & #19) are not landed/baked in M54 beta yet. So it will miss next week Stable release and we can take it in for next stable refresh release if any in future. Please let me know if there is any concern here.
Project Member

Comment 21 by sheriffbot@chromium.org, Sep 11 2016

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
The CL in #19 seems to be in canary for 2+ days, please confirm whether this fixed and safe to merge?If yes, merge your change to M54 (branch: 2840)by eod Tuesday-09/13 before 4.00 pm [PST].
 Issue 646335  has been merged into this issue.
I'll merge the CL in #19 ASAP. There was confusion because two fixes were developed because it wasn't clear that a proper fix would land in time for M54. But that has happened so it makes sense to merge that instead of the temporary fix.
Project Member

Comment 25 by bugdroid1@chromium.org, Sep 13 2016

Labels: -merge-approved-54 merge-merged-2840
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8340748678f1806b1feb8fbdad3984c74d65d57a

commit 8340748678f1806b1feb8fbdad3984c74d65d57a
Author: Sunny Sachanandani <sunnyps@chromium.org>
Date: Tue Sep 13 18:20:52 2016

Fix SGI_video_sync cpu usage and rendering issues with Nvidia driver.

Using the GLX_SGI_video_sync extension has caused high cpu usage and
rendering problems. The cpu usage was worked around by throttling the
use of the extension to once every 5 seconds. The rendering problems
started happening with Nvidia driver 361.42 and was being worked around
by not using the extension.

This CL fixes both problems by doing the following:
1. Using a separate unmapped child window (and GLX drawable) for use
   with SGI_video_sync.
2. Create a dummy unmapped window on both the main X Display and the
   video sync Display before the sandbox is set up.
3. Whitelist the Nvidia device files that are opened after the sandbox
   is set up.

This CL also makes all vsync providers use a single GLXContext because
there's only one thread that's used for all of them.

R=piman@chromium.org,rickyz@chromium.org
BUG= 636644 , 483721 
TEST=manual testing with i3 wm (no compositing)

Review-Url: https://codereview.chromium.org/2291373002
Cr-Commit-Position: refs/heads/master@{#417747}
(cherry picked from commit ffc0e467329470df9c2da085d133d59cc8ae2414)

Review URL: https://codereview.chromium.org/2335903004 .

Cr-Commit-Position: refs/branch-heads/2840@{#334}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/8340748678f1806b1feb8fbdad3984c74d65d57a/content/common/sandbox_linux/bpf_gpu_policy_linux.cc
[modify] https://crrev.com/8340748678f1806b1feb8fbdad3984c74d65d57a/ui/gl/gl_surface_glx.cc

Status: Fixed (was: Started)
Status: Assigned (was: Fixed)
sunnyps@ Please merge into M53 as well
govind@ Is this OK to merge to M53? This change affects Linux only and the risk should be minimal.
M53 is already in stable so bar is VERY high. We can take this change only if it is important and baked/verified in Canary and safe to merge.
thomasanderson@ can you say how strongly you feel about merging this CL to M53? I personally don't think that the merge is crucial given that there are at least two workarounds: run a compositing manager (e.g. Unity, cinnamon, kwin, xcompmgr, compman, etc.) or run chrome with the --disable-gpu flag. Most linux users are probably running a compositing manager so the impact of this bug should be limited.
Just FYI: I already cut RC for stable release tomorrow.
Based on thomasanderson@'s reply for #30, We may take this change in for future M53 release if any (by then the change will be baked in M54 beta too).
I think we can get away with merging this by next week.

As for the workarounds, I don't think it would be obvious to users to switch to a compositing WM as a workaround, or that the --disable-gpu flag even exists.  The 22 users that have starred this issue are a small minority who will be keeping tabs on this thread.
How is this change looking in beta?
I had a quick look at the Linux beta top gpu process crashers:
https://crash.corp.google.com/browse?q=product.name%3D%27Chrome_Linux%27%20AND%20product.version%3D%2754.0.2840.27%27%20AND%20custom_data.ChromeCrashProto.ptype%3D%27gpu-process%27#-samplereports:5,magicsignature

Nothing there looks related to this CL. Based on my limited testing the bug appears to be fixed on beta.
I haven't seen any reports of this bug in beta.
However, there are still bugs coming in for stable eg. 647328, and based on that, I think we should merge.
Labels: -Hotlist-Merge-review -Merge-Review-53
Status: Fixed (was: Assigned)
We discussed this offline and have decided not to merge to stable (M53). If you're affected by this bug please use Chrome beta (M54) or try the other workarounds in #30. To install beta channel:

sudo apt-get install google-chrome-beta

It uses a separate profile directory so migrating between stable and beta should be easy.
Project Member

Comment 37 by bugdroid1@chromium.org, Oct 27 2016

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

commit 8340748678f1806b1feb8fbdad3984c74d65d57a
Author: Sunny Sachanandani <sunnyps@chromium.org>
Date: Tue Sep 13 18:20:52 2016

Fix SGI_video_sync cpu usage and rendering issues with Nvidia driver.

Using the GLX_SGI_video_sync extension has caused high cpu usage and
rendering problems. The cpu usage was worked around by throttling the
use of the extension to once every 5 seconds. The rendering problems
started happening with Nvidia driver 361.42 and was being worked around
by not using the extension.

This CL fixes both problems by doing the following:
1. Using a separate unmapped child window (and GLX drawable) for use
   with SGI_video_sync.
2. Create a dummy unmapped window on both the main X Display and the
   video sync Display before the sandbox is set up.
3. Whitelist the Nvidia device files that are opened after the sandbox
   is set up.

This CL also makes all vsync providers use a single GLXContext because
there's only one thread that's used for all of them.

R=piman@chromium.org,rickyz@chromium.org
BUG= 636644 , 483721 
TEST=manual testing with i3 wm (no compositing)

Review-Url: https://codereview.chromium.org/2291373002
Cr-Commit-Position: refs/heads/master@{#417747}
(cherry picked from commit ffc0e467329470df9c2da085d133d59cc8ae2414)

Review URL: https://codereview.chromium.org/2335903004 .

Cr-Commit-Position: refs/branch-heads/2840@{#334}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/8340748678f1806b1feb8fbdad3984c74d65d57a/content/common/sandbox_linux/bpf_gpu_policy_linux.cc
[modify] https://crrev.com/8340748678f1806b1feb8fbdad3984c74d65d57a/ui/gl/gl_surface_glx.cc

Sign in to add a comment