Deblacklisting HW decoding and add Telemetry to see the decoder recovery time.

RESOLVED FIXED in Firefox 57

Status

()

enhancement
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: kaku, Assigned: kaku)

Tracking

(Blocks 1 bug)

unspecified
mozilla57
x86_64
Windows
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox57 fixed)

Details

Attachments

(5 attachments)

Assignee

Description

2 years ago
One more step toward bug 1372070.

We want to know how long it takes to recover video decoder from a GPU process crash. The data will help us make a call on removing blacklist.
Assignee

Updated

2 years ago
Blocks: 1372070
Assignee

Updated

2 years ago
Assignee: nobody → kaku
Status: NEW → ASSIGNED
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 6

2 years ago
mozreview-review
Comment on attachment 8903428 [details]
Bug 1393399 P1 - add telemetry probes;

https://reviewboard.mozilla.org/r/175268/#review180278

lgtm
Attachment #8903428 - Flags: review?(gsquelart) → review+

Comment 7

2 years ago
mozreview-review
Comment on attachment 8903429 [details]
Bug 1393399 P2 - keep the GPU process crash time and send back to MFR;

https://reviewboard.mozilla.org/r/175270/#review180288

::: dom/media/MediaResult.h:72
(Diff revision 1)
> +  const TimeStamp& GPUCrashTimeStamp() const { return mGPUCrashTimeStamp; }
> +
>  private:
>    nsresult mCode;
>    nsCString mMessage;
> +  TimeStamp mGPUCrashTimeStamp; // Used in bug 1393399 for telemetry usage.

Can you say "... for temporary telemetry usage" in the comment, to emphasize that this member will eventually be removed, when we don't need the telemetry anymore.
Attachment #8903429 - Flags: review?(gsquelart) → review+

Comment 8

2 years ago
mozreview-review
Comment on attachment 8903430 [details]
Bug 1393399 P3 - keep the MediaDecoderOwner's identification in MFR;

https://reviewboard.mozilla.org/r/175272/#review180282

::: commit-message-1175a:4
(Diff revision 1)
> +Bug 1393399 P3 - keep the MediaDecoderOwner's identification in MFR;
> +
> +When GPU process crashes, the MediaDecoder, MDSM, and MFR are all destroyed.
> +So, we use MediaDecoderOwner to identiy which video we're dealing with.

'identiy' -> 'identify'
Attachment #8903430 - Flags: review?(gsquelart) → review+

Comment 9

2 years ago
mozreview-review
Comment on attachment 8903431 [details]
Bug 1393399 P4 - implement GPUProcessCrashTelemetryLogger helper class;

https://reviewboard.mozilla.org/r/175274/#review180284

::: dom/media/MediaFormatReader.cpp:52
(Diff revision 1)
>  
> +
> +typedef void* MediaDataDecoderID;
> +
> +/**
> + * This helper class is used to report telemetry of the time used to recovery a

'recovery' -> 'recover'
Attachment #8903431 - Flags: review?(gsquelart) → review+

Comment 10

2 years ago
mozreview-review
Comment on attachment 8903432 [details]
Bug 1393399 P5 - report the recovery time telemetry;

https://reviewboard.mozilla.org/r/175276/#review180290
Attachment #8903432 - Flags: review?(gsquelart) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 16

2 years ago
mozreview-review
Comment on attachment 8903428 [details]
Bug 1393399 P1 - add telemetry probes;

https://reviewboard.mozilla.org/r/175268/#review180614

This is Category 1 data.

datareview+
Attachment #8903428 - Flags: review?(francois) → review+
Assignee

Comment 17

2 years ago
Thanks for the review!

Comment 18

2 years ago
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 550d713f8508 -d e88120f2f0dc: rebasing 417532:550d713f8508 "Bug 1393399 P1 - add telemetry probes; r=francois,gerald"
merging toolkit/components/telemetry/Histograms.json
warning: conflicts while merging toolkit/components/telemetry/Histograms.json! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 25

2 years ago
Pushed by tkuo@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c88a7744a050
P1 - add telemetry probes; r=francois,gerald
https://hg.mozilla.org/integration/autoland/rev/7de2b91cef4b
P2 - keep the GPU process crash time and send back to MFR; r=gerald
https://hg.mozilla.org/integration/autoland/rev/e3f58e6eeb8f
P3 - keep the MediaDecoderOwner's identification in MFR; r=gerald
https://hg.mozilla.org/integration/autoland/rev/00e03f0e1c1e
P4 - implement GPUProcessCrashTelemetryLogger helper class; r=gerald
https://hg.mozilla.org/integration/autoland/rev/120baee2ca23
P5 - report the recovery time telemetry; r=gerald
Depends on: 1396428
Assignee

Comment 27

2 years ago
https://mzl.la/2xT0nWC
I think there is something wrong that around 45% crash recoveries take more than 64 seconds.
Assignee

Updated

2 years ago
Depends on: 1401902
Depends on: 1408693
You need to log in before you can comment on or make changes to this bug.