+5
−0
Loading
BLASTBufferQueue/SF: apply transactions with one-way binder
This CL has three main components: 1. Expose a flag through Transaction::apply that results in one-way calls to setTransactionState 2. Use this flag in all transactions from BBQ which use its own apply token. 3. Implement and use a new transaction barrier system to preserve ordering when switching apply tokens (e.g. BLASTSync) We can see that this CL is safe by making a few assumptions and then arguing that transaction ordering is preserved. We assume: 1. All transactions on the BBQ apply token will remain in order, since they are one-way calls from a single process to a single interface on another. AND 2. The ordering of transactions applied on seperate apply tokens is undefined 3. When preparing transactions for sync, BBQ will set a commit callback, and wait on it before applying another frame of it's own. But when preparing transactions to apply directly, it will not set a callback, and can not (for performance reasons) 4. The ordering of consecutive transactions in a sync is the responsibility of the sync consumer, e.g. SysUI or WM, who will be using their own apply token. Now imagine there were two transactions for frames N and N+1 which were applied in order before this CL, but out of order after. They can't both be applied by BBQ, by assumption one. They can't both be sync transactions (by assumption 4). It can't be a sync transaction applied by a bbq transaction, because we will wait on the callback, and we didn't modify the sync transaction to be applied one-way anyway. So our hypothetically disordered frame must be frame N (applied by BBQ) and frame N+1 (sync). When we analyze this case, we can see that we actually have a bug in the existing code. By assumption 2 and 4, frame N and N+1 will be applied on different apply tokens and so their ordering is undefined. We can solve the existing issue and enable one-way transactions at the same time. When BBQ prepares a transaction for sync following a transaction that has been directly applied (e.g. our aforementioned potentially undefined N,N+1 case) it uses a new API (setBufferHasBarrier) to set a barrier on the previous frame number. SurfaceFlinger interprets this barrier by forcing the barrier dependent transaction to remain in the transaction queue until a buffer with frameNumber >= the barrier frame has arrived. We chose >= because by assumption 4, the sync consumer may choose (probably incorrectly) to apply transactions out of order and we wouldn't want this to deadlock the transaction queue. The implementation itself is relatively well commented in SurfaceFlinger.cpp and so for details refer there. We see that use of this barrier system ensures our frame sequence was in order, since we tried every combination of (Sync, NotSync) and every frame is either sync or not sync, we can see that there are no frames which are not in order. We can apply similar analysis to slowly make setTransactionState async everywhere but we will need to eliminate the several "sync transaction" usages that remain (e.g. syncInputTransactions, etc). Most of these can be replaced with commit callbacks, but there is a lot to go through. Bug: 220929926 Test: Existing tests pass Change-Id: I3fd622966a9d12e4a197cf8560040f492dff996c