Merge lp:~vanvugt/mir/always-double-buffers into lp:mir

[Daniel van Vugt]

Always use double buffers instead of triple. (LP: #1240909)

This reduces queue lag by ~50% and resource usage by ~33%. It just
wasn't safe to do till we fixed all the BufferQueue-related bugs as of release 0.11.0.

Tested on desktop with bypass, frame dropping, multi-monitors. The improvement is easy to notice using a mouse.

Tested on phone (vivid mako). The improvement is quite a bit harder to notice. Not just because we've only removed a third of the rendering lag (was 6 now 4 frames), but it's probably much less than a third when you account for the fact that touch screens themselves usually have a significant latency.

All works fine now. Does it work for you?

[Daniel van Vugt (vanvugt)]

This also significantly improves resize latency of course :)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:2331
http://jenkins.qa.ubuntu.com/job/mir-ci/3023/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/1362
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/1361
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/1318
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/1020
        deb: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/1020/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/1318
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/1318/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/4348
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/18229

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/3023/rebuild

review: Approve (continuous-integration)

[Alan Griffiths (alan-griffiths)]

I don't see any problems.

Two thoughts (not about this MP directly:

1. Maybe now is a good time to revisit the lttng reports on rendering?
2. We should try again to find a way to automate some smoke tests for visual issues

review: Approve

[Robert Carr (robertcarr)]

+1 Yay

review: Approve

[Kevin DuBois (kdub)]

> Two thoughts (not about this MP directly:
>
> 1. Maybe now is a good time to revisit the lttng reports on rendering?
> 2. We should try again to find a way to automate some smoke tests for visual
> issues

yes, and yes :)

[Kevin DuBois (kdub)]

okay changing the default by me.

Was it tested with the full stack on the n4, or just with the demo applications? (I'm curious to see if this plays nicely with Qt.) Also, bq/n10 have a bit different fencing then the n4, a sanity check there might be worthwhile

[Kevin DuBois (kdub)]

> Also, bq/n10
> have a bit different fencing then the n4, a sanity check there might be
> worthwhile

Did a sanity check on bq, lgtm!

review: Approve

[Daniel van Vugt (vanvugt)]

I tested full stack (backported to 0.11) and tested Ubuntu Touch proper on mako.

The bad news is I can now see stutters when crossing a screen boundary (multi-monitor with multiple display buffers historically needs triple or else the client starves). Also when entering and leaving bypass the transition is not seamless any more.

The worse news is I somehow made the server crash when testing this (invalid free) but can't reproduce it to debug.

Perhaps the ddouble branch still is the way forward. Either that or we need to examine our compositor/DB code to reduce buffer holding (stutters).

review: Needs Fixing

[Daniel van Vugt (vanvugt)]

Still vacillating lots :/

Part of me thinks while this works (which is a first since we started trying to enable it 10 months ago), we should land it. Not landing it is definitely slower, and risks us breaking double buffering support again without noticing.

Improvements like my other branch to dynamically assess client performance can come later.

review: Abstain

[Daniel van Vugt (vanvugt)]

OK, I've now tested this on arale/vivid with the full stack.

Works well. Only unity8-dash struggles to keep up with full frame rate. Sometimes 30 FPS, sometimes 60 FPS. Still, that's not bad and nothing new.

[Daniel van Vugt (vanvugt)]

Also the System Settings can't seem to scroll above 30 FPS. Still, not terrible and should be fixable.

=== modified file 'src/server/compositor/buffer_stream_factory.cpp'
--- src/server/compositor/buffer_stream_factory.cpp	2015-01-21 07:34:50 +0000
+++ src/server/compositor/buffer_stream_factory.cpp	2015-02-20 08:27:07 +0000
@@ -47,7 +47,6 @@
 std::shared_ptr<mc::BufferStream> mc::BufferStreamFactory::create_buffer_stream(
     mg::BufferProperties const& buffer_properties)
 {
-    // Note: Framedropping requires a minimum 3 buffers
-    auto switching_bundle = std::make_shared<mc::BufferQueue>(3, gralloc, buffer_properties, *policy_factory);
+    auto switching_bundle = std::make_shared<mc::BufferQueue>(2, gralloc, buffer_properties, *policy_factory);
     return std::make_shared<mc::BufferStreamSurfaces>(switching_bundle);
 }