7 files changed, 65 insertions, 362 deletions

diff --git a/core/jni/android_view_ThreadedRenderer.cpp b/core/jni/android_view_ThreadedRenderer.cpp
index ac7700786ea0..cd2c0d6643a1 100644
--- a/core/jni/android_view_ThreadedRenderer.cpp
+++ b/core/jni/android_view_ThreadedRenderer.cpp
@@ -17,6 +17,7 @@
 #define LOG_TAG "ThreadedRenderer"
 
 #include <algorithm>
+#include <atomic>
 
 #include "jni.h"
 #include <nativehelper/JNIHelp.h>
@@ -231,28 +232,13 @@ class ObserverProxy;
 
 class NotifyHandler : public MessageHandler {
 public:
-    NotifyHandler(JavaVM* vm) : mVm(vm) {}
-
-    void setObserver(ObserverProxy* observer) {
-        mObserver = observer;
-    }
-
-    void setBuffer(BufferPool::Buffer* buffer) {
-        mBuffer = buffer;
-    }
-
-    void setDropCount(int dropCount) {
-        mDropCount = dropCount;
-    }
+    NotifyHandler(JavaVM* vm, ObserverProxy* observer) : mVm(vm), mObserver(observer) {}
 
     virtual void handleMessage(const Message& message);
 
 private:
-    JavaVM* mVm;
-
-    sp<ObserverProxy> mObserver;
-    BufferPool::Buffer* mBuffer = nullptr;
-    int mDropCount = 0;
+    JavaVM* const mVm;
+    ObserverProxy* const mObserver;
 };
 
 static jlongArray get_metrics_buffer(JNIEnv* env, jobject observer) {
@@ -265,6 +251,9 @@ static jlongArray get_metrics_buffer(JNIEnv* env, jobject observer) {
     return reinterpret_cast<jlongArray>(buffer);
 }
 
+/*
+ * Implements JNI layer for hwui frame metrics reporting.
+ */
 class ObserverProxy : public FrameMetricsObserver {
 public:
     ObserverProxy(JavaVM *vm, jobject observer) : mVm(vm) {
@@ -284,7 +273,7 @@ public:
         mMessageQueue = android_os_MessageQueue_getMessageQueue(env, messageQueueLocal);
         LOG_ALWAYS_FATAL_IF(mMessageQueue == nullptr, "message queue not available");
 
-        mMessageHandler = new NotifyHandler(mVm);
+        mMessageHandler = new NotifyHandler(mVm, this);
         LOG_ALWAYS_FATAL_IF(mMessageHandler == nullptr,
                 "OOM: unable to allocate NotifyHandler");
     }
@@ -298,18 +287,53 @@ public:
         return mObserverWeak;
     }
 
-    virtual void notify(BufferPool::Buffer* buffer, int dropCount) {
-        buffer->incRef();
-        mMessageHandler->setBuffer(buffer);
-        mMessageHandler->setObserver(this);
-        mMessageHandler->setDropCount(dropCount);
-        mMessageQueue->getLooper()->sendMessage(mMessageHandler, mMessage);
+    bool getNextBuffer(JNIEnv* env, jlongArray sink, int* dropCount) {
+        FrameMetricsNotification& elem = mRingBuffer[mNextInQueue];
+
+        if (elem.hasData.load()) {
+            env->SetLongArrayRegion(sink, 0, kBufferSize, elem.buffer);
+            *dropCount = elem.dropCount;
+            mNextInQueue = (mNextInQueue + 1) % kRingSize;
+            elem.hasData = false;
+            return true;
+        }
+
+        return false;
+    }
+
+    virtual void notify(const int64_t* stats) {
+        FrameMetricsNotification& elem = mRingBuffer[mNextFree];
+
+        if (!elem.hasData.load()) {
+            memcpy(elem.buffer, stats, kBufferSize * sizeof(stats[0]));
+
+            elem.dropCount = mDroppedReports;
+            mDroppedReports = 0;
+
+            incStrong(nullptr);
+            mNextFree = (mNextFree + 1) % kRingSize;
+            elem.hasData = true;
+
+            mMessageQueue->getLooper()->sendMessage(mMessageHandler, mMessage);
+        } else {
+            mDroppedReports++;
+        }
     }
 
 private:
     static const int kBufferSize = static_cast<int>(FrameInfoIndex::NumIndexes);
+    static constexpr int kRingSize = 3;
 
-    JavaVM* mVm;
+    class FrameMetricsNotification {
+    public:
+        FrameMetricsNotification() : hasData(false) {}
+
+        std::atomic_bool hasData;
+        int64_t buffer[kBufferSize];
+        int dropCount = 0;
+    };
+
+    JavaVM* const mVm;
     jweak mObserverWeak;
     jobject mJavaBufferGlobal;
 
@@ -317,28 +341,28 @@ private:
     sp<NotifyHandler> mMessageHandler;
     Message mMessage;
 
+    int mNextFree = 0;
+    int mNextInQueue = 0;
+    FrameMetricsNotification mRingBuffer[kRingSize];
+
+    int mDroppedReports = 0;
 };
 
 void NotifyHandler::handleMessage(const Message& message) {
     JNIEnv* env = getenv(mVm);
 
-    ObserverProxy* observer = mObserver.get();
-    LOG_ALWAYS_FATAL_IF(observer == nullptr, "received message with no observer configured");
-    LOG_ALWAYS_FATAL_IF(mBuffer == nullptr, "received message with no data to report");
-
-    jobject target = env->NewLocalRef(observer->getObserverReference());
+    jobject target = env->NewLocalRef(mObserver->getObserverReference());
 
     if (target != nullptr) {
         jlongArray javaBuffer = get_metrics_buffer(env, target);
-        env->SetLongArrayRegion(javaBuffer,
-                0, mBuffer->getSize(), mBuffer->getBuffer());
-        env->CallVoidMethod(target, gFrameMetricsObserverClassInfo.callback,
-                mDropCount);
+        int dropCount = 0;
+        while (mObserver->getNextBuffer(env, javaBuffer, &dropCount)) {
+            env->CallVoidMethod(target, gFrameMetricsObserverClassInfo.callback, dropCount);
+        }
         env->DeleteLocalRef(target);
     }
 
-    mBuffer->release();
-    mObserver.clear();
+    mObserver->decStrong(nullptr);
 }
 
 static void android_view_ThreadedRenderer_setAtlas(JNIEnv* env, jobject clazz,
diff --git a/libs/hwui/Android.mk b/libs/hwui/Android.mk
index 6a565033093c..625712292234 100644
--- a/libs/hwui/Android.mk
+++ b/libs/hwui/Android.mk
@@ -232,7 +232,6 @@ LOCAL_CFLAGS := \
 
 LOCAL_SRC_FILES += \
     $(hwui_test_common_src_files) \
-    tests/unit/BufferPoolTests.cpp \
     tests/unit/CanvasStateTests.cpp \
     tests/unit/ClipAreaTests.cpp \
     tests/unit/CrashHandlerInjector.cpp \
diff --git a/libs/hwui/BufferPool.h b/libs/hwui/BufferPool.h
deleted file mode 100644
index 005b399f603b..000000000000
--- a/libs/hwui/BufferPool.h
+++ /dev/null
@@ -1,181 +0,0 @@
-/*
- * Copyright (C) 2016 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-#pragma once
-
-#include "utils/RefBase.h"
-#include "utils/Log.h"
-#include "utils/Macros.h"
-
-#include <atomic>
-#include <stdint.h>
-#include <memory>
-#include <mutex>
-
-namespace android {
-namespace uirenderer {
-
-/*
- * Simple thread-safe pool of int64_t arrays of a provided size.
- *
- * Permits allocating a client-provided max number of buffers.
- * If all buffers are in use, refuses to service any more
- * acquire requests until buffers are re-released to the pool.
- */
-class BufferPool : public VirtualLightRefBase {
-public:
-    class Buffer {
-        PREVENT_COPY_AND_ASSIGN(Buffer);
-    public:
-        int64_t* getBuffer() { return mBuffer.get(); }
-        size_t getSize() { return mSize; }
-
-        void release() {
-            LOG_ALWAYS_FATAL_IF(mPool.get() == nullptr, "attempt to release unacquired buffer");
-            mPool->release(this);
-        }
-
-        Buffer* incRef() {
-            mRefs++;
-            return this;
-        }
-
-        int decRef() {
-            int refs = mRefs.fetch_sub(1);
-            LOG_ALWAYS_FATAL_IF(refs == 0, "buffer reference decremented below 0");
-            return refs - 1;
-        }
-
-        bool isUniqueRef() {
-            return mRefs.load() == 1;
-        }
-
-    private:
-        friend class BufferPool;
-
-        Buffer(BufferPool* pool, size_t size) : mRefs(1) {
-            mSize = size;
-            mBuffer.reset(new int64_t[size]);
-            mPool = pool;
-        }
-
-        void setPool(BufferPool* pool) {
-            mPool = pool;
-        }
-
-        std::unique_ptr<Buffer> mNext;
-        std::unique_ptr<int64_t[]> mBuffer;
-        sp<BufferPool> mPool;
-        size_t mSize;
-
-        std::atomic_int mRefs;
-    };
-
-    BufferPool(size_t bufferSize, size_t count)
-            : mBufferSize(bufferSize), mCount(count) {}
-
-    /**
-     * Acquires a buffer from the buffer pool if available.
-     *
-     * Only `mCount` buffers are allowed to be in use at a single
-     * instance.
-     *
-     * If no buffer is available, i.e. `mCount` buffers are in use,
-     * returns nullptr.
-     *
-     * The pointer returned from this method *MUST NOT* be freed, instead
-     * BufferPool::release() must be called upon it when the client
-     * is done with it. Failing to release buffers will eventually make the
-     * BufferPool refuse to service any more BufferPool::acquire() requests.
-     */
-    BufferPool::Buffer* acquire() {
-        std::lock_guard<std::mutex> lock(mLock);
-
-        if (mHead.get() != nullptr) {
-            BufferPool::Buffer* res = mHead.release();
-            mHead = std::move(res->mNext);
-            res->mNext.reset(nullptr);
-            res->setPool(this);
-            res->incRef();
-            return res;
-        }
-
-        if (mAllocatedCount < mCount) {
-            ++mAllocatedCount;
-            return new BufferPool::Buffer(this, mBufferSize);
-        }
-
-        return nullptr;
-    }
-
-    /**
-     * Releases a buffer previously acquired by BufferPool::acquire().
-     *
-     * The released buffer is not valid after calling this method and
-     * attempting to use will result in undefined behavior.
-     */
-    void release(BufferPool::Buffer* buffer) {
-        std::lock_guard<std::mutex> lock(mLock);
-
-        if (buffer->decRef() != 0) {
-            return;
-        }
-
-        buffer->setPool(nullptr);
-
-        BufferPool::Buffer* list = mHead.get();
-        if (list == nullptr) {
-            mHead.reset(buffer);
-            mHead->mNext.reset(nullptr);
-            return;
-        }
-
-        while (list->mNext.get() != nullptr) {
-            list = list->mNext.get();
-        }
-
-        list->mNext.reset(buffer);
-    }
-
-    /*
-     * Used for testing.
-     */
-    size_t getAvailableBufferCount() {
-        size_t remainingToAllocateCount = mCount - mAllocatedCount;
-
-        BufferPool::Buffer* list = mHead.get();
-        if (list == nullptr) return remainingToAllocateCount;
-
-        int count = 1;
-        while (list->mNext.get() != nullptr) {
-            count++;
-            list = list->mNext.get();
-        }
-
-        return count + remainingToAllocateCount;
-    }
-
-private:
-    mutable std::mutex mLock;
-
-    size_t mBufferSize;
-    size_t mCount;
-    size_t mAllocatedCount = 0;
-    std::unique_ptr<BufferPool::Buffer> mHead;
-};
-
-}; // namespace uirenderer
-}; // namespace android
diff --git a/libs/hwui/FrameMetricsObserver.h b/libs/hwui/FrameMetricsObserver.h
index 2b42a80aca18..4f81c8681fc8 100644
--- a/libs/hwui/FrameMetricsObserver.h
+++ b/libs/hwui/FrameMetricsObserver.h
@@ -18,14 +18,12 @@
 
 #include <utils/RefBase.h>
 
-#include "BufferPool.h"
-
 namespace android {
 namespace uirenderer {
 
 class FrameMetricsObserver : public VirtualLightRefBase {
 public:
-    virtual void notify(BufferPool::Buffer* buffer, int dropCount);
+    virtual void notify(const int64_t* buffer);
 };
 
 }; // namespace uirenderer
diff --git a/libs/hwui/FrameMetricsReporter.h b/libs/hwui/FrameMetricsReporter.h
index 0831d24ccbd3..c1cd0a9224b6 100644
--- a/libs/hwui/FrameMetricsReporter.h
+++ b/libs/hwui/FrameMetricsReporter.h
@@ -19,7 +19,6 @@
 #include <utils/RefBase.h>
 #include <utils/Log.h>
 
-#include "BufferPool.h"
 #include "FrameInfo.h"
 #include "FrameMetricsObserver.h"
 
@@ -31,10 +30,7 @@ namespace uirenderer {
 
 class FrameMetricsReporter {
 public:
-    FrameMetricsReporter() {
-        mBufferPool = new BufferPool(kBufferSize, kBufferCount);
-        LOG_ALWAYS_FATAL_IF(mBufferPool.get() == nullptr, "OOM: unable to allocate buffer pool");
-    }
+    FrameMetricsReporter() {}
 
     void addObserver(FrameMetricsObserver* observer) {
         mObservers.push_back(observer);
@@ -55,36 +51,13 @@ public:
     }
 
     void reportFrameMetrics(const int64_t* stats) {
-        BufferPool::Buffer* statsBuffer = mBufferPool->acquire();
-
-        if (statsBuffer != nullptr) {
-            // copy in frame stats
-            memcpy(statsBuffer->getBuffer(), stats, kBufferSize * sizeof(*stats));
-
-            // notify on requested threads
-            for (size_t i = 0; i < mObservers.size(); i++) {
-                mObservers[i]->notify(statsBuffer, mDroppedReports);
-            }
-
-            // drop our reference
-            statsBuffer->release();
-            mDroppedReports = 0;
-        } else {
-            mDroppedReports++;
+        for (size_t i = 0; i < mObservers.size(); i++) {
+            mObservers[i]->notify(stats);
         }
     }
 
-    int getDroppedReports() { return mDroppedReports; }
-
 private:
-    static const size_t kBufferCount = 3;
-    static const size_t kBufferSize = static_cast<size_t>(FrameInfoIndex::NumIndexes);
-
     std::vector< sp<FrameMetricsObserver> > mObservers;
-
-    sp<BufferPool> mBufferPool;
-
-    int mDroppedReports = 0;
 };
 
 }; // namespace uirenderer
diff --git a/libs/hwui/renderthread/CanvasContext.h b/libs/hwui/renderthread/CanvasContext.h
index 1f819705dc3a..cb61e51a2158 100644
--- a/libs/hwui/renderthread/CanvasContext.h
+++ b/libs/hwui/renderthread/CanvasContext.h
@@ -159,14 +159,6 @@ public:
         }
     }
 
