1737682 - Add telemetry to measure cases where we isolate macOS video layers

Status: RESOLVED FIXED

(Core :: Graphics, task)

Description

[Brad Werth [:bradwerth] | Out until March 3]

The changes in Bug 1653417 are designed to put macOS fullscreen video in a low-power, "detached" state. Though we can't directly detect if we are successfully reaching the detached state, we can measure when we are trying, which is any time that video layer isolation is attempted.

Comment 1

[Brad Werth [:bradwerth] | Out until March 3]

It will also be necessary to note when we actually attempt to enqueue a surface in the video layer. Detached mode is only possible when both code paths are active.

Comment 2

[Brad Werth [:bradwerth] | Out until March 3]

Attachment: Bug 1737682 Part 1: Cache calls to NativeLayerCA::ShouldSpecializeVideo in a state bit.

This guarantees that any time mMutatedSpecializeVideo is set to true, the next
call to ApplyChanges will receive the value that actually triggered the
mutation. This was probably already true, but this change makes it explicit
and similar to other state changes.

Comment 3

[Brad Werth [:bradwerth] | Out until March 3]

Attachment: Bug 1737682 Part 1: Log whether or not we are hitting the detached state.

Depends on D129452

Comment 4

[Brad Werth [:bradwerth] | Out until March 3]

Changes in other Bugs will require D129452, which is r+, so I'm going to land it separately and keep this bug open while the remainder is reviewed.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/117961db7b95
Part 1: Cache calls to NativeLayerCA::ShouldSpecializeVideo in a state bit. r=gfx-reviewers,mstange

Comment 6

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/117961db7b95

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ce4dd013648f
Part 2: Log whether or not we are hitting the detached state. r=mstange

Comment 8

[Cristina Cozmuta (:CrissCozmuta)]

Backed out for causing build bustages on TelemetryHistogramEnums.h. CLOSED TREE

Backout link : https://hg.mozilla.org/integration/autoland/rev/da9d46bd54e7f8bc0f6d670e4fe634a16ba398a8

Push with failures : https://treeherder.mozilla.org/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Crunnable&revision=ce4dd013648f31a6c279f683f6a084453f67d6a7&selectedTaskRun=SNj6DG_SSpO_BkaupkOzMw.0

Link to failure log : https://treeherder.mozilla.org/logviewer?job_id=361662055&repo=autoland&lineNumber=8763

Failure message : /builds/worker/workspace/obj-build/dist/include/mozilla/TelemetryHistogramEnums.h:4774:3: error: expected identifier

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a7c322ebcfea
Part 2: Log whether or not we are hitting the detached state. r=mstange

Comment 10

[Sandor Molnar[:smolnar]]

Backed out for causing high frequency bc failures in browser_panelUINotifications_multiWindow.

Backout link: https://hg.mozilla.org/integration/autoland/rev/bc92598023d3a538ed5740bfa61022fec7b446d1

Push with failures

Failure log

INFO - PROCESS-CRASH | browser/components/customizableui/test/browser_panelUINotifications_multiWindow.js | application crashed [@ mozilla::detail::InvalidArrayIndex_CRASH(unsigned long, unsigned long)]
[task 2021-12-17T22:51:24.474Z] 22:51:24     INFO - Crash dump filename: /var/folders/yh/ndl89gtn6ld5tws_rhb495_r000014/T/tmpdunx67ab.mozrunner/minidumps/ED85CEB4-DCF0-44A5-A765-7DB5640D33A5.dmp
[task 2021-12-17T22:51:24.474Z] 22:51:24     INFO - Operating system: Mac OS X
[task 2021-12-17T22:51:24.474Z] 22:51:24     INFO -                   10.15.7 19H524
[task 2021-12-17T22:51:24.474Z] 22:51:24     INFO - CPU: amd64
[task 2021-12-17T22:51:24.474Z] 22:51:24     INFO -      family 6 model 158 stepping 10
[task 2021-12-17T22:51:24.475Z] 22:51:24     INFO -      12 CPUs
[task 2021-12-17T22:51:24.475Z] 22:51:24     INFO - 
[task 2021-12-17T22:51:24.475Z] 22:51:24     INFO - Crash reason:  EXC_BAD_ACCESS / KERN_INVALID_ADDRESS
[task 2021-12-17T22:51:24.475Z] 22:51:24     INFO - Crash address: 0x0
[task 2021-12-17T22:51:24.475Z] 22:51:24     INFO - Mac Crash Info:
[task 2021-12-17T22:51:24.475Z] 22:51:24     INFO - 
[task 2021-12-17T22:51:24.475Z] 22:51:24     INFO - Process uptime: 107 seconds
[task 2021-12-17T22:51:24.475Z] 22:51:24     INFO - 
[task 2021-12-17T22:51:24.475Z] 22:51:24     INFO - Thread 0 GeckoMain (crashed)
[task 2021-12-17T22:51:24.475Z] 22:51:24     INFO -  0  libmozglue.dylib!mozilla::detail::InvalidArrayIndex_CRASH(unsigned long, unsigned long) [Assertions.cpp:a7c322ebcfea97985f4ae7708cf94793e89d255e : 50 + 0xe]
[task 2021-12-17T22:51:24.475Z] 22:51:24     INFO -     rax = 0x000000010d3d4230   rdx = 0x0000000000000000
[task 2021-12-17T22:51:24.475Z] 22:51:24     INFO -     rcx = 0x68d4ecfe010000a1   rbx = 0x0000000000000000
[task 2021-12-17T22:51:24.475Z] 22:51:24     INFO -     rsi = 0x000000010d3cf6fa   rdi = 0x00007ffee28b9ea0
[task 2021-12-17T22:51:24.475Z] 22:51:24     INFO -     rbp = 0x00007ffee28ba310   rsp = 0x00007ffee28ba310
[task 2021-12-17T22:51:24.475Z] 22:51:24     INFO -      r8 = 0x0000000000000000    r9 = 0x0000000000000005
[task 2021-12-17T22:51:24.475Z] 22:51:24     INFO -     r10 = 0x0000000000000000   r11 = 0x0000000000004b6a
[task 2021-12-17T22:51:24.475Z] 22:51:24     INFO -     r12 = 0x0000000110e3ee80   r13 = 0x000000012e38e930
[task 2021-12-17T22:51:24.476Z] 22:51:24     INFO -     r14 = 0x000000012e38e8f0   r15 = 0x000000012e38e801
[task 2021-12-17T22:51:24.476Z] 22:51:24     INFO -     rip = 0x000000010d3c8de8

