Mir

Merge lp://staging/~vanvugt/mir/schedule-nonblocking into lp://staging/mir

  1. schedule-nonblocking
  2. Merge into development-branch
Proposed by Daniel van Vugt
Status: Merged
Approved by: Daniel van Vugt
Approved revision: no longer in the source branch.
Merged at revision: 4072
Proposed branch: lp://staging/~vanvugt/mir/schedule-nonblocking
Merge into: lp://staging/mir
Diff against target: 254 lines (+110/-3)
11 files modified
src/server/compositor/dropping_schedule.cpp (+17/-1)
src/server/compositor/dropping_schedule.h (+2/-0)
src/server/compositor/queueing_schedule.cpp (+7/-0)
src/server/compositor/queueing_schedule.h (+2/-0)
src/server/compositor/schedule.h (+3/-0)
src/server/compositor/stream.cpp (+11/-1)
src/server/compositor/stream.h (+1/-1)
tests/unit-tests/compositor/test_dropping_schedule.cpp (+25/-0)
tests/unit-tests/compositor/test_multi_monitor_arbiter.cpp (+6/-0)
tests/unit-tests/compositor/test_queueing_schedule.cpp (+19/-0)
tests/unit-tests/compositor/test_stream.cpp (+17/-0)
To merge this branch: bzr merge lp://staging/~vanvugt/mir/schedule-nonblocking
Reviewer Review Type Date Requested Status
Alan Griffiths Approve
Mir CI Bot continuous-integration Approve
Review via email: mp+318879@code.staging.launchpad.net

Commit message

Improve concurrency of the IPC and compositor threads.

The compositor thread is presently blocked waiting for the IPC thread to
complete socket IO, which is bad. This reorders operations to avoid
that delay so the compositor thread is not hostage to IPC performance.

This is the first step on the road to fixing GPU saturation issues
(LP: #1211700, LP: #1665802) but is not yet a fix in itself.

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

PASSED: Continuous integration, rev:4078
https://mir-jenkins.ubuntu.com/job/mir-ci/3098/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/4156
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/4243
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/4233
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/4233
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=zesty/4233
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4183
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4183/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/4183
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/4183/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4183
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4183/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/4183
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/4183/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/4183
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/4183/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/4183
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/4183/artifact/output/*zip*/output.zip

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

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

PASSED: Continuous integration, rev:4079
https://mir-jenkins.ubuntu.com/job/mir-ci/3099/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/4157
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/4244
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/4234
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/4234
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=zesty/4234
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4184
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4184/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/4184
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/4184/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4184
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4184/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/4184
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/4184/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/4184
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/4184/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/4184
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/4184/artifact/output/*zip*/output.zip

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

review: Approve (continuous-integration)
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

I remember there's something in this area that doesn't work as I expect: This codepath ought to be a corner case that occurs rarely, if ever. But it happens a lot.

We shouldn't be doing IPC on the composite thread at all: re-ordering the operations is just moving the deckchairs.

I haven't tried, but I don't see why this is easier than queuing the IPC to happen on the "Mir/IPC" thread.

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

The IPC _is_ happening on the IPC thread, but that is still indirectly blocking the compositor thread due to poor mutex management...

We just have a dumb ordering of operations right now:
.. 1. Received new buffer
.. 2. Lock the Stream lock (which hangs the compositor if it is trying to redraw).
.. 3. Queue the new buffer for compositing.
** 4. Send dropped buffer back to the client (blocking socket IO).
.. 5. Unlock the Stream lock so the compositor won't hang.
.. 6. Notify the compositor that the scene has changed and that it should start redrawing.

This branch changes the order of operations:
.. 1. Received new buffer
.. 2. Lock the Stream lock (which hangs the compositor if it is trying to redraw).
.. 3. Queue the new buffer for compositing.
.. 4. Unlock the Stream lock so the compositor won't hang.
.. 5. Notify the compositor that the scene has changed and that it should start redrawing.
** 6. Send dropped buffer back to the client (blocking socket IO).

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

Thanks for the explanation. Makes sense.

review: Approve

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