New issue
Advanced search Search tips

Issue 750286 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug
Proj-XR

Blocking:
issue 747159



Sign in to add a comment

WebVR: reduce sync IPCs on submitFrame

Project Member Reported by klausw@chromium.org, Jul 28 2017

Issue description

In  issue 747159 , kbr@ pointed out that our current WebVR pipeline is doing too many synchronous IPC calls to the GPU process each frame.

This appears to be caused by using an inefficient method to capture an image for the current context. We should use the same logic as WebGLRenderingContextBase::commit() instead.
 

Comment 1 by klausw@chromium.org, Jul 28 2017

Status: Started (was: Untriaged)

Comment 2 by klausw@chromium.org, Jul 28 2017

Here are trace files for comparison. trace_new-GetStaticBitmapImage uses the proposed fix from https://chromium-review.googlesource.com/c/592249/ . Both still show the slow/fast pattern (see  issue 747159  for that), but the sync IPCs in CrRenderMain's submitFrame are reduced from three to one.
trace_new-GetStaticBitmapImage.html
3.7 MB View Download
trace_totnoalign-fastslowpattern.html
4.2 MB View Download
Project Member

Comment 3 by bugdroid1@chromium.org, Aug 8 2017

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

commit 30ba029f1291127d6f5df4f478e9629f82609415
Author: Klaus Weidner <klausw@chromium.org>
Date: Tue Aug 08 18:35:02 2017

Use bitmap creation from commit() for WebVR

The WebVR pipeline used WebGLRenderingContextBase's GetImage which
required a total of three synchronous IPC calls to the GPU process.

Instead, reuse the logic from the commit() method, this needs just
a single SyncChannel::Send. Also, this made it possible to remove
additional flushes that had been needed to work around synchronization
issues.

BUG= 750286 
R=bajones@chromium.org

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: Ia27766575c183212aa59c52e86bfd1f619763c73
Reviewed-on: https://chromium-review.googlesource.com/592249
Reviewed-by: Brandon Jones <bajones@chromium.org>
Commit-Queue: Klaus Weidner <klausw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#492707}
[modify] https://crrev.com/30ba029f1291127d6f5df4f478e9629f82609415/third_party/WebKit/Source/modules/vr/VRDisplay.cpp
[modify] https://crrev.com/30ba029f1291127d6f5df4f478e9629f82609415/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp
[modify] https://crrev.com/30ba029f1291127d6f5df4f478e9629f82609415/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.h

Labels: -M-61 M-62
Status: Fixed (was: Started)
This is included in today's canary 62.0.3180.0. Adjusting milestone to M62 since I think there's no current plans to cherrypick this to M61, but we may want to revisit this in case it does improve performance sufficiently.
Components: Blink>WebXR

Sign in to add a comment