Mir

[enhancement] Mir protocol design prevents clients from ever being more than double-buffered

Bug #1253868 reported by Daniel van Vugt
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Mir
Fix Released
High
Unassigned

Bug Description

The Mir protocol design prevents clients from ever being able to gain a performance advantage beyond double-buffering. This means clients which do a lot of rendering need to keep up with double-buffering deadlines to meet the compositor's frame rate. And they can never take advantage of the catch-up that triple (or more) buffers should provide to eliminate missed frames.

The problem is:
  rpc next_buffer(SurfaceId) returns (Buffer);

When a client tells the server that SurfaceId is ready for rendering, it must then wait synchronously for a single Buffer in return. Thus even if the surface has more than two buffers, the protocol design forces clients to only be able to handle two at once.

This causes a performance bottleneck where even though the server might have free buffers ready to send to the client, it can't do so until the client calls next_buffer a sufficient number of times.

The solution would be to not make next_buffer return a Buffer. Instead have buffers arrive from the server asynchronously, as soon as they can. Much like the way we deliver MirEvent's.

Related branches

lp:~kdub/mir/submit-buffer
Cemil Azizoglu (community): Approve
PS Jenkins bot (community): Approve (continuous-integration)
Robert Carr (community): Approve
Alan Griffiths: Approve
Diff: 118 lines (+53/-0)
5 files modified
src/protobuf/mir_protobuf.proto (+1/-0)
src/server/frontend/protobuf_message_processor.cpp (+4/-0)
src/server/frontend/session_mediator.cpp (+9/-0)
src/server/frontend/session_mediator.h (+6/-0)
tests/integration-tests/test_exchange_buffer.cpp (+33/-0)
lp:~kdub/mir/server-sent-buffer
PS Jenkins bot (community): Approve (continuous-integration)
Chris Halse Rogers: Approve
Andreas Pokorny (community): Approve
Alexandros Frantzis (community): Approve
Alan Griffiths: Approve
Alberto Aguirre (community): Approve
Diff: 559 lines (+253/-10)
15 files modified
src/client/buffer_stream.cpp (+54/-1)
src/client/buffer_stream.h (+11/-0)
src/client/client_buffer_stream.h (+2/-1)
src/client/rpc/mir_protobuf_rpc_channel.cpp (+8/-0)
src/include/server/mir/frontend/event_sink.h (+3/-0)
src/protobuf/mir_protobuf.proto (+2/-0)
src/server/frontend/event_sender.cpp (+11/-0)
src/server/frontend/event_sender.h (+1/-0)
src/server/scene/global_event_sender.cpp (+4/-0)
src/server/scene/global_event_sender.h (+1/-0)
tests/include/mir_test_doubles/mock_client_buffer_stream.h (+1/-0)
tests/include/mir_test_doubles/mock_event_sink.h (+1/-0)
tests/include/mir_test_doubles/null_event_sink.h (+1/-0)
tests/integration-tests/test_exchange_buffer.cpp (+86/-3)
tests/unit-tests/client/test_client_buffer_stream.cpp (+67/-5)
kevin gunn (kgunn72)
tags: added: enhancement
summary: - Mir protocol design prevents clients from ever being more than double-
- buffered
+ [enhancement] Mir protocol design prevents clients from ever being more
+ than double-buffered
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

I just noticed the new callback/completion stuff in SwitchingBundle prevents clients from holding multiple buffers. We've gone backwards a little on this one.

Revision history for this message
Kevin DuBois (kdub) wrote :

There was some discussion on this desired improvement in the context of some work I embarked upon to implement some android-specific performance optimizations. The thread is still active, so I'll try to summarize the thoughts on the bug in the thread after its settled down.
https://lists.ubuntu.com/archives/mir-devel/2014-July/000773.html

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

Actually I think this bug was possibly resolved by the new(er) callback approach. We now send buffer replies to clients from multiple points asynchronously (ASAP) from within BufferQueue. So clients are no longer always waiting for a whole frame, unless the queue is full.

Changed in mir:
status: Triaged → Fix Released
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Triaged again. Now we're talking about fixing this issue properly in the protocol.

Changed in mir:
status: Fix Released → Triaged
Daniel van Vugt (vanvugt)
Changed in mir:
milestone: none → 0.14.0
assignee: nobody → Kevin DuBois (kdub)
status: Triaged → In Progress
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

Fix committed into lp:mir at revision None, scheduled for release in mir, milestone 0.14.0

Changed in mir:
status: In Progress → Fix Committed
Daniel van Vugt (vanvugt)
Changed in mir:
status: Fix Committed → In Progress
PS Jenkins bot (ps-jenkins)
Changed in mir:
status: In Progress → Fix Committed
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Still in progress. We're not at a stage where the new functionality is finished and clients are using it. I think...

Changed in mir:
status: Fix Committed → In Progress
milestone: 0.14.0 → 0.15.0
Alberto Aguirre (albaguirre)
Changed in mir:
milestone: 0.15.0 → 0.16.0
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

I spent much of last week chasing a bug which it turns out was due to the fact that the server already allows a client to have multiple buffers outstanding simultaneously. Or at least we already support returning buffers to clients who are holding and working on other buffers already.

I think this bug was resolved a while ago. Although we introduced a related regression that stopped things from working optimally, I'm working on that in bug 1480164...

Changed in mir:
milestone: 0.16.0 → none
status: In Progress → Fix Released
assignee: Kevin DuBois (kdub) → nobody
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.
Subscribing...

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.