Mir

Merge lp://staging/~raof/mir/dont-tear-down-compositor-if-unnecessary into lp://staging/mir

  1. dont-tear-down-compositor-if-unnecessary
  2. Merge into development-branch
Proposed by Chris Halse Rogers
Status: Merged
Approved by: Daniel van Vugt
Approved revision: no longer in the source branch.
Merged at revision: 3712
Proposed branch: lp://staging/~raof/mir/dont-tear-down-compositor-if-unnecessary
Merge into: lp://staging/mir
Diff against target: 541 lines (+293/-13)
16 files modified
include/platform/mir/graphics/display.h (+18/-0)
include/test/mir/test/doubles/null_display.h (+4/-0)
src/platforms/android/server/display.cpp (+6/-0)
src/platforms/android/server/display.h (+1/-0)
src/platforms/eglstream-kms/server/display.cpp (+6/-0)
src/platforms/eglstream-kms/server/display.h (+2/-0)
src/platforms/mesa/server/kms/display.cpp (+6/-0)
src/platforms/mesa/server/kms/display.h (+1/-0)
src/platforms/mesa/server/x11/graphics/display.cpp (+6/-0)
src/platforms/mesa/server/x11/graphics/display.h (+3/-0)
src/server/graphics/nested/display.cpp (+6/-0)
src/server/graphics/nested/display.h (+3/-0)
src/server/scene/mediating_display_changer.cpp (+37/-4)
tests/include/mir/test/doubles/mock_display.h (+2/-0)
tests/mir_test_framework/stubbed_graphics_platform.cpp (+4/-0)
tests/unit-tests/scene/test_mediating_display_changer.cpp (+188/-9)
To merge this branch: bzr merge lp://staging/~raof/mir/dont-tear-down-compositor-if-unnecessary
Reviewer Review Type Date Requested Status
Mir CI Bot continuous-integration Approve
Cemil Azizoglu (community) Approve
Alan Griffiths Approve
Review via email: mp+305778@code.staging.launchpad.net

This proposal supersedes a proposal from 2016-09-09.

Commit message

Prevent unnecessary reinitialisation of the compositor on display configuration (round 1)

We currently need to tear down and reinitialise the compositor on display changes because (a) we have no way of adding or removing compositors short of a full teardown/rebuild cycle and, worse (b) because the compositor takes a non-owning reference to a DisplayBuffer and display configuration will frequently destroy all existing DisplayBuffers.

Add an interface to the platform's Display to check when a display configuration change will *not* destroy existing DisplayBuffers. If it will not, that avoids problem (b), so check if any new outputs have been enabled. If neither of those are the case we can safely apply the display configuration without reinitialising the compositor.

Follow up work will enable this in the various graphics platforms.

Fixes (part one): https://bugs.launchpad.net/mir/+bug/1556142

To post a comment you must log in.
Revision history for this message
Mir CI Bot (mir-ci-bot) wrote : Posted in a previous version of this proposal

FAILED: Continuous integration, rev:3702
https://mir-jenkins.ubuntu.com/job/mir-ci/1678/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/2103/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/2165
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/2156
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/2156
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/2156
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/2131/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/2131
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/2131/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/2131
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/2131/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/2131
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/2131/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/2131
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/2131/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/1678/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Chris Halse Rogers (raof) wrote : Posted in a previous version of this proposal

08:42:03 clang: error: invalid argument '/build/mir' to -fdebug-prefix-map

Again. ???

Revision history for this message
Mir CI Bot (mir-ci-bot) wrote : Posted in a previous version of this proposal

FAILED: Continuous integration, rev:3702
https://mir-jenkins.ubuntu.com/job/mir-ci/1684/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/2109/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/2171
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/2162
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/2162
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/2162
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/2137/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/2137
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/2137/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/2137
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/2137/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/2137
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/2137/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/2137
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/2137/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/1684/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Mir CI Bot (mir-ci-bot) wrote : Posted in a previous version of this proposal

