Merge lp://staging/~vanvugt/mir/disable-early-release into lp://staging/mir
| Status: | Merged |
|---|---|
| Approved by: | Daniel van Vugt |
| Approved revision: | no longer in the source branch. |
| Merged at revision: | 3441 |
| Proposed branch: | lp://staging/~vanvugt/mir/disable-early-release |
| Merge into: | lp://staging/mir |
| Diff against target: |
277 lines (+83/-13) 10 files modified
src/server/compositor/buffer_bundle.h (+8/-0) src/server/compositor/buffer_queue.cpp (+10/-1) src/server/compositor/buffer_queue.h (+2/-0) src/server/compositor/multi_monitor_arbiter.h (+1/-7) tests/acceptance-tests/test_latency.cpp (+1/-5) tests/include/mir/test/doubles/mock_buffer_bundle.h (+1/-0) tests/integration-tests/test_buffer_scheduling.cpp (+27/-0) tests/integration-tests/test_exchange_buffer.cpp (+1/-0) tests/unit-tests/compositor/test_buffer_queue.cpp (+31/-0) tests/unit-tests/compositor/test_temporary_buffers.cpp (+1/-0) |
| To merge this branch: | bzr merge lp://staging/~vanvugt/mir/disable-early-release |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Cemil Azizoglu (community) | Approve | ||
| Kevin DuBois (community) | Approve | ||
| Mir CI Bot | continuous-integration | Approve | |
|
Review via email:
|
|||
Commit message
Disable the 'early release optimization' in BufferQueue, and add more
tests to verify it's really disabled (and the equivalent in NBS too).
The purpose of 'early release' (AKA single_monitor_fast in NBS) was to
reduce the chance of frame skipping when the number of buffers is reduced.
However now it seems unlikely we will ever reduce the number of buffers
to two.
'early release' has the side-effect of increasing buffer lag by one
frame. And that would have been canceled out by switching to double
buffering, but now it won't be. So in terms of latency we're one frame
better off not using 'early release'.
NBS's equivalent mode 'single_
default (LP: #1561418), so no change required there.
The new 'mirvanity' tool also confirms that this branch measurably reduces
latency by about 17ms (one frame).
That all said, Unity8 was never affected by the 'early release' lag
because (a) its QtMir compositor does not early-release; and (b) the
system compositor generally uses bypass/overlays that prevents the
possibility of early release anyway. But at least now BufferQueue and
NBS have the same default mode, and it is the lowest latency mode.
The branch incidentally also provides a basis for fixing LP: #1561418.
| Daniel van Vugt (vanvugt) wrote | # |
| Kevin DuBois (kdub) wrote | # |
| Mir CI Bot (mir-ci-bot) wrote | # |
| Daniel van Vugt (vanvugt) wrote | # |
| Mir CI Bot (mir-ci-bot) wrote | # |
| Mir CI Bot (mir-ci-bot) wrote | # |
| Daniel van Vugt (vanvugt) wrote | # |
When it was introduced in the nbs code, MultMonitorMode was criticized for not being very descriptive about what the benefits/tradeoffs are. If we don't need the mode anymore, could it just be removed? Since it still remains an internal interface (and is not exposed over protocol or command line options), that's a non-blocking concern.
>The purpose of 'early release' (AKA single_monitor_fast in NBS) was to
>reduce the chance of frame skipping when the number of buffers is reduced.
>However now it seems unlikely we will ever reduce the number of buffers
>to two.
The other thought I had was that we have info about reducing latency, but no info on the effect on throughput. The other thing 'early release' does is return the buffer to the client sooner so it can start to work on it sooner.
If its a good throughput/latency tradeoff, +1, but I guess needs info for now.