I added more traces, the specific problem is that fence->ClientWaitWithTimeoutNanos() (a wrapper for eglClientWaitSyncKHR) spends way more time than expected:

avg	17.804 ms
count	163
max	47.156 ms
min	0.391 ms
std	13.775 ms

It's supposed to wait for 2 ms, and average is 17.8ms. 

Deleted: trace_s8-waitwithtimeout.html 3.7 MB

trace_s8-waitwithtimeout.html 3.7 MB View Download

Machine model name: SM-G955U

GL_RENDERER: Adreno (TM) 540

Version information from logcat:

D libEGL  : loaded /vendor/lib/egl/libGLESv2_adreno.so

I Adreno  : QUALCOMM build                   : dd296bd, I7547f23799
I Adreno  : Build Date                       : 03/29/17
I Adreno  : OpenGL ES Shader Compiler Version: XE031.12.00.02
I Adreno  : Local Branch                     : 
I Adreno  : Remote Branch                    : refs/tags/AU_LINUX_ANDROID_LA.UM.5.7.C1.07.00.00.278.066
I Adreno  : Remote Branch                    : NONE
I Adreno  : Reconstruct Branch               : NOTHING

I Adreno  : PFP: 0x005ff104, ME: 0x005ff063
I OpenGLRenderer: Initialized EGL, version 1.4

Also, in the trace attached to comment #1, at 1900ms, it does two FenceCheckTimeout in a row for the same frame, taking 24 and 29ms back to back,  54.5ms total. So it's not just an issue of the fence check being slow, actual rendering seems very slow also.

Klaus, do you see this as a P2?  If so, it should probably target M-63.  If M-62, the priority should be increased.

See also internal b/65212787 , Qualcomm suggests getting in touch with Samsung. Anyone know a contact?

Bumping to M-63 since it seems unlikely to have a quick solution, we can revisit that if it does turn out to have an easy workaround.

Klaus may have a workaround.

Specifically, as of Canary 65.0.3312.0, turning on "WebVR experimental rendering" in chrome://flags should improve performance. This is currently off by default, we should consider a whitelist to auto-activate the workaround for the S8 on Android N.

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

commit 5859322791875be6f682139f736a090b58339c55
Author: Klaus Weidner <klausw@chromium.org>
Date: Wed Feb 07 01:38:00 2018

WebVR: fix GpuFence mode timing, set as default

Use a completion fence for the previous frame to avoid overstuffing GVR's
buffer queue and blocking on GVR frame Submit. This fixes the inconsistent
timing in this mode that was due to event handling being delayed during
the blocking wait. Based on initial testing this mode now seems to be
generally more efficient than the baseline ClientWait mode, so setting
it to default to get more data. This can be overridden via Finch
parameter if needed.

Bug:  760389 
Change-Id: I8bdf84e7fa2659e26daa467d815dc9b862c99610
Reviewed-on: https://chromium-review.googlesource.com/905786
Reviewed-by: Ian Vollick <vollick@chromium.org>
Commit-Queue: Klaus Weidner <klausw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534865}
[modify] https://crrev.com/5859322791875be6f682139f736a090b58339c55/chrome/browser/android/vr_shell/vr_shell_gl.cc
[modify] https://crrev.com/5859322791875be6f682139f736a090b58339c55/chrome/browser/android/vr_shell/vr_shell_gl.h

The commit in comment 10 has the side effect of working around this by using a different rendering path that is not affected by the driver issue.

Following the repro steps in the initial report of this bug, with the following setup:
Device: S8+
Chrome: 66.0.3355.0
Android 7.0

I observed:

- Just keeping still, with no head movement, ~26 - 30 fps (noticed a couple outliers of 24 and 31)

I am getting ~30fps for https://webvr.info/samples/test-slow-render.html?heavyGpu=1&workTime=4&cubeScale=0.3 which is about in the expected range. Based on the initial comment, it sounds as if framerates were lower when this was first tested. I think other improvements reduced the impact of the S8's bug on this specific heavyweight test.

A more direct test for this specific issue is https://webvr.info/samples/03-vr-presentation.html which gets ~20fps in the previously-default "ClientWait" render path, and 50-60fps (with some lower outliers) using the new default "GpuFence" render path.

