[DO NOT MERGE] Revert "Mitigation for mass GC deletion"
Revert submission 24828717
Reason for revert: performance concerns
Reverted changes: /q/submissionid:24828717
Bug: 300360668
Bug: 302620048
Change-Id: Iefb7488f6e100ca7cbb0d43337e5830860ebebe5
diff --git a/libs/hwui/renderthread/CanvasContext.cpp b/libs/hwui/renderthread/CanvasContext.cpp
index 310e39e..f5b3ca6 100644
--- a/libs/hwui/renderthread/CanvasContext.cpp
+++ b/libs/hwui/renderthread/CanvasContext.cpp
@@ -123,7 +123,7 @@
, mProfiler(mJankTracker.frames(), thread.timeLord().frameIntervalNanos())
, mContentDrawBounds(0, 0, 0, 0)
, mRenderPipeline(std::move(renderPipeline))
- , mHintSessionWrapper(std::make_shared<HintSessionWrapper>(uiThreadId, renderThreadId)) {
+ , mHintSessionWrapper(uiThreadId, renderThreadId) {
mRenderThread.cacheManager().registerCanvasContext(this);
rootRenderNode->makeRoot();
mRenderNodes.emplace_back(rootRenderNode);
@@ -160,7 +160,6 @@
destroyHardwareResources();
mAnimationContext->destroy();
mRenderThread.cacheManager().onContextStopped(this);
- mHintSessionWrapper->delayedDestroy(mRenderThread, 2_s, mHintSessionWrapper);
}
static void setBufferCount(ANativeWindow* window) {
@@ -740,7 +739,7 @@
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);
@@ -748,7 +747,7 @@
int64_t actualDuration = frameDuration -
(std::min(syncDelayDuration, mLastDequeueBufferDuration)) -
dequeueBufferDuration - idleDuration;
- mHintSessionWrapper->reportActualWorkDuration(actualDuration);
+ mHintSessionWrapper.reportActualWorkDuration(actualDuration);
}
mLastDequeueBufferDuration = dequeueBufferDuration;
@@ -1082,11 +1081,11 @@
}
void CanvasContext::sendLoadResetHint() {
- mHintSessionWrapper->sendLoadResetHint();
+ mHintSessionWrapper.sendLoadResetHint();
}
void CanvasContext::sendLoadIncreaseHint() {
- mHintSessionWrapper->sendLoadIncreaseHint();
+ mHintSessionWrapper.sendLoadIncreaseHint();
}
void CanvasContext::setSyncDelayDuration(nsecs_t duration) {
@@ -1094,7 +1093,7 @@
}
void CanvasContext::startHintSession() {
- mHintSessionWrapper->init();
+ mHintSessionWrapper.init();
}
bool CanvasContext::shouldDither() {
diff --git a/libs/hwui/renderthread/CanvasContext.h b/libs/hwui/renderthread/CanvasContext.h
index 10a4afb..32ac5af 100644
--- a/libs/hwui/renderthread/CanvasContext.h
+++ b/libs/hwui/renderthread/CanvasContext.h
@@ -359,7 +359,7 @@
std::function<bool(int64_t, int64_t, int64_t)> mASurfaceTransactionCallback;
std::function<void()> mPrepareSurfaceControlForWebviewCallback;
- std::shared_ptr<HintSessionWrapper> mHintSessionWrapper;
+ HintSessionWrapper mHintSessionWrapper;
nsecs_t mLastDequeueBufferDuration = 0;
nsecs_t mSyncDelayDuration = 0;
nsecs_t mIdleDuration = 0;
diff --git a/libs/hwui/renderthread/HintSessionWrapper.cpp b/libs/hwui/renderthread/HintSessionWrapper.cpp
index d1ebe6d..b34da51 100644
--- a/libs/hwui/renderthread/HintSessionWrapper.cpp
+++ b/libs/hwui/renderthread/HintSessionWrapper.cpp
@@ -24,7 +24,6 @@
#include <vector>
#include "../Properties.h"
-#include "RenderThread.h"
#include "thread/CommonPool.h"
using namespace std::chrono_literals;
@@ -63,9 +62,8 @@
}
void HintSessionWrapper::destroy() {
- if (mHintSessionFuture.has_value()) {
- mHintSession = mHintSessionFuture->get();
- mHintSessionFuture = std::nullopt;
+ if (mHintSessionFuture.valid()) {
+ mHintSession = mHintSessionFuture.get();
}
if (mHintSession) {
mBinding->closeSession(mHintSession);
@@ -76,12 +74,12 @@
bool HintSessionWrapper::init() {
if (mHintSession != nullptr) return true;
+
// If we're waiting for the session
- if (mHintSessionFuture.has_value()) {
+ if (mHintSessionFuture.valid()) {
// If the session is here
- if (mHintSessionFuture->wait_for(0s) == std::future_status::ready) {
- mHintSession = mHintSessionFuture->get();
- mHintSessionFuture = std::nullopt;
+ if (mHintSessionFuture.wait_for(0s) == std::future_status::ready) {
+ mHintSession = mHintSessionFuture.get();
if (mHintSession != nullptr) {
mSessionValid = true;
return true;
@@ -138,7 +136,6 @@
actualDurationNanos < kSanityCheckUpperBound) {
mBinding->reportActualWorkDuration(mHintSession, actualDurationNanos);
}
- mLastFrameNotification = systemTime();
}
void HintSessionWrapper::sendLoadResetHint() {
@@ -156,28 +153,6 @@
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 */
diff --git a/libs/hwui/renderthread/HintSessionWrapper.h b/libs/hwui/renderthread/HintSessionWrapper.h
index 36e91ea..f8b876e 100644
--- a/libs/hwui/renderthread/HintSessionWrapper.h
+++ b/libs/hwui/renderthread/HintSessionWrapper.h
@@ -19,7 +19,6 @@
#include <android/performance_hint.h>
#include <future>
-#include <optional>
#include "utils/TimeUtils.h"
@@ -28,8 +27,6 @@
namespace renderthread {
-class RenderThread;
-
class HintSessionWrapper {
public:
friend class HintSessionWrapperTests;
@@ -43,15 +40,10 @@
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;
- // This needs to work concurrently for testing
- std::optional<std::shared_future<APerformanceHintSession*>> mHintSessionFuture;
+ std::future<APerformanceHintSession*> mHintSessionFuture;
int mResetsSinceLastReport = 0;
nsecs_t mLastFrameNotification = 0;
diff --git a/libs/hwui/tests/unit/HintSessionWrapperTests.cpp b/libs/hwui/tests/unit/HintSessionWrapperTests.cpp
index 5a10f4f..623be1e 100644
--- a/libs/hwui/tests/unit/HintSessionWrapperTests.cpp
+++ b/libs/hwui/tests/unit/HintSessionWrapperTests.cpp
@@ -23,11 +23,9 @@
#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);
@@ -44,9 +42,6 @@
protected:
std::shared_ptr<HintSessionWrapper> mWrapper;
- std::promise<int> blockDestroyCallUntil;
- std::promise<int> waitForDestroyFinished;
-
class MockHintSessionBinding : public HintSessionWrapper::HintSessionBinding {
public:
void init() override;
@@ -58,17 +53,11 @@
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,
@@ -76,12 +65,6 @@
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) {
@@ -102,21 +85,7 @@
static void stubSendHint(APerformanceHintSession* session, int32_t hintId) {
sMockBinding->fakeSendHint(session, hintId);
};
- 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); });
- });
- }
+ void waitForWrapperReady() { mWrapper->mHintSessionFuture.wait(); }
};
std::shared_ptr<HintSessionWrapperTests::MockHintSessionBinding>
@@ -144,7 +113,6 @@
}
void HintSessionWrapperTests::TearDown() {
- // Ensure that anything running on RT is completely finished
mWrapper = nullptr;
sMockBinding = nullptr;
}
@@ -154,7 +122,6 @@
sMockBinding->createSession = stubSlowCreateSession;
mWrapper->init();
mWrapper = nullptr;
- Mock::VerifyAndClearExpectations(sMockBinding.get());
}
TEST_F(HintSessionWrapperTests, sessionInitializesCorrectly) {
@@ -181,107 +148,4 @@
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
+} // namespace android::uirenderer::renderthread
\ No newline at end of file