Code review comment for lp:~alan-griffiths/mir/fix-1535894-alt

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

> but only a small piece of the performance story. Even if this branch is
> working perfectly, you're still re-posting all displays on every frame
> (including the idle one) while any one of them is animating.

Remember, this is in the nested server which has a CompositingFunctor for each output.

The earlier change (that you have valid concerns about) ensures that the CompositingFunctor doesn't run for changes that do not apply to its display buffers compositor.

> So this is really
> a power management fix, but how is it an improvement to performance? You
> haven't changed the rendering load, and haven't reduced the number of posts
> either.

The problem this addresses occurs whenever the "idle" CompositingFunctor runs. First it composites the surfaces visible to its DBC and then checks the scene for unconsumed frames. Updates on the other screen cause it to increment frames_scheduled and composite unnecessarily.

By only checking for unconsumed frames in the surfaces that have been rendered we avoid that extra pass.

> (2) Needs fixing: FIXME comment is still missing.
> (4) I'm not yet confident this branch is working. As stated in (1) I can't yet
> put together a manual test case on desktop to reproduce any issue, and the
> diff below shows no new regression tests for anything getting fixed. So I
> think you need to try and write a regression test for the issue.


« Back to merge proposal