It's still not great, it really needs the driver fix to get better and more consistent results. 

I suspect that https://chromium-review.googlesource.com/c/chromium/src/+/905786 which fixed the GpuFence timing estimates may have made things worse again for the S8 - it reintroduces a fence check which is the problematic call for the buggy driver, but that seemed necessary to get better results on the other devices.

Reopening. Looks like the driver bug specifically only affects ClientWait with a nonzero timeout. Polling with a zero timeout works as expected. I'm looking into adding a new workaround which does this. Cost is slightly higher CPU load due to a shorter polling interval when the workaround is active.

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

commit c04d179e47d26700fe5c4b0e2dc6024a76d31cce
Author: Klaus Weidner <klausw@chromium.org>
Date: Wed Feb 28 01:09:05 2018

Add GPU workaround for eglClientWaitSyncKHR timeout on S8

Samsung's implementation of the Adreno 540 driver on Android N as used
on the S8/S8+ has a bug where eglClientWaitSyncKHR with a nonzero
timeout waits much longer than the specified timeout. This is fixed
in the Android O update for these devices.

Workaround is to replace the timeout with polling for completion.
Since we don't know if we need the workaround until the GPU process
initialization completes, add a OnGpuConnectionReady callback
in VrShellGl to check for this.

This bug is specific to Samsung S8/S8+ on Android N, but there isn't
currently a way to restrict the filter to those devices. The
functionality is currently only used for WebVR on Daydream ready
devices, and the non-Samsung Daydream devices generally use Android O,
so an overbroad match seems acceptable.

BUG= 760389 

Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I72b92d529307fe114d850eea576fd41fe0fcb645
Reviewed-on: https://chromium-review.googlesource.com/939724
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Brandon Jones <bajones@chromium.org>
Commit-Queue: Klaus Weidner <klausw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#539601}
[modify] https://crrev.com/c04d179e47d26700fe5c4b0e2dc6024a76d31cce/chrome/browser/android/vr/mailbox_to_surface_bridge.cc
[modify] https://crrev.com/c04d179e47d26700fe5c4b0e2dc6024a76d31cce/chrome/browser/android/vr/mailbox_to_surface_bridge.h
[modify] https://crrev.com/c04d179e47d26700fe5c4b0e2dc6024a76d31cce/chrome/browser/android/vr/vr_shell_gl.cc
[modify] https://crrev.com/c04d179e47d26700fe5c4b0e2dc6024a76d31cce/chrome/browser/android/vr/vr_shell_gl.h
[modify] https://crrev.com/c04d179e47d26700fe5c4b0e2dc6024a76d31cce/gpu/config/gpu_driver_bug_list.json
[modify] https://crrev.com/c04d179e47d26700fe5c4b0e2dc6024a76d31cce/gpu/config/gpu_driver_bug_workaround_type.h

The patch is merged and included in Canary 66.0.3357.0. It avoids the problematic excessively long wait. Marking as fixed since this specific issue is now addressed, and framerates have generally improved and are more consistent.

However, performance is still lower than expected. The S8+ I was testing on has a Adreno 540 GPU, same as the Pixel 2 XL, but is generally slower:

https://webvr.info/samples/test-slow-render.html?heavyGpu=1&workTime=4&cubeScale=0.3 - 35fps vs 50+fps

Under Neon Lights: complex part at 20fps vs 30fps

If applicable, and if it's not already addressed by the updated O+ GPU driver, we can follow up on that separately. 

Requesting merge of change https://chromium-review.googlesource.com/939724 from comment #18 into M66.

This had missed the branch point and was intended to be included to work around the severe performance issues. The change should only affect WebVR in Daydream mode on Samsung S8 and Pixel phones that are still running Android N, and has been tested in Canary.

Reopening for merge request

Your change meets the bar and is auto-approved for M66. Please go ahead and merge the CL to branch 3359 manually. Please contact milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), josafat@(ChromeOS), abdulsyed@(Desktop)

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

Removing the "merge" labels, this change is in 3359 already. Sorry about the confusion, I must have checked the wrong label when looking at history.