Comment 11

[Brad Werth [:bradwerth] | Out until March 3]

(In reply to Sandor Molnar from comment #10)

Backed out for causing high frequency bc failures in browser_panelUINotifications_multiWindow.

Hmm, looks like this has changed the logic to detect when there are no sublayers. I'll build a fix.

Comment 12

[Sandor Molnar[:smolnar]]

there are also some dt failures that came up.

Comment 13

[Brad Werth [:bradwerth] | Out until March 3]

This is taking me some time to fix correctly. I'll revisit when I'm back from PTO.

Comment 14

[Markus Stange [:mstange]]

Oh, and the telemetry changes need data review.

Comment 15

[Brad Werth [:bradwerth] | Out until March 3]

The video layer code may change some more as we solve the regressions in Bug 1731815. I'll rework this patch after that code settles down again.

Comment 16

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

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:bradwerth, could you have a look please?
If you still have some work to do, you can add an action "Plan Changes" in Phabricator.
For more information, please visit auto_nag documentation.

Comment 17

[Brad Werth [:bradwerth] | Out until March 3]

I'm cleared off some other things and will move this forward. The patch needs a new part with a test of capturing the telemetry.

Comment 18

[Brad Werth [:bradwerth] | Out until March 3]

Attachment: Bug 1737682 Part 2: Test video low power telemetry collection.

Depends on D129453

Comment 19

[Brad Werth [:bradwerth] | Out until March 3]

Attachment: request_GFX_MACOS_VIDEO_LOW_POWER.md

Comment 20

[Travis Long [:travis_]]

Comment on attachment 9283720 [details]
request_GFX_MACOS_VIDEO_LOW_POWER.md

Data Review

1. Is there or will there be documentation that describes the schema for the ultimate data set in a public, complete, and accurate way?

Yes, through the Histograms.json file and through the probe dictionary.

2. Is there a control mechanism that allows the user to turn the data collection on and off?

Yes, through the standard telemetry preference in the application settings.

3. If the request is for permanent data collection, is there someone who will monitor the data over time?

Permanent collection of this data to be monitored by Brad Werth ([e-mail])

4. Using the category system of data types on the Mozilla wiki, what collection type of data do the requested measurements fall under?

Category 2, User interaction

5. Is the data collection request for default-on or default-off?

Default-on

6. Does the instrumentation include the addition of any new identifiers (whether anonymous or otherwise; e.g., username, random IDs, etc. See the appendix for more details)?

No

7. Is the data collection covered by the existing Firefox privacy notice?

Yes

8. Does the data collection use a third-party collection tool?

No

Result

data-review+

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/362b1e430789
Part 1: Log whether or not we are hitting the detached state. r=mstange
https://hg.mozilla.org/integration/autoland/rev/297b16133aa9
Part 2: Test video low power telemetry collection. r=mstange,alwu

Comment 22

[Cosmin Sabou [:CosminS]]

Backed out for causing build bustages on TelemetryHistogramEnums.h

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=297b16133aa928dc16f052fa3ba04281846bcf94&selectedTaskRun=WnSwNqxjS62ot2-wgOzalQ.0

Failure log: https://treeherder.mozilla.org/logviewer?job_id=384913995&repo=autoland

Backout link: https://hg.mozilla.org/integration/autoland/rev/8a5bb39ad57a3c8327d633aa6d76078af698e1b7

Comment 23

[Brad Werth [:bradwerth] | Out until March 3]

Ugh, I forgot that "Success" is not allowed as a telemetry enum.

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ef90978439c5
Part 1: Log whether or not we are hitting the detached state. r=mstange
https://hg.mozilla.org/integration/autoland/rev/c1329427f36f
Part 2: Test video low power telemetry collection. r=mstange,alwu

Comment 25

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/ef90978439c5
https://hg.mozilla.org/mozilla-central/rev/c1329427f36f