commit 640b7290a5704a8d26875968c0050e8978b63638
author Patrick Williams <pdwilliams@google.com> Tue Oct 11 19:25:06 2022 +0000
committer Patrick Williams <pdwilliams@google.com> Tue Oct 11 19:25:06 2022 +0000
tree 63cbb877ba9acea58eefabe4f4ae3369ce472449
parent 83876a7381c9d9b375632da308f7a9d6a804c957

SF: Trace buffer cache errors

Bug: 244218818
Test: presubmits
Change-Id: I68e46d1952fe064f6f21958108cfc75e96976b3a

libs/gui/SurfaceComposerClient.cpp

diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp
index 694bc5a..e5ec30a 100644
--- a/libs/gui/SurfaceComposerClient.cpp
+++ b/libs/gui/SurfaceComposerClient.cpp
@@ -25,6 +25,7 @@
 #include <android/gui/ISurfaceComposerClient.h>
 #include <android/gui/IWindowInfosListener.h>
 #include <android/os/IInputConstants.h>
+#include <gui/TraceUtils.h>
 #include <utils/Errors.h>
 #include <utils/Log.h>
 #include <utils/SortedVector.h>
@@ -910,9 +911,14 @@
     uncacheBuffer.token = BufferCache::getInstance().getToken();
     uncacheBuffer.id = cacheId;
 
-    sf->setTransactionState(FrameTimelineInfo{}, {}, {}, ISurfaceComposer::eOneWay,
-                            Transaction::getDefaultApplyToken(), {}, systemTime(), true,
-                            uncacheBuffer, false, {}, generateId());
+    status_t status =
+            sf->setTransactionState(FrameTimelineInfo{}, {}, {}, ISurfaceComposer::eOneWay,
+                                    Transaction::getDefaultApplyToken(), {}, systemTime(), true,
+                                    uncacheBuffer, false, {}, generateId());
+    if (status != NO_ERROR) {
+        ALOGE_AND_TRACE("SurfaceComposerClient::doUncacheBufferTransaction - %s",
+                        strerror(-status));
+    }
 }
 
 void SurfaceComposerClient::Transaction::cacheBuffers() {

libs/gui/include/gui/TraceUtils.h

diff --git a/libs/gui/include/gui/TraceUtils.h b/libs/gui/include/gui/TraceUtils.h
index 4c01683..441b833 100644
--- a/libs/gui/include/gui/TraceUtils.h
+++ b/libs/gui/include/gui/TraceUtils.h
@@ -30,6 +30,12 @@
 #define ATRACE_FORMAT_INSTANT(fmt, ...) \
     (CC_UNLIKELY(ATRACE_ENABLED()) && (TraceUtils::instantFormat(fmt, ##__VA_ARGS__), true))
 
+#define ALOGE_AND_TRACE(fmt, ...)                  \
+    do {                                           \
+        ALOGE(fmt, ##__VA_ARGS__);                 \
+        ATRACE_FORMAT_INSTANT(fmt, ##__VA_ARGS__); \
+    } while (false)
+
 namespace android {
 
 class TraceUtils {

services/surfaceflinger/ClientCache.cpp

diff --git a/services/surfaceflinger/ClientCache.cpp b/services/surfaceflinger/ClientCache.cpp
index cf932a8..b01932e 100644
--- a/services/surfaceflinger/ClientCache.cpp
+++ b/services/surfaceflinger/ClientCache.cpp
@@ -22,6 +22,7 @@
 #include <cinttypes>
 
 #include <android-base/stringprintf.h>
+#include <gui/TraceUtils.h>
 #include <renderengine/impl/ExternalTexture.h>
 
 #include "ClientCache.h"
@@ -36,12 +37,12 @@
                             ClientCacheBuffer** outClientCacheBuffer) {
     auto& [processToken, id] = cacheId;
     if (processToken == nullptr) {
-        ALOGE("failed to get buffer, invalid (nullptr) process token");
+        ALOGE_AND_TRACE("ClientCache::getBuffer - invalid (nullptr) process token");
         return false;
     }
     auto it = mBuffers.find(processToken);
     if (it == mBuffers.end()) {
-        ALOGE("failed to get buffer, invalid process token");
+        ALOGE_AND_TRACE("ClientCache::getBuffer - invalid process token");
         return false;
     }
 
@@ -49,7 +50,7 @@
 
     auto bufItr = processBuffers.find(id);
     if (bufItr == processBuffers.end()) {
-        ALOGV("failed to get buffer, invalid buffer id");
+        ALOGE_AND_TRACE("ClientCache::getBuffer - invalid buffer id");
         return false;
     }
 
@@ -61,12 +62,12 @@
 bool ClientCache::add(const client_cache_t& cacheId, const sp<GraphicBuffer>& buffer) {
     auto& [processToken, id] = cacheId;
     if (processToken == nullptr) {
-        ALOGE("failed to cache buffer: invalid process token");
+        ALOGE_AND_TRACE("ClientCache::add - invalid (nullptr) process token");
         return false;
     }
 
     if (!buffer) {
-        ALOGE("failed to cache buffer: invalid buffer");
+        ALOGE_AND_TRACE("ClientCache::add - invalid (nullptr) buffer");
         return false;
     }
 
@@ -79,7 +80,7 @@
     if (it == mBuffers.end()) {
         token = processToken.promote();
         if (!token) {
-            ALOGE("failed to cache buffer: invalid token");
+            ALOGE_AND_TRACE("ClientCache::add - invalid token");
             return false;
         }
 
@@ -87,7 +88,7 @@
         if (token->localBinder() == nullptr) {
             status_t err = token->linkToDeath(mDeathRecipient);
             if (err != NO_ERROR) {
-                ALOGE("failed to cache buffer: could not link to death");
+                ALOGE_AND_TRACE("ClientCache::add - could not link to death");
                 return false;
             }
         }
@@ -102,7 +103,7 @@
     auto& processBuffers = it->second.second;
 
     if (processBuffers.size() > BUFFER_CACHE_MAX_SIZE) {
-        ALOGE("failed to cache buffer: cache is full");
+        ALOGE_AND_TRACE("ClientCache::add - cache is full");
         return false;
     }