diff --git a/libs/hwui/renderthread/CanvasContext.cpp b/libs/hwui/renderthread/CanvasContext.cpp index 310e39e1d0a2b0510acb866c6d19f9a2efc2aa10..f5b3ca602469b74be9deda82b397edd5df6a00bd 100644 --- a/libs/hwui/renderthread/CanvasContext.cpp +++ b/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(std::make_shared<HintSessionWrapper>(uiThreadId, renderThreadId)) { + , mHintSessionWrapper(uiThreadId, renderThreadId) { mRenderThread.cacheManager().registerCanvasContext(this); rootRenderNode->makeRoot(); mRenderNodes.emplace_back(rootRenderNode); @@ -160,7 +160,6 @@ void CanvasContext::destroy() { destroyHardwareResources(); mAnimationContext->destroy(); mRenderThread.cacheManager().onContextStopped(this); - mHintSessionWrapper->delayedDestroy(mRenderThread, 2_s, mHintSessionWrapper); } static void setBufferCount(ANativeWindow* window) { @@ -740,7 +739,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); @@ -748,7 +747,7 @@ void CanvasContext::draw(bool solelyTextureViewUpdates) { int64_t actualDuration = frameDuration - (std::min(syncDelayDuration, mLastDequeueBufferDuration)) - dequeueBufferDuration - idleDuration; - mHintSessionWrapper->reportActualWorkDuration(actualDuration); + mHintSessionWrapper.reportActualWorkDuration(actualDuration); } mLastDequeueBufferDuration = dequeueBufferDuration; @@ -1082,11 +1081,11 @@ void CanvasContext::prepareSurfaceControlForWebview() { } 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::setSyncDelayDuration(nsecs_t duration) { } 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 10a4afb23d35a4c4260debb60d6e5a7a1c74f5de..32ac5af94c1460b3487eac1e8ee6fc5edf48001f 100644 --- a/libs/hwui/renderthread/CanvasContext.h +++ b/libs/hwui/renderthread/CanvasContext.h @@ -359,7 +359,7 @@ private: 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 565da52de9403190025d6809fa0f6487d29c5c6b..b34da5153a723bf584be6dfbffa1d73462b06d33 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,26 +62,24 @@ HintSessionWrapper::~HintSessionWrapper() { } void HintSessionWrapper::destroy() { - if (mHintSessionFuture.has_value()) { - mHintSession = mHintSessionFuture->get(); - mHintSessionFuture = std::nullopt; + if (mHintSessionFuture.valid()) { + mHintSession = mHintSessionFuture.get(); } if (mHintSession) { mBinding->closeSession(mHintSession); mSessionValid = true; mHintSession = nullptr; } - mResetsSinceLastReport = 0; } 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; @@ -112,10 +109,10 @@ bool HintSessionWrapper::init() { // Use a placeholder target value to initialize, // this will always be replaced elsewhere before it gets used - int64_t targetDurationNanos = - mLastTargetWorkDuration == 0 ? kDefaultTargetDuration : mLastTargetWorkDuration; + int64_t defaultTargetDurationNanos = 16666667; mHintSessionFuture = CommonPool::async([=, this, tids = std::move(tids)] { - return mBinding->createSession(manager, tids.data(), tids.size(), targetDurationNanos); + return mBinding->createSession(manager, tids.data(), tids.size(), + defaultTargetDurationNanos); }); return false; } @@ -139,7 +136,6 @@ void HintSessionWrapper::reportActualWorkDuration(long actualDurationNanos) { actualDurationNanos < kSanityCheckUpperBound) { mBinding->reportActualWorkDuration(mHintSession, actualDurationNanos); } - mLastFrameNotification = systemTime(); } void HintSessionWrapper::sendLoadResetHint() { @@ -157,28 +153,6 @@ 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 */ diff --git a/libs/hwui/renderthread/HintSessionWrapper.h b/libs/hwui/renderthread/HintSessionWrapper.h index 41891cd80a42bcbd8f0a27a67c6e85bc56166645..f8b876e28d5195213c456c57633baa5c88e9a601 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 uirenderer { namespace renderthread { -class RenderThread; - class HintSessionWrapper { public: friend class HintSessionWrapperTests; @@ -43,15 +40,10 @@ 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; - // 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; @@ -65,7 +57,6 @@ private: static constexpr nsecs_t kResetHintTimeout = 100_ms; static constexpr int64_t kSanityCheckLowerBound = 100_us; static constexpr int64_t kSanityCheckUpperBound = 10_s; - static constexpr int64_t kDefaultTargetDuration = 16666667; // Allows easier stub when testing class HintSessionBinding { diff --git a/libs/hwui/tests/unit/HintSessionWrapperTests.cpp b/libs/hwui/tests/unit/HintSessionWrapperTests.cpp index 5a10f4f959aa8507fff0049ea2b7172e38df3340..623be1e3f3f36b1c648c0f2fd0f9a8b07f4e8626 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 @@ public: 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 @@ 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, @@ -76,12 +65,6 @@ 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) { @@ -102,21 +85,7 @@ protected: 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::MockHintSessionBinding::init() { } void HintSessionWrapperTests::TearDown() { - // Ensure that anything running on RT is completely finished mWrapper = nullptr; sMockBinding = nullptr; } @@ -154,7 +122,6 @@ TEST_F(HintSessionWrapperTests, destructorClosesBackgroundSession) { sMockBinding->createSession = stubSlowCreateSession; mWrapper->init(); mWrapper = nullptr; - Mock::VerifyAndClearExpectations(sMockBinding.get()); } TEST_F(HintSessionWrapperTests, sessionInitializesCorrectly) { @@ -181,107 +148,4 @@ 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 +} // namespace android::uirenderer::renderthread \ No newline at end of file