Merge lp:~vanvugt/mir/no-savings into lp:mir

[Daniel van Vugt]

Remove spurious saved_resources vector. The duration of the buffer
acquisition in this case is unimportant, only the frequency of
acquisitions matters.

For a detailed discussion on the topic see also:
https://code.launchpad.net/~vanvugt/mir/save-resources-in-the-renderer/+merge/215197
https://bugs.launchpad.net/mir/+bug/1264934

[Alexandros Frantzis (afrantzis)]

Not holding the buffer for a reasonable amount of time, is like stealing the buffer from the real compositor. When the timings are right (or wrong rather) this results in speed-ups surges (= essentially dropping buffers). This is what I get on my laptop with mir_demo_server_shell and mir_demo_client_egltriangle:

1 FPS
17 FPS
95 FPS
82 FPS
60 FPS
60 FPS
60 FPS
61 FPS
60 FPS
97 FPS
80 FPS
60 FPS
60 FPS
61 FPS
60 FPS
60 FPS
98 FPS
77 FPS
60 FPS
60 FPS
60 FPS
61 FPS
60 FPS
102 FPS
75 FPS
60 FPS

review: Disapprove

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1558
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/1348/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-trusty-i386-build/1632
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-trusty-amd64-build/1630
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-trusty-touch/1205
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/1080
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/1080/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/1085
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/1085/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/1206
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/1206/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/1115
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/6115

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/1348/rebuild

review: Approve (continuous-integration)

[Robert Carr (robertcarr)]

Is this supposed to depend on something ?? (i.e. save-resources in renderer)

review: Needs Information

[Kevin DuBois (kdub)]

I think we need a consistent rule for how a compostior must consume all the buffers given to it. It is difficult to distill a rule because we using (abusing?) the compositor buffer consuming. Its natural to start to use functions in a different way as the code grows, but I think the time has come to unify the guarantees and the ways we are using lock_compositor_buffer() in these different consumer entities.

for instance:
BufferConsumingFunctor will consume all the buffers on every pass
DefaultDisplayBufferCompositor (bypass) will just consume the bypassed buffer
DefaultDisplayBufferCompositor (not bypass) will consume all non-obscured renderables

this makes it difficult to schedule compositions, and makes it difficult to make guarantees to our clients.

[Daniel van Vugt (vanvugt)]

"Stealing buffers" cannot happen the way I designed SwitchingBundle.

Sounds like Alexandros is experiencing a different bug. SwitchingBundle is designed to not consume buffers faster than the fastest unique user_id does. So a client frame rate above 60FPS means one of your compositor threads is running faster than 60 FPS. Either that or it's a new bug in SwitchingBundle.

Can anyone else reproduce > 60FPS? I can't yet...

[Daniel van Vugt (vanvugt)]

Robert: No this branch does not depend on anything. Theoretically it just deletes dead code.

Alexandros: Are you able to write a test for the "surges" you experience?

[Alexandros Frantzis (afrantzis)]

> Alexandros: Are you able to write a test for the "surges" you experience?

I haven't been able write a test for it yet, since I don't understand exactly what's going on.

Here is a trace excerpt from one of my runs: http://paste.ubuntu.com/7266508/

See the points marked with "###".

[Daniel van Vugt (vanvugt)]

I'm fairly confident the bug experienced by Alexandros, while real, is not directly caused by this branch. So Disappoved is not appropriate. But something needs fixing before this can land...

[Daniel van Vugt (vanvugt)]

Found the problem --> bug 1308843

It does pre-date this proposal, although it's become slightly more pronounced with this branch.

So a fix for bug 1308843 is not strictly a prerequisite to this branch, but probably preferable.

[Alexandros Frantzis (afrantzis)]

> Found the problem --> bug 1308843

I think the problem described in that bug is different from what I am experiencing. If I understood correctly, bug 1308843 is about the buffer consumption thread consuming at a rate different from the real compositor.
From irc: "duflu: It's exactly the same as multi-monitor with 59.9Hz vs 60.0Hz. With physical displays we accept the beating, but with the virtual one we shouldn't"

However, with this branch, even if I reduce the vsync rate to 10Hz, I still get periodic speed-ups reaching ~65FPS, which contradict an earlier statement that "SwitchingBundle is designed to not consume buffers faster than the fastest unique user_id does". So perhaps we have two different problem in our hands?