-    long getDroppedFrameReportCount() {
-        if (mFrameMetricsReporter.get() != nullptr) {
-            return mFrameMetricsReporter->getDroppedReports();
-        }
-
-        return 0;
-    }
-
 private:
     friend class RegisterFrameCallbackTask;
     // TODO: Replace with something better for layer & other GL object
diff --git a/libs/hwui/tests/unit/BufferPoolTests.cpp b/libs/hwui/tests/unit/BufferPoolTests.cpp
deleted file mode 100644
index 44e6d3a0cbea..000000000000
--- a/libs/hwui/tests/unit/BufferPoolTests.cpp
+++ /dev/null
@@ -1,102 +0,0 @@
-/*
- * Copyright (C) 2016 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-#include <gtest/gtest.h>
-
-#include <BufferPool.h>
-#include <utils/StrongPointer.h>
-
-namespace android {
-namespace uirenderer {
-
-TEST(BufferPool, acquireThenRelease) {
-    static const int numRuns = 5;
-
-    // 10 buffers of size 1
-    static const size_t bufferSize = 1;
-    static const size_t bufferCount = 10;
-    sp<BufferPool> pool = new BufferPool(bufferSize, bufferCount);
-
-    for (int run = 0; run < numRuns; run++) {
-        BufferPool::Buffer* acquiredBuffers[bufferCount];
-        for (size_t i = 0; i < bufferCount; i++) {
-            ASSERT_EQ(bufferCount - i, pool->getAvailableBufferCount());
-            acquiredBuffers[i] = pool->acquire();
-            ASSERT_NE(nullptr, acquiredBuffers[i]);
-            ASSERT_TRUE(acquiredBuffers[i]->isUniqueRef());
-        }
-
-        for (size_t i = 0; i < bufferCount; i++) {
-            ASSERT_EQ(i, pool->getAvailableBufferCount());
-            acquiredBuffers[i]->release();
-            acquiredBuffers[i] = nullptr;
-        }
-
-        ASSERT_EQ(bufferCount, pool->getAvailableBufferCount());
-    }
-}
-
-TEST(BufferPool, acquireReleaseInterleaved) {
-    static const int numRuns = 5;
-
-    // 10 buffers of size 1
-    static const size_t bufferSize = 1;
-    static const size_t bufferCount = 10;
-
-    sp<BufferPool> pool = new BufferPool(bufferSize, bufferCount);
-
-    for (int run = 0; run < numRuns; run++) {
-        BufferPool::Buffer* acquiredBuffers[bufferCount];
-
-        // acquire all
-        for (size_t i = 0; i < bufferCount; i++) {
-            ASSERT_EQ(bufferCount - i, pool->getAvailableBufferCount());
-            acquiredBuffers[i] = pool->acquire();
-            ASSERT_NE(nullptr, acquiredBuffers[i]);
-        }
-
-        // release half
-        for (size_t i = 0; i < bufferCount / 2; i++) {
-            ASSERT_EQ(i, pool->getAvailableBufferCount());
-            acquiredBuffers[i]->release();
-            acquiredBuffers[i] = nullptr;
-        }
-
-        const size_t expectedRemaining = bufferCount / 2;
-        ASSERT_EQ(expectedRemaining, pool->getAvailableBufferCount());
-
-        // acquire half
-        for (size_t i = 0; i < bufferCount / 2; i++) {
-            ASSERT_EQ(expectedRemaining - i, pool->getAvailableBufferCount());
-            acquiredBuffers[i] = pool->acquire();
-        }
-
-        // acquire one more, should fail
-        ASSERT_EQ(nullptr, pool->acquire());
-
-        // release all
-        for (size_t i = 0; i < bufferCount; i++) {
-            ASSERT_EQ(i, pool->getAvailableBufferCount());
-            acquiredBuffers[i]->release();
-            acquiredBuffers[i] = nullptr;
-        }
-
-        ASSERT_EQ(bufferCount, pool->getAvailableBufferCount());
-    }
-}
-
-};
-};