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

(1) Thanks for explaining. So I took some time today adding and analysing some debug logging to try and catch Mir in the act of repeatedly scheduling frames on some display but never consuming them. Unfortunately I can't yet reproduce such a busy wait but that could be just that I'm using desktop, or it could just be bad luck. So I'm assuming there's still a theoretical problem needing fixing.

But now I'm curious: You've clarified you're just fixing a busy wait. Or as I would say it "made Mir sleep properly". That's very good for power management, 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. 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.

(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.

(5) In theory there's a much simpler solution you should consider. That is just make every call to BasicSurface::generate_renderables acquire its compositor buffer immediately, instead of the lazy acquisition it does in SurfaceSnapshot::buffer(). In theory that's a tiny change, it would solve the same problem, and is easier to test.