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

Issue 774748 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 27
Cc:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 1
Type: Bug

Blocking:
issue 773960



Sign in to add a comment

Collect as much GPUInfo as possible when errors happen

Project Member Reported by zmo@chromium.org, Oct 14 2017

Issue description

At the moment collection happens in ANGLE lib, and if an error is encountered, nothing is copied over to GPUInfo.

This may not be optimal for the wild wild world Chrome is facing.

We still should return as much as possible, and allow blacklisting/driver bug workarounds to be computed on partial information.
 
Cc: zmo@chromium.org
 Issue 774891  has been merged into this issue.
Owner: cwallez@chromium.org
Status: Started (was: Available)

Comment 3 by ericrk@chromium.org, Oct 16 2017

Blocking: 773960
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 16 2017

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

commit 2257d92b4adbf4681614c93e60553eb332bb0fb3
Author: Corentin Wallez <cwallez@chromium.org>
Date: Mon Oct 16 18:00:05 2017

gpu/config: Return partial info from angle::GetSystemInfo

A filure in angle::GetSystemInfo will still return a gpu info collection
failure but partial results will be shown in about:gpu which will help
diagnosing problems.

BUG= chromium:774748 

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: I87dc125d72a0566dc3e3ae02b570237e654b7181
Reviewed-on: https://chromium-review.googlesource.com/721499
Reviewed-by: Zhenyao Mo <zmo@chromium.org>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509103}
[modify] https://crrev.com/2257d92b4adbf4681614c93e60553eb332bb0fb3/gpu/config/gpu_info_collector.cc
[modify] https://crrev.com/2257d92b4adbf4681614c93e60553eb332bb0fb3/gpu/config/gpu_info_collector_linux.cc
[modify] https://crrev.com/2257d92b4adbf4681614c93e60553eb332bb0fb3/gpu/config/gpu_info_collector_mac.mm

ericrk@: Please confirm that this CL fixes the problem we were having, then I'll do the merge request.
Project Member

Comment 6 by bugdroid1@chromium.org, Oct 18 2017

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

commit c9203cfa3eb511238e67fcdf3892add8b1f68086
Author: Corentin Wallez <cwallez@chromium.org>
Date: Wed Oct 18 22:30:15 2017

Roll ANGLE 2d88e9b..d042fba

https://chromium.googlesource.com/angle/angle.git/+log/2d88e9b..d042fba

BUG=,chromium:774309

TBR=geofflang@chromium.org

TEST=bots

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel

Merge branch 'master' of https://chromium.googlesource.com/chromium/src


gpu/config: Return partial info from angle::GetSystemInfo

A filure in angle::GetSystemInfo will still return a gpu info collection
failure but partial results will be shown in about:gpu which will help
diagnosing problems.

BUG= chromium:774748 

Change-Id: I000b9f83dc35389beeb14657460baed22ac25daa
Reviewed-on: https://chromium-review.googlesource.com/726020
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Geoff Lang <geofflang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509898}
[modify] https://crrev.com/c9203cfa3eb511238e67fcdf3892add8b1f68086/DEPS

Comment 7 by ericrk@chromium.org, Oct 18 2017

Labels: -Pri-2 Pri-1
cwallez@, re #5, I can't reproduce this issue, so we're relying on users. The user in the email thread indicated that zmo@'s custom build fixed the issue for them, so assuming this is along the same lines, we should be good. If so, please request a merge.
Labels: Merge-Request-63 Merge-Request-62
Thanks for the update. I believe this fix should be merged in both M62 and M63 and is pretty low risk.
Project Member

Comment 9 by sheriffbot@chromium.org, Oct 19 2017

Labels: -Merge-Request-62 Merge-Review-62 Hotlist-Merge-Review
This bug requires manual review: Request affecting a post-stable build
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Cc: abdulsyed@chromium.org
+abdulsyed - FYI, this is the alternative fix for  issue 751919  which we discussed on Monday. This fix is more comprehensive and simpler, so I think this is the one we should take. We've had a number of additional user reports of the issue (see  issue 773960  which is blocked on this), so I think this is a real issue.
Thanks ericrk@ - have we already confirmed this in Canary or Dev?  Is the merge for both 4 and 6?
cwallez@ - I assume we only need to merge 4?

Users have confirmed the fix in Canary (see blocked bug).
ericrk@ correct, only #4 is needed.

Comment 14 by zmo@chromium.org, Oct 19 2017

Per data from Steve, on the build I gave him (with similar change to #4), the bug was fixed, but also, the ANGLE side collection doesn't fail, which means the bug should be fixed even without the change.

I just don't want us to merge back a change that doesn't actually address the issue. There might be a fix already landed on ToT before #4.

If anyone has a machine that can reproduce this issue, we should bisect to make sure.
Project Member

Comment 15 by sheriffbot@chromium.org, Oct 19 2017

Labels: -Merge-Request-63 Merge-Review-63
This bug requires manual review: DEPS changes referenced in bugdroid comments.
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), gkihumba@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
For M63 merge: Per comment #14, change listed at #4 may not a need a merge. Please see #14 for more details and confirm merge is needed or not. Thank you.
I don't know that we have any local devices that repro. Re #16, #14 - Looking at our metrics, we can see the number of users with 0x0 for device ID and vendor ID. This number is consistently positive in Canary, and drops to 0 after the change in #4. This seems to indicate that the issue was not fixed on ToT and that the latest change will improve things for some number of users. Given this, I'd lean towards a merge. zmo@, WDYT?
zmo@ I agree that this CL probably doesn't fix the GPU info collection failure we saw, however don't you think it would be useful to merge the CL at least in M63 to help diagnose issues that might arise with collection?

