1288746 - CANVAS_WEBGL_SUCCESS shows failures while CANVAS_WEBGL_FAILURE_ID is empty

Status: REOPENED

(Core :: Graphics: CanvasWebGL, defect, P3)

Description

[Jeff Muizelaar [:jrmuizel]]

We have some pings that look like this:

{'adapter': {u'GPUActive': True,
   u'RAM': 1024,
   u'description': u'NVIDIA GeForce 9600 GT',
   u'deviceID': u'0x0622',
   u'driver': u'nvd3dumx,nvwgf2umx,nvwgf2umx nvd3dum,nvwgf2um,nvwgf2um',
   u'driverDate': u'2-3-2015',
   u'driverVersion': u'9.18.13.4144',
   u'subsysID': u'505319da',
   u'vendorID': u'0x10de'},
  'application/buildId': u'20160720030208',
  'clientId': u'826ad85b-5fc1-419b-9c1c-38b0273c1f8a',
  'driverVersion': [9, 18, 13, 4144],
  'environment/build/version': u'50.0a1',
  'environment/system/gfx/adapters': [{u'GPUActive': True,
    u'RAM': 1024,
    u'description': u'NVIDIA GeForce 9600 GT',
    u'deviceID': u'0x0622',
    u'driver': u'nvd3dumx,nvwgf2umx,nvwgf2umx nvd3dum,nvwgf2um,nvwgf2um',
    u'driverDate': u'2-3-2015',
    u'driverVersion': u'9.18.13.4144',
    u'subsysID': u'505319da',
    u'vendorID': u'0x10de'}],
  'environment/system/gfx/features': {u'compositor': u'd3d11',
   u'd2d': {u'status': u'available', u'version': u'1.1'},
   u'd3d11': {u'blacklisted': False,
    u'status': u'available',
    u'textureSharing': True,
    u'version': 40960,
    u'warp': False}},
  'environment/system/os/name': u'Windows_NT',
  'environment/system/os/servicePackMajor': 1,
  'environment/system/os/servicePackMinor': 0,
  'environment/system/os/version': u'6.1',
  'features': {u'compositor': u'd3d11',
   u'd2d': {u'status': u'available', u'version': u'1.1'},
   u'd3d11': {u'blacklisted': False,
    u'status': u'available',
    u'textureSharing': True,
    u'version': 40960,
    u'warp': False}},
  'payload/histograms/CANVAS_WEBGL_SUCCESS': 0    4.0
  1    0.0
  2    0.0
  dtype: float64,
  'payload/keyedHistograms/CANVAS_WEBGL_ACCL_FAILURE_ID': None,
  'payload/keyedHistograms/CANVAS_WEBGL_FAILURE_ID': None}

It would be interesting to know why. One of the interesting things that I noticed is that dtype of CANVAS_WEBGL_SUCCESS in these cases seems to be float64 where as in some of the other cases it is int64

Comment 1

[Benoit Girard (:BenWa)]

A quick look shows we're missing a few failure paths:
https://dxr.mozilla.org/mozilla-central/rev/4c05938a64a7fde3ac2d7f4493aee1c5f2ad8a0a/dom/canvas/WebGLContext.cpp#846
https://dxr.mozilla.org/mozilla-central/rev/4c05938a64a7fde3ac2d7f4493aee1c5f2ad8a0a/dom/canvas/WebGLContext.cpp#979
https://dxr.mozilla.org/mozilla-central/rev/4c05938a64a7fde3ac2d7f4493aee1c5f2ad8a0a/dom/canvas/WebGLContext.cpp#1004
https://dxr.mozilla.org/mozilla-central/rev/4c05938a64a7fde3ac2d7f4493aee1c5f2ad8a0a/dom/canvas/WebGLContext.cpp#1014

Comment 2

[Benoit Girard (:BenWa)]

Attachment: Bug 1288746 - Handle missing WebGL failure IDs.

Review commit: https://reviewboard.mozilla.org/r/66520/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/66520/

