Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 7ad6a353 authored by Matt Buckley's avatar Matt Buckley Committed by Android (Google) Code Review
Browse files

Merge changes from topic "revert_gc_fix" into udc-qpr-dev

* changes:
  [DO NOT MERGE] Revert "Mitigation for mass GC deletion"
  [DO NOT MERGE] Revert "Send cached target duration when creating sessions"
parents 170fd61c be9bd5da
Loading
Loading
Loading
Loading
+6 −7
Original line number Diff line number Diff line
@@ -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() {
+1 −1
Original line number Diff line number Diff line
@@ -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;
+9 −35
Original line number Diff line number Diff line
@@ -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 */
+1 −10
Original line number Diff line number Diff line
@@ -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 {
+2 −138
Original line number Diff line number Diff line
@@ -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
 No newline at end of file