Comment 19 by zmo@chromium.org, Oct 20 2017

OK, my data point is from one person (Steve), which contradicts the metrics data mentioned in #17. I think we should listen to metrics data. Let's merge.
Agreeing with #19 - from discussions here and offline, I think we want to merge the patch in #4 to M62.
To clarify #20, we'd like to merge to both M62 and M63, not just M62.
Labels: -Merge-Review-63 Merge-Approved-63
Approving merge to M63 branch 3239 based on comments #12, #20 and #21. Please merge ASAP. Thank you.
Thanks ericrk@, cwallez@, zmo@ - I'm a bit concerned regarding the merge to M62. Since we are already at stable and merge requirements are quite high, what are the real benefits of merging this to M62? From 14,17,18,19, seems like this isn't actually fixing but more for collecting more data to help diagnose? How urgent is this to merge for M62?

Comment 24 by zmo@chromium.org, Oct 23 2017

It's not simply for collecting data.  Without the fix, the data collection fail partially but also discard the valid part. Therefore, feature blacklisting wasn't effective. This was a simple/safe fix, but the results on effected machines could be dramatic.
Project Member

Comment 25 by bugdroid1@chromium.org, Oct 23 2017

Labels: -merge-approved-63 merge-merged-3239
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/17ff39fa62d666ef2951832b72557f6f112a2fdc

commit 17ff39fa62d666ef2951832b72557f6f112a2fdc
Author: Corentin Wallez <cwallez@chromium.org>
Date: Mon Oct 23 16:16:53 2017

gpu/config: Return partial info from angle::GetSystemInfo

A filure in angle::GetSystemInfo will still return a gpu info collection
failure but partial results will be shown in about:gpu which will help
diagnosing problems.

BUG= chromium:774748 

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: I87dc125d72a0566dc3e3ae02b570237e654b7181
Reviewed-on: https://chromium-review.googlesource.com/721499
Reviewed-by: Zhenyao Mo <zmo@chromium.org>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#509103}(cherry picked from commit 2257d92b4adbf4681614c93e60553eb332bb0fb3)
Reviewed-on: https://chromium-review.googlesource.com/733500
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Cr-Commit-Position: refs/branch-heads/3239@{#152}
Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578}
[modify] https://crrev.com/17ff39fa62d666ef2951832b72557f6f112a2fdc/gpu/config/gpu_info_collector.cc
[modify] https://crrev.com/17ff39fa62d666ef2951832b72557f6f112a2fdc/gpu/config/gpu_info_collector_linux.cc
[modify] https://crrev.com/17ff39fa62d666ef2951832b72557f6f112a2fdc/gpu/config/gpu_info_collector_mac.mm

Ok thanks for confirming. Our M62 Stable is planned for Wednesday, 10/25. Let's have it tested in Dev tomorrow, and then I can approve merge to M62 tomorrow afternoon. 
Labels: -Merge-Review-62 Merge-Approved-62
Approving merge for M62. Branch:3202. 

Let's still confirm this in Dev; however, approving it so we can avoid any last minute merge issues. 
Project Member

Comment 28 by bugdroid1@chromium.org, Oct 25 2017

Labels: -merge-approved-62 merge-merged-3202
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/56ded35dea3c64aa3936cc85f5973be17850c8a4

commit 56ded35dea3c64aa3936cc85f5973be17850c8a4
Author: Corentin Wallez <cwallez@chromium.org>
Date: Wed Oct 25 14:47:19 2017

gpu/config: Return partial info from angle::GetSystemInfo

A filure in angle::GetSystemInfo will still return a gpu info collection
failure but partial results will be shown in about:gpu which will help
diagnosing problems.

BUG= chromium:774748 

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: I87dc125d72a0566dc3e3ae02b570237e654b7181
Reviewed-on: https://chromium-review.googlesource.com/721499
Reviewed-by: Zhenyao Mo <zmo@chromium.org>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#509103}(cherry picked from commit 2257d92b4adbf4681614c93e60553eb332bb0fb3)
Reviewed-on: https://chromium-review.googlesource.com/736060
Cr-Commit-Position: refs/branch-heads/3202@{#746}
Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098}
[modify] https://crrev.com/56ded35dea3c64aa3936cc85f5973be17850c8a4/gpu/config/gpu_info_collector.cc
[modify] https://crrev.com/56ded35dea3c64aa3936cc85f5973be17850c8a4/gpu/config/gpu_info_collector_linux.cc
[modify] https://crrev.com/56ded35dea3c64aa3936cc85f5973be17850c8a4/gpu/config/gpu_info_collector_mac.mm

Status: Fixed (was: Started)

Sign in to add a comment