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

Issue 859952 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 11
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression
Proj-VR
Proj-XR
Proj-XR-VR

Blocked on:
issue 861920

Blocking:
issue 850271
issue 879704



Sign in to add a comment

VR Instrumentation tests hitting ring_buffer DCHECK

Project Member Reported by bsheedy@chromium.org, Jul 3

Issue description

Many VR tests are flakily hitting the DCHECK in ring_buffer.cc making sure that the available size is aligned (https://cs.chromium.org/chromium/src/gpu/command_buffer/client/ring_buffer.cc?q=ring_buffer.cc&sq=package:chromium&dr&l=151). This seems to be possible for any VR browser-related test, but appears most common in the VrShellNavigationTest#* ones.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jul 3

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

commit 6df2efa5313f48302a51d510fa9b5983811c2328
Author: Brian Sheedy <bsheedy@chromium.org>
Date: Tue Jul 03 17:42:32 2018

Revert "GPU: Grow and shrink transfer buffer"

This reverts commit 65c38f54b0ae666e8b27bfa5aef6d3ab92d4a783.

Reason for revert: Causing DCHECK failures in VR instrumentation tests

Original change's description:
> GPU: Grow and shrink transfer buffer
> 
> Automatically grow the transfer buffer when it is full and shrink it
> when the full capacity is not being used. Allow choosing larger and
> smaller sizes than before.
> 
> Before we would only grow it when a single transfer request was larger
> than the whole buffer; requests smaller than the whole buffer would just
> block if the buffer was full. Also we would not ever shrink the buffer
> until someone called Free() manually.
> 
> Bug:  850271 , 835353, 828363,  856347 
> Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
> Change-Id: If2f2235a1d5297c63663398b37e1e30791347a3e
> Reviewed-on: https://chromium-review.googlesource.com/1105505
> Commit-Queue: James Darpinian <jdarpinian@chromium.org>
> Reviewed-by: Antoine Labour <piman@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#572135}

TBR=jdarpinian@chromium.org,sunnyps@chromium.org,piman@chromium.org

Change-Id: I031b30234cdcfbd8a4e48c7ebb6fd23312ec10ed
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  850271 , 835353, 828363,  856347 ,  859952 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Reviewed-on: https://chromium-review.googlesource.com/1124822
Reviewed-by: Brian Sheedy <bsheedy@chromium.org>
Commit-Queue: Brian Sheedy <bsheedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572277}
[modify] https://crrev.com/6df2efa5313f48302a51d510fa9b5983811c2328/gpu/command_buffer/client/client_test_helper.cc
[modify] https://crrev.com/6df2efa5313f48302a51d510fa9b5983811c2328/gpu/command_buffer/client/client_test_helper.h
[modify] https://crrev.com/6df2efa5313f48302a51d510fa9b5983811c2328/gpu/command_buffer/client/gles2_implementation.cc
[modify] https://crrev.com/6df2efa5313f48302a51d510fa9b5983811c2328/gpu/command_buffer/client/implementation_base.cc
[modify] https://crrev.com/6df2efa5313f48302a51d510fa9b5983811c2328/gpu/command_buffer/client/implementation_base.h
[modify] https://crrev.com/6df2efa5313f48302a51d510fa9b5983811c2328/gpu/command_buffer/client/mock_transfer_buffer.cc
[modify] https://crrev.com/6df2efa5313f48302a51d510fa9b5983811c2328/gpu/command_buffer/client/mock_transfer_buffer.h
[modify] https://crrev.com/6df2efa5313f48302a51d510fa9b5983811c2328/gpu/command_buffer/client/ring_buffer.h
[modify] https://crrev.com/6df2efa5313f48302a51d510fa9b5983811c2328/gpu/command_buffer/client/shared_memory_limits.h
[modify] https://crrev.com/6df2efa5313f48302a51d510fa9b5983811c2328/gpu/command_buffer/client/transfer_buffer.cc
[modify] https://crrev.com/6df2efa5313f48302a51d510fa9b5983811c2328/gpu/command_buffer/client/transfer_buffer.h
[modify] https://crrev.com/6df2efa5313f48302a51d510fa9b5983811c2328/gpu/command_buffer/client/transfer_buffer_unittest.cc

Owner: jdarpinian@chromium.org
Status: Assigned (was: Available)
I was only able to repro this on the Pixel XLs with N on swarming, not with my Pixel 2 with O locally.