FAILED: Continuous integration, rev:3704
https://mir-jenkins.ubuntu.com/job/mir-ci/1698/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/2126/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/2188
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/2179
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/2179
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/2179
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/2154
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/2154/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/2154
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/2154/artifact/output/*zip*/output.zip
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/2154/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/2154
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/2154/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/2154
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/2154/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/1698/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Chris Halse Rogers (raof) wrote : Posted in a previous version of this proposal

02:05:53 cc1plus: error: invalid argument ���/build/mir��� to -fdebug-prefix-map
Retriggering

Revision history for this message
Mir CI Bot (mir-ci-bot) wrote : Posted in a previous version of this proposal

PASSED: Continuous integration, rev:3704
https://mir-jenkins.ubuntu.com/job/mir-ci/1699/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/2127
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/2189
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/2180
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/2180
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/2180
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/2155
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/2155/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/2155
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/2155/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/2155
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/2155/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/2155
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/2155/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/2155
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/2155/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/1699/rebuild

review: Approve (continuous-integration)
Revision history for this message
Alan Griffiths (alan-griffiths) wrote : Posted in a previous version of this proposal

+ virtual bool configuration_will_preserve_display_buffers(DisplayConfiguration const& conf) const = 0;

While not an issue in the current usage I worry that this style invites racy code. I would prefer something that's obviously atomic:

 bool apply_configuration_if_it_preserves_display_buffers(DisplayConfiguration const& conf) const = 0;

Revision history for this message
Alan Griffiths (alan-griffiths) wrote : Posted in a previous version of this proposal

> + virtual bool
> configuration_will_preserve_display_buffers(DisplayConfiguration const& conf)
> const = 0;
>
> While not an issue in the current usage I worry that this style invites racy
> code. I would prefer something that's obviously atomic:
>
> bool apply_configuration_if_it_preserves_display_buffers(DisplayConfiguration
> const& conf) const = 0;

I'm not going to block on this, but please consider it before TA.

review: Approve
Revision history for this message
Chris Halse Rogers (raof) wrote : Posted in a previous version of this proposal

Hm.

It's pretty easy to use correctly now - the results of will_preserve_display_buffers() are valid until the next configure() call, we serialize all of those, and they're easy to notice.

But apply_configuration_if_it_preserves_display_buffers() would indeed be even easier to use correctly, and might even be easier to implement in the platforms. Thanks for the review!

Revision history for this message
Chris Halse Rogers (raof) wrote :

Now with atomic test-and-set semantics.

Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

FAILED: Continuous integration, rev:3711
https://mir-jenkins.ubuntu.com/job/mir-ci/1721/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/2151/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/2214
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/2205
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/2205
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/2205
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/2179/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/2179
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/2179/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/2179
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/2179/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/2179
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/2179/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/2179
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/2179/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/1721/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

^^^
Just the usual bug 1523621

Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

Looks good

review: Approve
Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

FAILED: Continuous integration, rev:3711
https://mir-jenkins.ubuntu.com/job/mir-ci/1766/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/2213/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/2276
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/2267
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/2267
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/2267
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/2241/console
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/2241/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/2241
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/2241/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/2241
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/2241/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/2241
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/2241/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/1766/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Cemil Azizoglu (cemil-azizoglu) wrote :

lgtmt

review: Approve
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

^^^
Jenkins failures are just bug 1523621 as usual.

Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

FAILED: Autolanding.
More details in the following jenkins job:
https://mir-jenkins.ubuntu.com/job/mir-autolanding/602/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/2225/console
    None: https://mir-jenkins.ubuntu.com/job/generic-land-mp/642/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/2288
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/2279
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/2279
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/2279
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/2253
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/2253/artifact/output/*zip*/output.zip
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/2253/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/2253
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/2253/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/2253
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/2253/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/2253
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/2253/artifact/output/*zip*/output.zip

review: Needs Fixing (continuous-integration)
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

^^^
Bug 1523621 again

Revision history for this message
Mir CI Bot (mir-ci-bot) :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches