Mitigation for mass GC deletion

Delete sessions after 2 seconds from when CC::destroy() is called, to
ensure they get the chance to be re-used without being destroyed but
still get destroyed if they aren't re-used.

This also adds several new unit tests around the functionality to ensure
its thread-safety.

Test: hwuitest
Bug: 300360668
Change-Id: Icc35293ff49e14dc3599fc12d153f70acd29042a

libs/hwui/renderthread/CanvasContext.cpp

@@ -123,7 +123,7 @@ CanvasContext::CanvasContext(RenderThread& thread, bool translucent, RenderNode*
         , mProfiler(mJankTracker.frames(), thread.timeLord().frameIntervalNanos())
         , mContentDrawBounds(0, 0, 0, 0)
         , mRenderPipeline(std::move(renderPipeline))
-        , mHintSessionWrapper(uiThreadId, renderThreadId) {
+        , mHintSessionWrapper(std::make_shared<HintSessionWrapper>(uiThreadId, renderThreadId)) {
     mRenderThread.cacheManager().registerCanvasContext(this);
     mRenderThread.renderState().registerContextCallback(this);
     rootRenderNode->makeRoot();
@@ -162,6 +162,7 @@ void CanvasContext::destroy() {
     destroyHardwareResources();
     mAnimationContext->destroy();
     mRenderThread.cacheManager().onContextStopped(this);
+    mHintSessionWrapper->delayedDestroy(mRenderThread, 2_s, mHintSessionWrapper);
 }
 
 static void setBufferCount(ANativeWindow* window) {
@@ -766,7 +767,7 @@ void CanvasContext::draw(bool solelyTextureViewUpdates) {
     int64_t frameDeadline = mCurrentFrameInfo->get(FrameInfoIndex::FrameDeadline);
     int64_t dequeueBufferDuration = mCurrentFrameInfo->get(FrameInfoIndex::DequeueBufferDuration);
 
-    mHintSessionWrapper.updateTargetWorkDuration(frameDeadline - intendedVsync);
+    mHintSessionWrapper->updateTargetWorkDuration(frameDeadline - intendedVsync);
 
     if (didDraw) {
         int64_t frameStartTime = mCurrentFrameInfo->get(FrameInfoIndex::FrameStartTime);
@@ -774,7 +775,7 @@ void CanvasContext::draw(bool solelyTextureViewUpdates) {
         int64_t actualDuration = frameDuration -
                                  (std::min(syncDelayDuration, mLastDequeueBufferDuration)) -
                                  dequeueBufferDuration - idleDuration;
-        mHintSessionWrapper.reportActualWorkDuration(actualDuration);
+        mHintSessionWrapper->reportActualWorkDuration(actualDuration);
     }
 
     mLastDequeueBufferDuration = dequeueBufferDuration;
@@ -1112,11 +1113,11 @@ void CanvasContext::prepareSurfaceControlForWebview() {
 }
 
 void CanvasContext::sendLoadResetHint() {
-    mHintSessionWrapper.sendLoadResetHint();
+    mHintSessionWrapper->sendLoadResetHint();
 }
 
 void CanvasContext::sendLoadIncreaseHint() {
-    mHintSessionWrapper.sendLoadIncreaseHint();
+    mHintSessionWrapper->sendLoadIncreaseHint();
 }
 
 void CanvasContext::setSyncDelayDuration(nsecs_t duration) {
@@ -1124,7 +1125,7 @@ void CanvasContext::setSyncDelayDuration(nsecs_t duration) {
 }
 
 void CanvasContext::startHintSession() {
-    mHintSessionWrapper.init();
+    mHintSessionWrapper->init();
 }
 
 bool CanvasContext::shouldDither() {

libs/hwui/renderthread/CanvasContext.h

@@ -363,7 +363,7 @@ private:
     std::function<bool(int64_t, int64_t, int64_t)> mASurfaceTransactionCallback;
     std::function<void()> mPrepareSurfaceControlForWebviewCallback;
 
-    HintSessionWrapper mHintSessionWrapper;
+    std::shared_ptr<HintSessionWrapper> mHintSessionWrapper;
     nsecs_t mLastDequeueBufferDuration = 0;
     nsecs_t mSyncDelayDuration = 0;
     nsecs_t mIdleDuration = 0;

libs/hwui/renderthread/HintSessionWrapper.cpp

@@ -24,6 +24,7 @@
 #include <vector>
 
 #include "../Properties.h"
+#include "RenderThread.h"
 #include "thread/CommonPool.h"
 
 using namespace std::chrono_literals;
@@ -62,8 +63,9 @@ HintSessionWrapper::~HintSessionWrapper() {
 }
 
 void HintSessionWrapper::destroy() {
-    if (mHintSessionFuture.valid()) {
-        mHintSession = mHintSessionFuture.get();
+    if (mHintSessionFuture.has_value()) {
+        mHintSession = mHintSessionFuture->get();
+        mHintSessionFuture = std::nullopt;
     }
     if (mHintSession) {
         mBinding->closeSession(mHintSession);
@@ -74,12 +76,12 @@ void HintSessionWrapper::destroy() {
 
 bool HintSessionWrapper::init() {
     if (mHintSession != nullptr) return true;
-
     // If we're waiting for the session
-    if (mHintSessionFuture.valid()) {
+    if (mHintSessionFuture.has_value()) {
         // If the session is here
-        if (mHintSessionFuture.wait_for(0s) == std::future_status::ready) {
-            mHintSession = mHintSessionFuture.get();
+        if (mHintSessionFuture->wait_for(0s) == std::future_status::ready) {
+            mHintSession = mHintSessionFuture->get();
+            mHintSessionFuture = std::nullopt;
             if (mHintSession != nullptr) {
                 mSessionValid = true;
                 return true;
@@ -136,6 +138,7 @@ void HintSessionWrapper::reportActualWorkDuration(long actualDurationNanos) {
         actualDurationNanos < kSanityCheckUpperBound) {
         mBinding->reportActualWorkDuration(mHintSession, actualDurationNanos);
     }
+    mLastFrameNotification = systemTime();
 }
 
 void HintSessionWrapper::sendLoadResetHint() {
@@ -153,6 +156,28 @@ void HintSessionWrapper::sendLoadResetHint() {
 void HintSessionWrapper::sendLoadIncreaseHint() {
     if (!init()) return;
     mBinding->sendHint(mHintSession, static_cast<int32_t>(SessionHint::CPU_LOAD_UP));
+    mLastFrameNotification = systemTime();
+}
+
+bool HintSessionWrapper::alive() {
+    return mHintSession != nullptr;
+}
+
+nsecs_t HintSessionWrapper::getLastUpdate() {
+    return mLastFrameNotification;
+}
+
+// Requires passing in its shared_ptr since it shouldn't own a shared_ptr to itself
+void HintSessionWrapper::delayedDestroy(RenderThread& rt, nsecs_t delay,
+                                        std::shared_ptr<HintSessionWrapper> wrapperPtr) {
+    nsecs_t lastUpdate = wrapperPtr->getLastUpdate();
+    rt.queue().postDelayed(delay, [lastUpdate = lastUpdate, wrapper = wrapperPtr]() mutable {
+        if (wrapper->getLastUpdate() == lastUpdate) {
+            wrapper->destroy();
+        }
+        // Ensure the shared_ptr is killed at the end of the method
+        wrapper = nullptr;
+    });
 }
 
 } /* namespace renderthread */

libs/hwui/renderthread/HintSessionWrapper.h

@@ -19,6 +19,7 @@
 #include <android/performance_hint.h>
 
 #include <future>
+#include <optional>
 
 #include "utils/TimeUtils.h"
 
@@ -27,6 +28,8 @@ namespace uirenderer {
 
 namespace renderthread {
 
+class RenderThread;
+
 class HintSessionWrapper {
 public:
     friend class HintSessionWrapperTests;
@@ -40,10 +43,15 @@ public:
     void sendLoadIncreaseHint();
     bool init();
     void destroy();
+    bool alive();
+    nsecs_t getLastUpdate();
+    void delayedDestroy(renderthread::RenderThread& rt, nsecs_t delay,
+                        std::shared_ptr<HintSessionWrapper> wrapperPtr);
 
 private:
     APerformanceHintSession* mHintSession = nullptr;
-    std::future<APerformanceHintSession*> mHintSessionFuture;
+    // This needs to work concurrently for testing
+    std::optional<std::shared_future<APerformanceHintSession*>> mHintSessionFuture;
 
     int mResetsSinceLastReport = 0;
     nsecs_t mLastFrameNotification = 0;

libs/hwui/tests/unit/HintSessionWrapperTests.cpp

@@ -23,9 +23,11 @@
 #include <chrono>
 
 #include "Properties.h"
+#include "tests/common/TestUtils.h"
 
 using namespace testing;
 using namespace std::chrono_literals;
+using namespace android::uirenderer::renderthread;
 
 APerformanceHintManager* managerPtr = reinterpret_cast<APerformanceHintManager*>(123);
 APerformanceHintSession* sessionPtr = reinterpret_cast<APerformanceHintSession*>(456);
@@ -42,6 +44,9 @@ public:
 protected:
     std::shared_ptr<HintSessionWrapper> mWrapper;
 
+    std::promise<int> blockDestroyCallUntil;
+    std::promise<int> waitForDestroyFinished;
+
     class MockHintSessionBinding : public HintSessionWrapper::HintSessionBinding {
     public:
         void init() override;
@@ -53,11 +58,17 @@ protected:
         MOCK_METHOD(void, fakeUpdateTargetWorkDuration, (APerformanceHintSession*, int64_t));
         MOCK_METHOD(void, fakeReportActualWorkDuration, (APerformanceHintSession*, int64_t));
         MOCK_METHOD(void, fakeSendHint, (APerformanceHintSession*, int32_t));
+        // Needs to be on the binding so it can be accessed from static methods
+        std::promise<int> allowCreationToFinish;
     };
 
     // Must be static so it can have function pointers we can point to with static methods
     static std::shared_ptr<MockHintSessionBinding> sMockBinding;
 
+    static void allowCreationToFinish() { sMockBinding->allowCreationToFinish.set_value(1); }
+    void allowDelayedDestructionToStart() { blockDestroyCallUntil.set_value(1); }
+    void waitForDelayedDestructionToFinish() { waitForDestroyFinished.get_future().wait(); }
+
     // Must be static so we can point to them as normal fn pointers with HintSessionBinding
     static APerformanceHintManager* stubGetManager() { return sMockBinding->fakeGetManager(); };
     static APerformanceHintSession* stubCreateSession(APerformanceHintManager* manager,
@@ -65,6 +76,12 @@ protected:
                                                       int64_t initialTarget) {
         return sMockBinding->fakeCreateSession(manager, ids, idsSize, initialTarget);
     }
+    static APerformanceHintSession* stubManagedCreateSession(APerformanceHintManager* manager,
+                                                             const int32_t* ids, size_t idsSize,
+                                                             int64_t initialTarget) {
+        sMockBinding->allowCreationToFinish.get_future().wait();
+        return sMockBinding->fakeCreateSession(manager, ids, idsSize, initialTarget);
+    }
     static APerformanceHintSession* stubSlowCreateSession(APerformanceHintManager* manager,
                                                           const int32_t* ids, size_t idsSize,
                                                           int64_t initialTarget) {
@@ -85,7 +102,21 @@ protected:
     static void stubSendHint(APerformanceHintSession* session, int32_t hintId) {
         sMockBinding->fakeSendHint(session, hintId);
     };
-    void waitForWrapperReady() { mWrapper->mHintSessionFuture.wait(); }
+    void waitForWrapperReady() {
+        if (mWrapper->mHintSessionFuture.has_value()) {
+            mWrapper->mHintSessionFuture->wait();
+        }
+    }
+    void scheduleDelayedDestroyManaged() {
+        TestUtils::runOnRenderThread([&](renderthread::RenderThread& rt) {
+            // Guaranteed to be scheduled first, allows destruction to start
+            rt.queue().postDelayed(0_ms, [&] { blockDestroyCallUntil.get_future().wait(); });
+            // Guaranteed to be scheduled second, destroys the session
+            mWrapper->delayedDestroy(rt, 1_ms, mWrapper);
+            // This is guaranteed to be queued after the destroy, signals that destruction is done
+            rt.queue().postDelayed(1_ms, [&] { waitForDestroyFinished.set_value(1); });
+        });
+    }
 };
 
 std::shared_ptr<HintSessionWrapperTests::MockHintSessionBinding>
@@ -113,6 +144,7 @@ void HintSessionWrapperTests::MockHintSessionBinding::init() {
 }
 
 void HintSessionWrapperTests::TearDown() {
+    // Ensure that anything running on RT is completely finished
     mWrapper = nullptr;
     sMockBinding = nullptr;
 }
@@ -122,6 +154,7 @@ TEST_F(HintSessionWrapperTests, destructorClosesBackgroundSession) {
     sMockBinding->createSession = stubSlowCreateSession;
     mWrapper->init();
     mWrapper = nullptr;
+    Mock::VerifyAndClearExpectations(sMockBinding.get());
 }
 
 TEST_F(HintSessionWrapperTests, sessionInitializesCorrectly) {
@@ -148,4 +181,107 @@ TEST_F(HintSessionWrapperTests, loadResetHintsSendCorrectly) {
     mWrapper->sendLoadResetHint();
 }
 
+TEST_F(HintSessionWrapperTests, delayedDeletionWorksCorrectlyAndOnlyClosesOnce) {
+    EXPECT_CALL(*sMockBinding, fakeCloseSession(sessionPtr)).Times(1);
+    mWrapper->init();
+    waitForWrapperReady();
+    // Init a second time just to ensure the wrapper grabs the promise value
+    mWrapper->init();
+
+    EXPECT_EQ(mWrapper->alive(), true);
+
+    // Schedule delayed destruction, allow it to run, and check when it's done
+    scheduleDelayedDestroyManaged();
+    allowDelayedDestructionToStart();
+    waitForDelayedDestructionToFinish();
+
+    // Ensure it closed within the timeframe of the test
+    Mock::VerifyAndClearExpectations(sMockBinding.get());
+    EXPECT_EQ(mWrapper->alive(), false);
+    // If we then delete the wrapper, it shouldn't close the session again
+    EXPECT_CALL(*sMockBinding, fakeCloseSession(_)).Times(0);
+    mWrapper = nullptr;
+}
+
+TEST_F(HintSessionWrapperTests, delayedDeletionResolvesBeforeAsyncCreationFinishes) {
+    // Here we test whether queueing delayedDestroy works while creation is still happening, if
+    // creation happens after
+    EXPECT_CALL(*sMockBinding, fakeCloseSession(sessionPtr)).Times(1);
+    sMockBinding->createSession = &stubManagedCreateSession;
+
+    // Start creating the session and destroying it at the same time
+    mWrapper->init();
+    scheduleDelayedDestroyManaged();
+
+    // Allow destruction to happen first
+    allowDelayedDestructionToStart();
+
+    // Make sure destruction has had time to happen
+    std::this_thread::sleep_for(50ms);
+
+    // Then, allow creation to finish after delayed destroy runs
+    allowCreationToFinish();
+
+    // Wait for destruction to finish
+    waitForDelayedDestructionToFinish();
+
+    // Ensure it closed within the timeframe of the test
+    Mock::VerifyAndClearExpectations(sMockBinding.get());
+    EXPECT_EQ(mWrapper->alive(), false);
+}
+
+TEST_F(HintSessionWrapperTests, delayedDeletionResolvesAfterAsyncCreationFinishes) {
+    // Here we test whether queueing delayedDestroy works while creation is still happening, if
+    // creation happens before
+    EXPECT_CALL(*sMockBinding, fakeCloseSession(sessionPtr)).Times(1);
+    sMockBinding->createSession = &stubManagedCreateSession;
+
+    // Start creating the session and destroying it at the same time
+    mWrapper->init();
+    scheduleDelayedDestroyManaged();
+
+    // Allow creation to happen first
+    allowCreationToFinish();
+
+    // Make sure creation has had time to happen
+    waitForWrapperReady();
+
+    // Then allow destruction to happen after creation is done
+    allowDelayedDestructionToStart();
+
+    // Wait for it to finish
+    waitForDelayedDestructionToFinish();
+
+    // Ensure it closed within the timeframe of the test
+    Mock::VerifyAndClearExpectations(sMockBinding.get());
+    EXPECT_EQ(mWrapper->alive(), false);
+}
+
+TEST_F(HintSessionWrapperTests, delayedDeletionDoesNotKillReusedSession) {
+    EXPECT_CALL(*sMockBinding, fakeCloseSession(sessionPtr)).Times(0);
+    EXPECT_CALL(*sMockBinding,
+                fakeSendHint(sessionPtr, static_cast<int32_t>(SessionHint::CPU_LOAD_UP)))
+            .Times(1);
+
+    mWrapper->init();
+    waitForWrapperReady();
+    // Init a second time just to grab the wrapper from the promise
+    mWrapper->init();
+    EXPECT_EQ(mWrapper->alive(), true);
+
+    // First schedule the deletion
+    scheduleDelayedDestroyManaged();
+
+    // Then, send a hint to update the timestamp
+    mWrapper->sendLoadIncreaseHint();
+
+    // Then, run the delayed deletion after sending the update
+    allowDelayedDestructionToStart();
+    waitForDelayedDestructionToFinish();
+
+    // Ensure it didn't close within the timeframe of the test
+    Mock::VerifyAndClearExpectations(sMockBinding.get());
+    EXPECT_EQ(mWrapper->alive(), true);
+}
+
 }  // namespace android::uirenderer::renderthread
\ No newline at end of file