Closed
Bug 1313883
Opened 8 years ago
Closed 8 years ago
Intermittent crashes in nvwgf2umx.dll during media playback tests on Win64
Categories
(Core :: Graphics: Layers, defect)
Tracking
()
RESOLVED
FIXED
mozilla52
Tracking | Status | |
---|---|---|
firefox50 | --- | unaffected |
firefox51 | --- | fixed |
firefox52 | --- | fixed |
People
(Reporter: RyanVM, Assigned: mattwoodrow)
References
Details
(Keywords: crash, intermittent-failure)
Attachments
(1 file)
(deleted),
patch
|
dvander
:
review+
gchang
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
For awhile now, there has been a spate of random crashes in nvwgf2umx.dll during various media playback tests (mostly in the web-platform media-source tests). I was finally able to track them down to the push for bug 1289640 and bug 1284672. https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&filter-searchStr=win%20x64%20w(3)&group_state=expanded&tochange=b407b980351600f6708db179c9bdb3338bf24796&fromchange=577158be08e8c1211ffa1198a21e3c059f98f477 Matt, can you please take a look? They appear to be Win64 only. Affects both opt and debug builds. The below OrangeFactor link has logs for affected runs (may take a few minutes to load). https://brasstacks.mozilla.com/orangefactor/?display=OrangeFactor&includefiltertype=quicksearch&includefilterdetailsexcludeResolved=false&includefilterdetailsexcludeDisabled=false&includefilterdetailsquicksearch=nvwgf2umx.dll&includefilterdetailsnumbugs=0&includefilterdetailsresolvedIds=&excludefiltertype=quicksearch&excludefilterdetailsquicksearch=&excludefilterdetailsnumbugs=0&excludefilterdetailsresolvedIds=&plat=All&test=All&type=All&startday=2016-10-01&tree=all
Flags: needinfo?(matt.woodrow)
Assignee | ||
Comment 1•8 years ago
|
||
This is sad, we made this change (after a recommendation from AMD) to avoid crashes. However, it looks like it actually made things worse on both Intel (bug 1292923) and NVIDIA (this one). I guess we should try reverting this change, and supporting both configurations (single device for all threads, as well as one device per thread) and choose which one to take based on the driver vendor. Does that sound reasonable you Jeff? Bas?
Flags: needinfo?(matt.woodrow)
Flags: needinfo?(jmuizelaar)
Flags: needinfo?(bas)
We discussed having a preference for this in bug 1295075 (Ryan started a patch in that bug with gfx.direct3d11.reuse-decoder-device, but we didn't get far with it.) This would be a way to override the default behaviour, but we could easily have a vendor specific bias to go on top of the preference (vendor default, force on, force off.) We can then run the A/B test and see what can be associated with each of these values.
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → matt.woodrow
Assignee | ||
Comment 3•8 years ago
|
||
I did a try push to confirm that the decoder devices were the problem: https://treeherder.mozilla.org/#/jobs?repo=try&revision=aad588202486 This makes the gfx.direct3d11.reuse-decoder-device a tri-state pref. < 0 is the default, shares devices on AMD, creates new ones elsewhere. 0 disables reuse, always creates new devices. > 0 forces reuse (when possible), always tries to use existing devices.
Attachment #8806569 -
Flags: review?(dvander)
Comment on attachment 8806569 [details] [diff] [review] Only reuse devices on AMD Review of attachment 8806569 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/thebes/DeviceManagerDx.cpp @@ +521,5 @@ > + } > + > + if (reuseDevice) { > + if (mCompositorDevice && mCompositorDeviceSupportsVideo && !mDecoderDevice) { > + mDecoderDevice = mCompositorDevice; I was wondering why this assignment occurs outside of the mDeviceLock - it looks like it's safe because mDecoderDevice is not reset on driver resets. We should probably fix both of those things.
Attachment #8806569 -
Flags: review?(dvander) → review+
Comment 5•8 years ago
|
||
(In reply to Matt Woodrow (:mattwoodrow) from comment #1) > This is sad, we made this change (after a recommendation from AMD) to avoid > crashes. However, it looks like it actually made things worse on both Intel > (bug 1292923) and NVIDIA (this one). > > I guess we should try reverting this change, and supporting both > configurations (single device for all threads, as well as one device per > thread) and choose which one to take based on the driver vendor. > > Does that sound reasonable you Jeff? Bas? It does sound reasonable.
Flags: needinfo?(bas)
Pushed by mwoodrow@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/4e6ed6748067 Allow using multiple decoder devices on non-AMD hardware since they seem to crash less that way. r=dvander
Comment 7•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/4e6ed6748067
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Reporter | ||
Comment 8•8 years ago
|
||
None of the dependent bugs have been hit since this landed. Please request Beta approval on this when you get a chance.
Flags: needinfo?(matt.woodrow)
Comment 9•8 years ago
|
||
Hi Astely, Can you help find someone to create uplift request to Beta51?
Flags: needinfo?(aschen)
Comment 10•8 years ago
|
||
Leave ni? I'd prefer to let Matt work on the uplift request.
Flags: needinfo?(aschen)
Updated•8 years ago
|
Flags: needinfo?(aschen)
Assignee | ||
Comment 11•8 years ago
|
||
Comment on attachment 8806569 [details] [diff] [review] Only reuse devices on AMD Approval Request Comment [Feature/Bug causing the regression]: Bug 1284672. [User impact if declined]: More frequent crashes in automation, likely for real users too. [Is this code covered by automated tests?]: Yes. [Has the fix been verified in Nightly?]: Yes. [Needs manual test from QE? If yes, steps to reproduce]: No, changes in automation crashes verify the fix. [List of other uplifts needed for the feature/fix]: None. [Is the change risky?]: No. [Why is the change risky/not risky?]: Just reverts to our existing behaviour for affected drivers. [String changes made/needed]: None.
Flags: needinfo?(matt.woodrow)
Attachment #8806569 -
Flags: approval-mozilla-beta?
Assignee | ||
Updated•8 years ago
|
Flags: needinfo?(jmuizelaar)
Comment 12•8 years ago
|
||
Comment on attachment 8806569 [details] [diff] [review] Only reuse devices on AMD Reduce the risk of crash. Beta51+. Should be in 51 beta 6.
Attachment #8806569 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Updated•8 years ago
|
Flags: needinfo?(aschen)
Reporter | ||
Comment 13•8 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/88ae43bdada9
Updated•2 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•