[wayland] Wayland vsync implementation has unnecessary threading
Categories
(Core :: Widget: Gtk, defect)
Tracking
()
Tracking | Status | |
---|---|---|
firefox79 | --- | fixed |
People
(Reporter: kennylevinsen, Assigned: kennylevinsen)
References
(Blocks 2 open bugs)
Details
Attachments
(2 files, 1 obsolete file)
In order to fit in with the various existing thread assumptions for vsync sources and dispatchers, the wayland vsync implementation uses a dispatcher thread - one per vsync source for simplicity.
With this in mind, we currently have an event pump flow that looks something like:
- Main thread receives N frame callback completions on the Wayland display connection.
- Main thread dispatches N frame callbacks, with our implementation leading to N threads being woken up.
- N threads notify all the appropriate surface-local vsync observers.
- All vsync observers post tasks to other threads, which must now wake up, a lot of which are back to the main thread (refresh drivers).
All in all, this just makes things much slower and more complicated than needed. It just increases latency and jitter for our vsync unnecessarily. As all the observers do is to post tasks, it would be more sensible to just let the frame callback drive the observers.
Assignee | ||
Comment 1•4 years ago
|
||
Assignee | ||
Comment 2•4 years ago
|
||
Depends on D77044
Updated•4 years ago
|
Updated•4 years ago
|
Updated•4 years ago
|
Assignee | ||
Comment 3•4 years ago
|
||
Depends on D77044
Assignee | ||
Comment 4•4 years ago
|
||
Trampled a little on Phabricator by switching from arcanist to moz-phab, but should be in order now.
Updated•4 years ago
|
Pushed by rmaries@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/4cfcf5a973da nsRefreshDriver vsync observer should always post task to main thread. r=jrmuizel
Comment 6•4 years ago
|
||
bugherder |
Pushed by ncsoregi@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/6b49139f2b9d Fire vsync observers directly from Wayland frame callback. r=stransky
Comment 8•4 years ago
|
||
Backed out changeset 6b49139f2b9d (Bug 1641033) for causing assertions in gfxPlatform.cpp CLOSED TREE
Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=305150159&repo=autoland&lineNumber=1645
Backout: https://hg.mozilla.org/integration/autoland/rev/2af2816872ce55325e6e022e52024c88c0fc0abd
Updated•4 years ago
|
Updated•4 years ago
|
Pushed by btara@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/14ff816a22bd Fire vsync observers directly from Wayland frame callback. r=stransky
Comment 10•4 years ago
|
||
bugherder |
Assignee | ||
Updated•4 years ago
|
Comment 11•4 years ago
•
|
||
Why is the first patch here correct? Don't we now in the child process dispatch another runnable to the main thread when before the patch in child process the vsync was normally processed immediately.
And don't now also child processes use code which is named like "ParentProcessVsyncNotifier", or am I missing something obvious?
Assignee | ||
Comment 12•4 years ago
|
||
You might be right - I don't quite remember 3 months later. I do remember checking all the real vsync drivers though.
In that case, I think we should add a guard if we're a child process rather than if we're the main thread.
Description
•