Update pending buffer counts immediately

Increment any buffer updates in the binder thread so systrace
has an accurate idea of when the buffers arrived on the server.
The buffer count will continue to be dropped when the buffers
are dropped or latched in the main thread.

Test: capture systrace and check pending buffer counts
Change-Id: Ided1d60197db29cd490691d515443acaab864a48

5 files changed, 66 insertions, 27 deletions

diff --git a/services/surfaceflinger/BufferStateLayer.cpp b/services/surfaceflinger/BufferStateLayer.cpp
index f30e1eb1c3..7d37319a3d 100644
--- a/services/surfaceflinger/BufferStateLayer.cpp
+++ b/services/surfaceflinger/BufferStateLayer.cpp
@@ -871,18 +871,13 @@ bool BufferStateLayer::bufferNeedsFiltering() const {
     return layerSize.width() != bufferWidth || layerSize.height() != bufferHeight;
 }
 
-void BufferStateLayer::incrementPendingBufferCount() {
-    mPendingBufferTransactions++;
-    tracePendingBufferCount();
-}
-
 void BufferStateLayer::decrementPendingBufferCount() {
-    mPendingBufferTransactions--;
-    tracePendingBufferCount();
+    int32_t pendingBuffers = --mPendingBufferTransactions;
+    tracePendingBufferCount(pendingBuffers);
 }
 