[Daniel van Vugt (vanvugt)]

No problem. We don't have to prove one way or the other. Just wait till the bug is fixed and then see.

[Daniel van Vugt (vanvugt)]

You essentially have two monitors, one real and one fake. So the spurious frame rate I would expect to see with the aliasing/beating would be anything up to 120Hz.

[Daniel van Vugt (vanvugt)]

Wait, that's wrong too. Let me fix the bug first then see.

[lp:~vanvugt/mir/no-savings]

1559. By Daniel van Vugt on 2014-04-22

Merge latest development-branch

1560. By Daniel van Vugt on 2014-04-22

Merge latest development-branch

1561. By Daniel van Vugt on 2014-04-24

Merge latest development-branch

1562. By Daniel van Vugt on 2014-04-28

Merge latest development-branch and fix conflicts

[Daniel van Vugt (vanvugt)]

We now have four branches proposed to solve bug 1308844 (and they all do solve it). So this one may not be the one that lands.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1562
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/1413/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-trusty-i386-build/1722
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-trusty-amd64-build/1720
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-trusty-touch/1292
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/1145
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/1145/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/1150
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/1150/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/1295
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/1295/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/1192
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/6445

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/1413/rebuild

review: Approve (continuous-integration)

[lp:~vanvugt/mir/no-savings]

1563. By Daniel van Vugt on 2014-04-29

Merge latest development-branch

1564. By Daniel van Vugt on 2014-04-29

Merge latest development-branch and fix conflicts

1565. By Daniel van Vugt on 2014-04-29

Fix FTBFS from previous commit

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1565
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/1428/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-trusty-i386-build/1746
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-trusty-amd64-build/1744/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-trusty-touch/1316
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/1160
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/1160/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/1165
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/1165/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/1319
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/1319/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/1215
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/6496

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/1428/rebuild

review: Needs Fixing (continuous-integration)

[lp:~vanvugt/mir/no-savings]

1566. By Daniel van Vugt on 2014-04-30

Merge latest development-branch

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1566
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/1443/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-trusty-i386-build/1764
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-trusty-amd64-build/1762
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-trusty-touch/1334
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/1175
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/1175/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/1180
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/1180/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/1337
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/1337/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/1230
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/6547

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/1443/rebuild

review: Approve (continuous-integration)

[Alexandros Frantzis (afrantzis)]

As a result of merging the 10Hz branch, this change is not relevant any more.

Unmerged revisions

1566. By Daniel van Vugt on 2014-04-30

Merge latest development-branch

1565. By Daniel van Vugt on 2014-04-29

Fix FTBFS from previous commit

1564. By Daniel van Vugt on 2014-04-29

Merge latest development-branch and fix conflicts

1563. By Daniel van Vugt on 2014-04-29

Merge latest development-branch

1562. By Daniel van Vugt on 2014-04-28

Merge latest development-branch and fix conflicts

1561. By Daniel van Vugt on 2014-04-24

Merge latest development-branch

1560. By Daniel van Vugt on 2014-04-22

Merge latest development-branch

1559. By Daniel van Vugt on 2014-04-22

Merge latest development-branch

1558. By Daniel van Vugt on 2014-04-16

Remove spurious saved_resources vector. The duration of the buffer
acquisition in this case is unimportant, only the frequency of
acquisitions matters.

For a detailed discussion on the topic see also:
https://code.launchpad.net/~vanvugt/mir/save-resources-in-the-renderer/+merge/215197
https://bugs.launchpad.net/mir/+bug/1264934

=== modified file 'src/server/compositor/multi_threaded_compositor.cpp'
--- src/server/compositor/multi_threaded_compositor.cpp	2014-04-28 23:19:46 +0000
+++ src/server/compositor/multi_threaded_compositor.cpp	2014-04-30 02:53:24 +0000
@@ -196,14 +196,12 @@
         run_compositing_loop(
             [this]
             {
-                std::vector<std::shared_ptr<mg::Buffer>> saved_resources;
-
                 auto const& renderables = scene->renderable_list_for(this);
 
                 for (auto const& r : renderables)
                 {
                     if (r->buffers_ready_for_compositor() > 0)
-                        saved_resources.push_back(r->buffer());
+                        r->buffer(); // consume, don't hold the result
                 }
 
                 wait_until_next_fake_vsync();