summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRobert Carr <racarr@google.com>2020-06-24 15:26:32 -0700
committerRobert Carr <racarr@google.com>2020-06-24 15:26:32 -0700
commit6417d2e504e6d5ac86b753b3ab9d05a5c20fb838 (patch)
tree3fd8b3d352977c5ecacfb965235ee0a72f54eb2c
parent109ad7156997f031717183664ac51a03110bc2b3 (diff)
downloadnative-6417d2e504e6d5ac86b753b3ab9d05a5c20fb838.tar.gz
SurfaceFlinger: Force runHandleTransaction
In the linked bug, there are traces documenting cases where the user observes jank while deferred transactions are in play. We can see several cases where the client gets stuck for a long time on dequeueBuffer. Around these times we can also see cases where latchBuffer aborts early due to the !allTransactionsSignaled condition. We expect all the transactions to be signalled at this point, because before handlePageFlip we expect to reach handleTransactionLocked, which should call notifyAvailableFrames, marking the sync points available based on the head frame number. However if we study the trace we also see there is an onMessageInvalidate, which contains a handlePageFlip but does NOT contain the handleTransaction before it which we would expect. However we know if onMessageInvalidate calls handlePageFlip (via handleMessageInvalidate) then it must have called handleMessageTransaction, as these two lines are right next to eachother. We can thus conclude that we see the problem because handleMessageTransaction didn't call handleTransaction due to runHandleTransaction evaluating to false. This makes sense in the context of recent changes. Previously deferred transactions would set the transaction flags continuously while they were pending, so the server would always perform transactions when waking up, and we wouldn't get in this condition. However, setting the transaction flags always triggers a wake up! But we don't actually want to wake up until the frame actually arrives, so first we tried setting the transaction flags without triggering a wake up. But we found this could prevent other people from triggering wakeups, so a recent CL modified it to set mForceTraversal instead. Intending to force that we call doTransaction from handleMessageInvalidate regardless of whether a new transaction has arrived (because we need to check if an old transaction is now ready). However, it didn't quite work because of this early abort due to runHandleTransaction. Bug: 159677043 Test: Existing tests pass Change-Id: I84f2a9cca2ebb3f59d30924efdb39c917f47111b
-rw-r--r--services/surfaceflinger/SurfaceFlinger.cpp3
1 files changed, 2 insertions, 1 deletions
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 2e019039f8..07690cbf32 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -2023,7 +2023,8 @@ bool SurfaceFlinger::handleMessageTransaction() {
bool runHandleTransaction =
(transactionFlags && (transactionFlags != eTransactionFlushNeeded)) ||
- flushedATransaction;
+ flushedATransaction ||
+ mForceTraversal;
if (runHandleTransaction) {
handleTransaction(eTransactionMask);