Mir

Code review comment for lp:~mir-team/mir/enable-dynamic-buffer-queue

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

(1) A useful FIXME comment is missing from mc::BufferQueue::min_buffers() -
// FIXME: Sometimes required_buffers > nbuffers (LP: #1317403)

(2) An important test has been deleted:
1061 -TEST_F(BufferQueueTest, bypass_clients_get_more_than_two_buffers)
It needs to stay, and to pass(!). At least bypass needed 3+ buffers at some peak times last I checked. Otherwise it will intermittently deadlock. Although I forget the full reasoning right now.

(3) You have fudged slow_client_framerate_matches_compositor, which is kind of cheating as mentioned in https://code.launchpad.net/~vanvugt/mir/double/+merge/221431/comments/530360
Not sure how safe the fudge is or if it risks regression. I would be more sure if I knew how to reintroduce the bug it protects against :)

review: Needs Fixing

« Back to merge proposal