-void BufferStateLayer::tracePendingBufferCount() {
-    ATRACE_INT(mBlastTransactionName.c_str(), mPendingBufferTransactions);
+void BufferStateLayer::tracePendingBufferCount(int32_t pendingBuffers) {
+    ATRACE_INT(mBlastTransactionName.c_str(), pendingBuffers);
 }
 
 uint32_t BufferStateLayer::doTransaction(uint32_t flags) {
diff --git a/services/surfaceflinger/BufferStateLayer.h b/services/surfaceflinger/BufferStateLayer.h
index 175a40b80e..6b422ea204 100644
--- a/services/surfaceflinger/BufferStateLayer.h
+++ b/services/surfaceflinger/BufferStateLayer.h
@@ -113,9 +113,10 @@ public:
     uint32_t getEffectiveScalingMode() const override;
 
     // See mPendingBufferTransactions
-    void incrementPendingBufferCount() override;
     void decrementPendingBufferCount();
     uint32_t doTransaction(uint32_t flags) override;
+    std::atomic<int32_t>* getPendingBufferCounter() override { return &mPendingBufferTransactions; }
+    std::string getPendingBufferCounterName() override { return mBlastTransactionName; }
 
 protected:
     void gatherBufferInfo() override;
@@ -127,7 +128,7 @@ private:
     friend class TransactionFrameTracerTest;
     friend class TransactionSurfaceFrameTest;
 
-    inline void tracePendingBufferCount();
+    inline void tracePendingBufferCount(int32_t pendingBuffers);
 
     bool updateFrameEventHistory(const sp<Fence>& acquireFence, nsecs_t postedTime,
                                  nsecs_t requestedPresentTime);
@@ -184,7 +185,7 @@ private:
     //     - If the integer increases, a buffer arrived at the server.
     //     - If the integer decreases in latchBuffer, that buffer was latched
     //     - If the integer decreases in setBuffer or doTransaction, a buffer was dropped
-    uint64_t mPendingBufferTransactions{0};
+    std::atomic<int32_t> mPendingBufferTransactions{0};
 
     // TODO(marissaw): support sticky transform for LEGACY camera mode
 
diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h
index 23758c930c..dd723b33fd 100644
--- a/services/surfaceflinger/Layer.h
+++ b/services/surfaceflinger/Layer.h
@@ -503,8 +503,6 @@ public:
     virtual void useEmptyDamage() {}
     Region getVisibleRegion(const DisplayDevice*) const;
 
-    virtual void incrementPendingBufferCount() {}
-
     /*
      * isOpaque - true if this surface is opaque
      *
@@ -946,6 +944,9 @@ public:
     bool setStretchEffect(const StretchEffect& effect);
     StretchEffect getStretchEffect() const;
 
+    virtual std::atomic<int32_t>* getPendingBufferCounter() { return nullptr; }
+    virtual std::string getPendingBufferCounterName() { return ""; }
+
 protected:
     class SyncPoint {
     public:
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 5364adfa45..a6e8ff9c55 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -3268,7 +3268,6 @@ void SurfaceFlinger::flushTransactionQueues() {
                     if (!transactionIsReadyToBeApplied(transaction.isAutoTimestamp,
                                                        transaction.desiredPresentTime,
                                                        transaction.states,
-                                                       false /* updateTransactionCounters*/,
                                                        pendingBuffers)) {
                         setTransactionFlags(eTransactionFlushNeeded);
                         break;
@@ -3293,13 +3292,9 @@ void SurfaceFlinger::flushTransactionQueues() {
                 const auto& transaction = mTransactionQueue.front();
                 bool pendingTransactions = mPendingTransactionQueues.find(transaction.applyToken) !=
                         mPendingTransactionQueues.end();
-                // Call transactionIsReadyToBeApplied first in case we need to
-                // incrementPendingBufferCount and keep track of pending buffers
-                // if the transaction contains a buffer.
                 if (!transactionIsReadyToBeApplied(transaction.isAutoTimestamp,
                                                    transaction.desiredPresentTime,
                                                    transaction.states,
-                                                   true /* updateTransactionCounters */,
                                                    pendingBuffers) ||
                     pendingTransactions) {
                     mPendingTransactionQueues[transaction.applyToken].push(transaction);
@@ -3331,7 +3326,6 @@ bool SurfaceFlinger::transactionFlushNeeded() {
 
 bool SurfaceFlinger::transactionIsReadyToBeApplied(
         bool isAutoTimestamp, int64_t desiredPresentTime, const Vector<ComposerState>& states,
-        bool updateTransactionCounters,
         std::unordered_set<sp<IBinder>, ISurfaceComposer::SpHash<IBinder>>& pendingBuffers) {
     const nsecs_t expectedPresentTime = mExpectedPresentTime.load();
     bool ready = true;
@@ -3366,10 +3360,6 @@ bool SurfaceFlinger::transactionIsReadyToBeApplied(
             ATRACE_NAME("!isVsyncValidForUid");
             ready = false;
         }
-        if (updateTransactionCounters) {
-            // See BufferStateLayer::mPendingBufferTransactions
-            layer->incrementPendingBufferCount();
-        }
 
         // If backpressure is enabled and we already have a buffer to commit, keep the transaction
         // in the queue.
@@ -3460,6 +3450,13 @@ status_t SurfaceFlinger::setTransactionState(
         const std::vector<ListenerCallbacks>& listenerCallbacks, uint64_t transactionId) {
     ATRACE_CALL();
 
+    // Check for incoming buffer updates and increment the pending buffer count.
+    for (const auto& state : states) {
+        if ((state.state.what & layer_state_t::eAcquireFenceChanged) && (state.state.surface)) {
+            mBufferCountTracker.increment(state.state.surface->localBinder());
+        }
+    }
+
     uint32_t permissions =
             callingThreadHasUnscopedSurfaceFlingerAccess() ? Permission::ACCESS_SURFACE_FLINGER : 0;
     // Avoid checking for rotation permissions if the caller already has ACCESS_SURFACE_FLINGER
@@ -4037,10 +4034,16 @@ status_t SurfaceFlinger::createLayer(const String8& name, const sp<Client>& clie
                                             std::move(metadata), format, handle, gbp, &layer);
 
             break;
-        case ISurfaceComposerClient::eFXSurfaceBufferState:
+        case ISurfaceComposerClient::eFXSurfaceBufferState: {
             result = createBufferStateLayer(client, std::move(uniqueName), w, h, flags,
                                             std::move(metadata), handle, &layer);
-            break;
+            std::atomic<int32_t>* pendingBufferCounter = layer->getPendingBufferCounter();
+            if (pendingBufferCounter) {
+                std::string counterName = layer->getPendingBufferCounterName();
+                mBufferCountTracker.add((*handle)->localBinder(), counterName,
+                                        pendingBufferCounter);
+            }
+        } break;
         case ISurfaceComposerClient::eFXSurfaceEffect:
             // check if buffer size is set for color layer.
             if (w > 0 || h > 0) {
@@ -4207,6 +4210,7 @@ void SurfaceFlinger::onHandleDestroyed(sp<Layer>& layer) {
     auto it = mLayersByLocalBinderToken.begin();
     while (it != mLayersByLocalBinderToken.end()) {
         if (it->second == layer) {
+            mBufferCountTracker.remove(it->first->localBinder());
             it = mLayersByLocalBinderToken.erase(it);
         } else {
             it++;
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 1615ab6d35..131721d1c4 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -416,6 +416,43 @@ private:
         void traverseInReverseZOrder(const LayerVector::Visitor& visitor) const;
     };
 
+    // Keeps track of pending buffers per layer handle in the transaction queue or current/drawing
+    // state before the buffers are latched. The layer owns the atomic counters and decrements the
+    // count in the main thread when dropping or latching a buffer.
+    //
+    // The binder threads increment the same counter when a new transaction containing a buffer is
+    // added to the transaction queue. The map is updated with the layer handle lifecycle updates.
+    // This is done to avoid lock contention with the main thread.
+    class BufferCountTracker {
+    public:
+        void increment(BBinder* layerHandle) {
+            std::lock_guard<std::mutex> lock(mLock);
+            auto it = mCounterByLayerHandle.find(layerHandle);
+            if (it != mCounterByLayerHandle.end()) {
+                auto [name, pendingBuffers] = it->second;
+                int32_t count = ++(*pendingBuffers);
+                ATRACE_INT(name.c_str(), count);
+            } else {
+                ALOGW("Handle not found! %p", layerHandle);
+            }
+        }
+
+        void add(BBinder* layerHandle, const std::string& name, std::atomic<int32_t>* counter) {
+            std::lock_guard<std::mutex> lock(mLock);
+            mCounterByLayerHandle[layerHandle] = std::make_pair(name, counter);
+        }
+
+        void remove(BBinder* layerHandle) {
+            std::lock_guard<std::mutex> lock(mLock);
+            mCounterByLayerHandle.erase(layerHandle);
+        }
+
+    private:
+        std::mutex mLock;
+        std::unordered_map<BBinder*, std::pair<std::string, std::atomic<int32_t>*>>
+                mCounterByLayerHandle GUARDED_BY(mLock);
+    };
+
     struct ActiveModeInfo {
         DisplayModeId modeId;
         Scheduler::ModeEvent event = Scheduler::ModeEvent::None;
@@ -764,7 +801,6 @@ private:
     void commitOffscreenLayers();
     bool transactionIsReadyToBeApplied(
             bool isAutoTimestamp, int64_t desiredPresentTime, const Vector<ComposerState>& states,
-            bool updateTransactionCounters,
             std::unordered_set<sp<IBinder>, ISurfaceComposer::SpHash<IBinder>>& pendingBuffers)
             REQUIRES(mStateLock);
     uint32_t setDisplayStateLocked(const DisplayState& s) REQUIRES(mStateLock);
@@ -1314,6 +1350,8 @@ private:
     int mFrameRateFlexibilityTokenCount = 0;
 
     sp<IBinder> mDebugFrameRateFlexibilityToken;
+
+    BufferCountTracker mBufferCountTracker;
 };
 
 } // namespace android