1630389 - Enable WaitForVBlank by default on Windows 10

Status: RESOLVED FIXED

(Core :: Graphics, enhancement, P3)

Comment 1

[Bert Peers [:bpeers]]

Attachment: Bug 1630389 - Enable WaitForVBlank by default on Windows 10

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/af41b9549456
Enable WaitForVBlank by default on Windows 10 r=jrmuizel

Comment 3

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/22a6286b594e
Backed out changeset af41b9549456 for causing mass Windows failures.

Comment 4

[Arthur Iakab [arthur_iakab]]

Backed out changeset af41b9549456 (bug 1630389) for causing mass Windows failures.

Backout revision https://hg.mozilla.org/integration/autoland/rev/22a6286b594e931fd5dd18c4ae5d2ff6c5a7f47d

Failed push https://treeherder.mozilla.org/#/jobs?repo=autoland&searchStr=windows%2C10%2Cx64%2Cdebug%2Cmochitests%2Ctest-windows10-64%2Fdebug-mochitest-webgl2-ext-e10s-4%2Cm%28gl2e4%29&revision=af41b95494561bb912a9f0bf31c983116348cabb&selectedJob=297832366

Failures https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&searchStr=windows&tochange=345f6e048c1df601e8d0224aa048ca399a527007&fromchange=ece240a560220215d8758906e46c485ee1eb3905

Failure logs: https://treeherder.mozilla.org/logviewer.html#?job_id=297836316&repo=autoland
https://treeherder.mozilla.org/logviewer.html#?job_id=297828692&repo=autoland
https://treeherder.mozilla.org/logviewer.html#?job_id=297834390&repo=autoland
https://treeherder.mozilla.org/logviewer.html#?job_id=297834307&repo=autoland

Bert can you please take a look?

Comment 5

[Jeff Muizelaar [:jrmuizel]]

It looks like we might be OOMing for some reason.

Comment 6

[Bert Peers [:bpeers]]

Thanks for catching this Arthur.

The failure logs suggest that the crash is in the vsync notifications, not in the VBlank loop itself:

• PVsyncBridgeChild::SendNotifyVsync
• VsyncBridgeChild::NotifyVsync
• PVsyncParent::SendNotify

and in 2 of those the failed allocation is in Pickle::Write(something), the 3rd is (inlined?) IPC::Message::WriteSentinel I think?
At least one of the logs suggests this may be exhausting a pre-allocated pool (like an SBA maybe) rather than a run-away memory leak:
AddressSanitizer: Out of memory. The process has exhausted 8192MB for size class 5120

I'm running a test locally where I've disabled this line:
mWaitVBlankMonitor = primary_monitor;
to force a call to GetOutputFromMonitor in every frame, which is where all the COM Add/Release action is. So far I'm not seeing any runaway memory use after ~15 mins of vsynctester.

Comment 7

[Bert Peers [:bpeers]]

I did notice that when I spam the F5 key on this file:
dom/canvas/test/webgl-conf/checkout/conformance/textures/canvas/tex-2d-rgb-rgb-unsigned_short_5_6_5.html
that I can push one of the processes to consume an extra 100 MB. Which eventually drops down again when I stop spamming, but it takes a bit of time.
It's wild guess but do you think it's possible that increased refresh speed pushes up the highwater mark of this test and it thinks it detects a leak?
Otherwise I don't have a lot of ideas.

Comment 8

[Jeff Muizelaar [:jrmuizel]]

Maybe the refresh speed is not being throttled for some reason? Perhaps we can check if we're waiting for a reasonable amount of time?

Comment 9

[Jeff Muizelaar [:jrmuizel]]

From the Chrome issue: "Deal with monitor power off. WaitForVBlank may return right away without waiting in this case. "

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/613bd6719175
Enable WaitForVBlank by default on Windows 10 r=jrmuizel

Comment 11

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/613bd6719175