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
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index e8034de..9a5d5b2 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -3674,11 +3674,12 @@
return false;
}
-void SurfaceFlinger::flushPendingTransactionQueues(
+int SurfaceFlinger::flushPendingTransactionQueues(
std::vector<TransactionState>& transactions,
- std::unordered_set<sp<IBinder>, SpHash<IBinder>>& bufferLayersReadyToPresent,
+ std::unordered_map<sp<IBinder>, uint64_t, SpHash<IBinder>>& bufferLayersReadyToPresent,
std::unordered_set<sp<IBinder>, SpHash<IBinder>>& applyTokensWithUnsignaledTransactions,
bool tryApplyUnsignaled) {
+ int transactionsPendingBarrier = 0;
auto it = mPendingTransactionQueues.begin();
while (it != mPendingTransactionQueues.end()) {
auto& [applyToken, transactionQueue] = *it;
@@ -3701,8 +3702,21 @@
setTransactionFlags(eTransactionFlushNeeded);
break;
}
+ if (ready == TransactionReadiness::NotReadyBarrier) {
+ transactionsPendingBarrier++;
+ setTransactionFlags(eTransactionFlushNeeded);
+ break;
+ }
transaction.traverseStatesWithBuffers([&](const layer_state_t& state) {
- bufferLayersReadyToPresent.insert(state.surface);
+ const bool frameNumberChanged =
+ state.bufferData->flags.test(BufferData::BufferDataChange::frameNumberChanged);
+ if (frameNumberChanged) {
+ bufferLayersReadyToPresent[state.surface] = state.bufferData->frameNumber;
+ } else {
+ // Barrier function only used for BBQ which always includes a frame number
+ bufferLayersReadyToPresent[state.surface] =
+ std::numeric_limits<uint64_t>::max();
+ }
});
const bool appliedUnsignaled = (ready == TransactionReadiness::ReadyUnsignaled);
if (appliedUnsignaled) {
@@ -3720,6 +3734,7 @@
it = std::next(it, 1);
}
}
+ return transactionsPendingBarrier;
}
bool SurfaceFlinger::flushTransactionQueues(int64_t vsyncId) {
@@ -3728,19 +3743,21 @@
// states) around outside the scope of the lock
std::vector<TransactionState> transactions;
// Layer handles that have transactions with buffers that are ready to be applied.
- std::unordered_set<sp<IBinder>, SpHash<IBinder>> bufferLayersReadyToPresent;
+ std::unordered_map<sp<IBinder>, uint64_t, SpHash<IBinder>> bufferLayersReadyToPresent;
std::unordered_set<sp<IBinder>, SpHash<IBinder>> applyTokensWithUnsignaledTransactions;
{
Mutex::Autolock _l(mStateLock);
{
Mutex::Autolock _l(mQueueLock);
+ int lastTransactionsPendingBarrier = 0;
+ int transactionsPendingBarrier = 0;
// First collect transactions from the pending transaction queues.
// We are not allowing unsignaled buffers here as we want to
// collect all the transactions from applyTokens that are ready first.
- flushPendingTransactionQueues(transactions, bufferLayersReadyToPresent,
- applyTokensWithUnsignaledTransactions,
- /*tryApplyUnsignaled*/ false);
+ transactionsPendingBarrier =
+ flushPendingTransactionQueues(transactions, bufferLayersReadyToPresent,
+ applyTokensWithUnsignaledTransactions, /*tryApplyUnsignaled*/ false);
// Second, collect transactions from the transaction queue.
// Here as well we are not allowing unsignaled buffers for the same
@@ -3765,18 +3782,48 @@
/*tryApplyUnsignaled*/ false);
}();
ATRACE_INT("TransactionReadiness", static_cast<int>(ready));
- if (ready == TransactionReadiness::NotReady) {
+ if (ready != TransactionReadiness::Ready) {
+ if (ready == TransactionReadiness::NotReadyBarrier) {
+ transactionsPendingBarrier++;
+ }
mPendingTransactionQueues[transaction.applyToken].push(std::move(transaction));
} else {
transaction.traverseStatesWithBuffers([&](const layer_state_t& state) {
- bufferLayersReadyToPresent.insert(state.surface);
- });
+ const bool frameNumberChanged =
+ state.bufferData->flags.test(BufferData::BufferDataChange::frameNumberChanged);
+ if (frameNumberChanged) {
+ bufferLayersReadyToPresent[state.surface] = state.bufferData->frameNumber;
+ } else {
+ // Barrier function only used for BBQ which always includes a frame number.
+ // This value only used for barrier logic.
+ bufferLayersReadyToPresent[state.surface] =
+ std::numeric_limits<uint64_t>::max();
+ }
+ });
transactions.emplace_back(std::move(transaction));
}
mTransactionQueue.pop_front();
ATRACE_INT("TransactionQueue", mTransactionQueue.size());
}
+ // Transactions with a buffer pending on a barrier may be on a different applyToken
+ // than the transaction which satisfies our barrier. In fact this is the exact use case
+ // that the primitive is designed for. This means we may first process
+ // the barrier dependent transaction, determine it ineligible to complete
+ // and then satisfy in a later inner iteration of flushPendingTransactionQueues.
+ // The barrier dependent transaction was eligible to be presented in this frame
+ // but we would have prevented it without case. To fix this we continually
+ // loop through flushPendingTransactionQueues until we perform an iteration
+ // where the number of transactionsPendingBarrier doesn't change. This way
+ // we can continue to resolve dependency chains of barriers as far as possible.
+ while (lastTransactionsPendingBarrier != transactionsPendingBarrier) {
+ lastTransactionsPendingBarrier = transactionsPendingBarrier;
+ transactionsPendingBarrier =
+ flushPendingTransactionQueues(transactions, bufferLayersReadyToPresent,
+ applyTokensWithUnsignaledTransactions,
+ /*tryApplyUnsignaled*/ false);
+ }
+
// We collected all transactions that could apply without latching unsignaled buffers.
// If we are allowing latch unsignaled of some form, now it's the time to go over the
// transactions that were not applied and try to apply them unsignaled.
@@ -3892,7 +3939,8 @@
auto SurfaceFlinger::transactionIsReadyToBeApplied(
const FrameTimelineInfo& info, bool isAutoTimestamp, int64_t desiredPresentTime,
uid_t originUid, const Vector<ComposerState>& states,
- const std::unordered_set<sp<IBinder>, SpHash<IBinder>>& bufferLayersReadyToPresent,
+ const std::unordered_map<
+ sp<IBinder>, uint64_t, SpHash<IBinder>>& bufferLayersReadyToPresent,
size_t totalTXapplied, bool tryApplyUnsignaled) const -> TransactionReadiness {
ATRACE_FORMAT("transactionIsReadyToBeApplied vsyncId: %" PRId64, info.vsyncId);
const nsecs_t expectedPresentTime = mExpectedPresentTime.load();
@@ -3930,6 +3978,17 @@
continue;
}
+ if (s.hasBufferChanges() && s.bufferData->hasBarrier &&
+ ((layer->getDrawingState().frameNumber) < s.bufferData->barrierFrameNumber)) {
+ const bool willApplyBarrierFrame =
+ (bufferLayersReadyToPresent.find(s.surface) != bufferLayersReadyToPresent.end()) &&
+ (bufferLayersReadyToPresent.at(s.surface) >= s.bufferData->barrierFrameNumber);
+ if (!willApplyBarrierFrame) {
+ ATRACE_NAME("NotReadyBarrier");
+ return TransactionReadiness::NotReadyBarrier;
+ }
+ }
+
const bool allowLatchUnsignaled = tryApplyUnsignaled &&
shouldLatchUnsignaled(layer, s, states.size(), totalTXapplied);
ATRACE_FORMAT("%s allowLatchUnsignaled=%s", layer->getName().c_str(),
@@ -3950,8 +4009,8 @@
if (s.hasBufferChanges()) {
// If backpressure is enabled and we already have a buffer to commit, keep the
// transaction in the queue.
- const bool hasPendingBuffer =
- bufferLayersReadyToPresent.find(s.surface) != bufferLayersReadyToPresent.end();
+ const bool hasPendingBuffer = bufferLayersReadyToPresent.find(s.surface) !=
+ bufferLayersReadyToPresent.end();
if (layer->backpressureEnabled() && hasPendingBuffer && isAutoTimestamp) {
ATRACE_NAME("hasPendingBuffer");
return TransactionReadiness::NotReady;