Closed
Bug 1135408
Opened 10 years ago
Closed 10 years ago
Add telemetry reporting for device resets
Categories
(Core :: Graphics, defect)
Tracking
()
RESOLVED
FIXED
mozilla38
People
(Reporter: bas.schouten, Assigned: bas.schouten)
References
(Depends on 1 open bug)
Details
Attachments
(1 file)
6.19 KB,
patch
|
vladan
:
review+
lmandel
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
We should record device removals occurring out in the wild so we can see when we're impacting them in a negative way, this will be particularly important when we no longer crash on them.
Attachment #8567557 -
Flags: review?(vdjeric)
Comment 1•10 years ago
|
||
Comment on attachment 8567557 [details] [diff] [review]
Add telemetry reporting for device resets
Review of attachment 8567557 [details] [diff] [review]:
-----------------------------------------------------------------
::: gfx/thebes/gfxWindowsPlatform.cpp
@@ +476,5 @@
> */
> bool didReset = false;
> + DeviceResetReason resetReason = DeviceResetReason::OK;
> + if (DidRenderingDeviceReset(&resetReason)) {
> + Telemetry::Accumulate(Telemetry::DEVICE_RESET_REASON, uint32_t(resetReason));
you're never going to report DeviceResetReason::OK, is that intended?
@@ +1175,5 @@
> return true;
> }
> }
> if (mD3D11ContentDevice) {
> if (mD3D11ContentDevice->GetDeviceRemovedReason() != S_OK) {
You don't want to report the other device types resetting?
::: toolkit/components/telemetry/Histograms.json
@@ +212,5 @@
> },
> + "DEVICE_RESET_REASON": {
> + "expires_in_version": "never",
> + "kind": "enumerated",
> + "n_values": 5,
- There are 6 enum values though
- Also, you should round up n_values to 10 or more to leave room for future values. It's not really possible to change the definition of a histogram after data has been collected
Attachment #8567557 -
Flags: review?(vdjeric)
Comment 2•10 years ago
|
||
Comment on attachment 8567557 [details] [diff] [review]
Add telemetry reporting for device resets
Review of attachment 8567557 [details] [diff] [review]:
-----------------------------------------------------------------
After talking to Bas in person, r+ if you change n_values to 10
Attachment #8567557 -
Flags: review+
Comment 3•10 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
status-firefox38:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
Assignee | ||
Comment 4•10 years ago
|
||
Comment on attachment 8567557 [details] [diff] [review]
Add telemetry reporting for device resets
Approval Request Comment
[Feature/regressing bug #]: None
[User impact if declined]: Unable to tell TDRs in the wild
[Describe test coverage new/current, TreeHerder]: Limited
[Risks and why]: Very low, only telemetry reporting
[String/UUID change made/needed]: None
Attachment #8567557 -
Flags: approval-mozilla-aurora?
Updated•10 years ago
|
status-firefox37:
--- → affected
Comment 5•10 years ago
|
||
Comment on attachment 8567557 [details] [diff] [review]
Add telemetry reporting for device resets
Although this change has only been on m-c for a few hours, I spoke with Vladan and he said this is very low risk. We also really need to get more graphics data to get on top of some of the issues that we're seeing. 37 is now Beta so I'm approving for Beta 1.
Beta+
Attachment #8567557 -
Flags: approval-mozilla-aurora? → approval-mozilla-beta+
Comment 6•10 years ago
|
||
You need to log in
before you can comment on or make changes to this bug.
Description
•