https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Nougat%20Phone%20Tester/6770 is a specific failed build, logcat output and such can be found under "result_details" (note that the WebVrInputTest#testFocusUpdatesSynchronously failures are unrelated).

Sample logcat output: https://luci-logdog.appspot.com/v/?s=chromium%2Fandroid%2Fswarming%2Flogcats%2F3e791cc68e022111%2F%2B%2Flogcat_logcat_org.chromium.chrome.browser.vr_shell.VrShellNavigationTest.testWebVrFullscreenToWebVr_20180703T134217-UTC_HT73Y0201289

The provided stack trace is garbage when symbolized: https://luci-logdog.appspot.com/v/?s=chromium%2Fandroid%2Fswarming%2Flogcats%2F3e791cc68e022111%2F%2B%2Ftombstones_tombstones_20180703T134231-UTC_HT73Y0201289
How can I repro this myself? Is there a way to run it on swarming?
Yes, this repros on swarming. I can get you the exact command line I used if you need on Monday when I'm back in the office, but it should be doable using mb.py's swarming run with the following dimensions:

pool Chrome
device_type marlin
device_os NMF26U

The target you'll want to build is chrome_public_test_vr_apk. I used the same GN args as the bot, but it might repro with others:

ffmpeg_branding = "Chrome"
is_component_build = false
is_debug = true
proprietary_codecs = true
strip_absolute_paths_from_debug_symbols = true
symbol_level = 1
target_cpu = "arm64"
target_os = "android"
use_goma = true

You'll also need to some special steps in order to install and setup an additional APK necessary for VR tests. Full instructions are available at https://cs.chromium.org/chromium/src/chrome/android/javatests/src/org/chromium/chrome/browser/vr_shell/README.md?l=56, but the TL;DR is:

 * Set the DOWNLOAD_VR_TEST_APKS environment variable locally
 * Run 'gclient runhooks'
 * When triggering the test, pass in these two additional arguments:
-replace-system-package com.google.vr.vrcore,//third_party/gvr-android-sdk/test-apks/vr_services/vr_services_current.apk
--shared-prefs-file //chrome/android/shared_preference_files/test/vr_ddview_skipdon_setupcomplete.json
Blocking: 850271
Cc: jdarpinian@chromium.org
Owner: kbr@chromium.org
Taking over from James while he's OOO.

Here's the exact command I used to trigger the tests on swarming and repro:

ninja -C out/nougatbot -j1000 -l70 chrome_public_test_vr_apk && tools/mb/mb.py run -s --no-default-dimensions -d pool Chrome -d device_os NMF26U -d device_type marlin //out/nougatbot chrome_public_test_vr_apk -- --shared-prefs-file=//chrome/android/shared_preference_files/test/vr_ddview_skipdon_setupcomplete.json --replace-system-package=com.google.vr.vrcore,//third_party/gvr-android-sdk/test-apks/vr_services/vr_services_current.apk --num-retries 0 '--logcat-output-dir=${ISOLATED_OUTDIR}' --test-filter VrShellNavigationTest#*
Cc: dpranke@chromium.org kbr@chromium.org
Owner: jdarpinian@chromium.org
To run these tests locally:
1) Flash a Pixel 1 XL with NMF26U Userdebug.
2) Do the following:

export DOWNLOAD_VR_TEST_APKS=1
python third_party/gvr-android-sdk/test-apks/update.py

3) Set up e.g. out/AndroidDebug with the following args.gn:

------
# Build arguments go here.
# See "gn args <out_dir> --list" for available build arguments.
target_os = "android"
target_cpu = "arm64"  # (default)
is_debug = true  # (default)
ffmpeg_branding = "Chrome"
proprietary_codecs = true

# Other args you may want to set:
is_component_build = true
# is_clang = true
strip_absolute_paths_from_debug_symbols = true
symbol_level = 1  # Faster build with fewer symbols. -g1 rather than -g2
# enable_incremental_javac = true  # Much faster; experimental

use_goma = true
goma_dir = "/usr/local/google/home/kbr/goma"
-----

4) Apply the following patch to tools/swarming_client/isolate.py:

------
diff --git a/isolate.py b/isolate.py
index 795c4e4..407305d 100755
--- a/isolate.py
+++ b/isolate.py
@@ -1036,7 +1036,8 @@ def CMDrun(parser, args):
           (' '.join(cmd), cwd))
       result = 1
   finally:
-    file_path.rmtree(outdir)
+    # file_path.rmtree(outdir)
+    print 'Deliberately leaking temp dir ' + outdir
 
   if complete_state.isolated_filepath:
     complete_state.save_files()
------

Otherwise the logs will be lost when the run completes. (I didn't test this but found that the logs were lost)


5) Run via the following (replace AndroidDebug with the name of your build directory):

tools/mb/mb.py run //out/AndroidDebug chrome_public_test_vr_apk -- --shared-prefs-file=//chrome/android/shared_preference_files/test/vr_ddview_skipdon_setupcomplete.json --replace-system-package=com.google.vr.vrcore,//third_party/gvr-android-sdk/test-apks/vr_services/vr_services_current.apk --num-retries 0 '--logcat-output-dir=${ISOLATED_OUTDIR}' --test-filter VrShellNavigationTest#*


This will take a long time and fail a lot of tests. I recommend starting with a --test-filter that runs just one of these.

Giving back to James as he's not OOO yet.

Blockedon: 861920
I have found and fixed the problem for the reland. On Android the compositor sets the max transfer buffer size to the size of a full screen texture, which is not necessarily divisible by the alignment. My fix is to round the transfer buffer size to the alignment.

I found this script useful to run the tests quickly and output correct stack traces:

# turn on and unlock screen
adb shell input keyevent 82
adb shell input keyevent 82
time $OUT_DIR/bin/run_chrome_public_test_vr_apk \
  --shared-prefs-file=//chrome/android/shared_preference_files/test/vr_ddview_skipdon_setupcomplete.json \
  --replace-system-package=com.google.vr.vrcore,//third_party/gvr-android-sdk/test-apks/vr_services/vr_services_current.apk \
  --num-retries 0 \
  --logcat-output-dir=$ISOLATED_OUTDIR \
  --test-filter VrShellNavigationTest#* \
  || cat $ISOLATED_OUTDIR/$(ls -Art $ISOLATED_OUTDIR | tail -n 1) | ./third_party/android_platform/development/scripts/stack --output-dir=$OUT_DIR

In addition I found it helpful to comment out all the reboot-related stuff in third_party/catapult/devil/devil/android/tools/system_app.py EnableSystemAppModification, which would waste minutes rebooting the device multiple times for no reason otherwise.
Status: Fixed (was: Assigned)
The fixed reland CL is here, still waiting on a fix for  bug 859998  before submitting: https://chromium-review.googlesource.com/c/chromium/src/+/1125503
Labels: -VR-Caught-By-Test XR-Caught-By-Test
Blocking: 879704

Sign in to add a comment