Comment 3

[Benoit Girard (:BenWa)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=dea04b6d4288

Comment 4

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8773863 [details]
Bug 1288746 - Handle missing WebGL failure IDs.

https://reviewboard.mozilla.org/r/66520/#review63330

::: dom/canvas/WebGLContext.cpp:908
(Diff revision 1)
>  
>          return NS_OK;
>      }
>  
> +    nsCString failureId = NS_LITERAL_CSTRING("FEATURE_FAILURE_WEBGL_UNKOWN");
> +    auto autoTelemetry = mozilla::MakeScopeExit([&] {

No need to capture everything. Let's just capture failureId.

Comment 5

[Benoit Girard (:BenWa)]

Happy to make the change but I'd like to understand better.

Is there any downside to specifying & instead of failureId? Particularly in the generated code? I'd assume other than this being stricter about not accidentally capturing something else it should be equivalent right?

Comment 6

[Jeff Muizelaar [:jrmuizel]]

(In reply to Benoit Girard (:BenWa) from comment #5)
> Happy to make the change but I'd like to understand better.
> 
> Is there any downside to specifying & instead of failureId? Particularly in
> the generated code? I'd assume other than this being stricter about not
> accidentally capturing something else it should be equivalent right?

It doesn't seem like there's a big downside.

Google's style guide has had the following change:

-<p>Use lambda expressions where appropriate.  Do not use
-default lambda captures; write all captures explicitly.</p>
+<p>Use lambda expressions where appropriate.  Avoid
+default lambda captures when capturing <code>this</code> or
+if the lambda will escape the current scope.</p>

an Apache C++ style guide says:

    Prefer default capture by value, explicit capture by value, then capture by reference. To avoid dangling-pointer bugs, never use default capture by reference:

It seems like there's no strong reason to prefer either, so do what you think is best.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/aef31d829693
Handle missing WebGL failure IDs. r=jrmuizel

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/aef31d829693

Comment 9

[Jeff Muizelaar [:jrmuizel]]

Let's keep this opened until we can confirm the problem is gone.

Comment 10

[Benoit Girard (:BenWa)]

We should have some data by now. Let's check today so that we can keep looking if this wasn't it.

Comment 11

[Jeff Muizelaar [:jrmuizel]]

This is still very prevalent.

Of the 2613 pings from the last two days on Nightly with data in payload/histograms/CANVAS_WEBGL_SUCCESS, 2125 are missing data in payload/keyedHistograms/CANVAS_WEBGL_FAILURE_ID.

Comment 12

[Benoit Girard (:BenWa)]

Looking for more missed paths:
https://dxr.mozilla.org/mozilla-central/rev/1851b78b5a9673ee422f189b92e5f1e86b82a01c/dom/canvas/WebGLContext.cpp#689
https://dxr.mozilla.org/mozilla-central/rev/1851b78b5a9673ee422f189b92e5f1e86b82a01c/gfx/gl/GLLibraryEGL.cpp#350
https://dxr.mozilla.org/mozilla-central/rev/1851b78b5a9673ee422f189b92e5f1e86b82a01c/gfx/gl/GLLibraryEGL.cpp#375
https://dxr.mozilla.org/mozilla-central/rev/1851b78b5a9673ee422f189b92e5f1e86b82a01c/gfx/gl/GLLibraryEGL.cpp#415

Comment 13

[Benoit Girard (:BenWa)]

Attachment: Bug 1288746 - Handle missing EGL failure IDs. r=jmuizel

Comment 14

[Benoit Girard (:BenWa)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a3fb806941a7

Comment 15

[Benoit Girard (:BenWa)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/484087507430a3008fdc0d54685e3c3d4b82f948
Bug 1288746 - Handle missing EGL failure IDs. r=jmuizel

Comment 16

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/484087507430

Comment 17

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last 7 months.
:jgilbert, could you have a look please?
For more information, please visit auto_nag documentation.