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

Commit e0edc3af authored by Jiang Tian's avatar Jiang Tian Committed by John Reck
Browse files

Fix crash from asynchronous GPU metrics 

Making the scope more accurate that only acquire the lock when trying
to access frame info in FrameInfoVisualizer, then make it irrelevant
to the real draw operation.

Bug: 317995179
Test: 1.going to developer options
      2. swapping the "profile hwui" option from "none" to "bars"
          and back a couple times, no crash

Change-Id: I069a28a7e847c0c3fca94fd9c43e95382f501b80
parent a35c8f83

libs/hwui/pipeline/skia/SkiaOpenGLPipeline.cpp

+2 −1
Original line number Diff line number Diff line
@@ -118,7 +118,7 @@ IRenderPipeline::DrawResult SkiaOpenGLPipeline::draw( 
        const LightGeometry& lightGeometry, LayerUpdateQueue* layerUpdateQueue,

        const Rect& contentDrawBounds, bool opaque, const LightInfo& lightInfo,

        const std::vector<sp<RenderNode>>& renderNodes, FrameInfoVisualizer* profiler,

        const HardwareBufferRenderParams& bufferParams) {

        const HardwareBufferRenderParams& bufferParams, std::mutex& profilerLock) {

    if (!isCapturingSkp() && !mHardwareBuffer) {

        mEglManager.damageFrame(frame, dirty);

    }
@@ -171,6 +171,7 @@ IRenderPipeline::DrawResult SkiaOpenGLPipeline::draw( 
    // Draw visual debugging features

    if (CC_UNLIKELY(Properties::showDirtyRegions ||

                    ProfileType::None != Properties::getProfileType())) {

        std::scoped_lock lock(profilerLock);

        SkCanvas* profileCanvas = surface->getCanvas();

        SkiaProfileRenderer profileRenderer(profileCanvas, frame.width(), frame.height());

        profiler->draw(profileRenderer);

libs/hwui/pipeline/skia/SkiaOpenGLPipeline.h

+2 −1
Original line number Diff line number Diff line
@@ -42,7 +42,8 @@ public: 
            const LightGeometry& lightGeometry, LayerUpdateQueue* layerUpdateQueue,

            const Rect& contentDrawBounds, bool opaque, const LightInfo& lightInfo,

            const std::vector<sp<RenderNode> >& renderNodes, FrameInfoVisualizer* profiler,

            const renderthread::HardwareBufferRenderParams& bufferParams) override;

            const renderthread::HardwareBufferRenderParams& bufferParams,

            std::mutex& profilerLock) override;

    GrSurfaceOrigin getSurfaceOrigin() override { return kBottomLeft_GrSurfaceOrigin; }

    bool swapBuffers(const renderthread::Frame& frame, IRenderPipeline::DrawResult& drawResult,

                     const SkRect& screenDirty, FrameInfo* currentFrameInfo,

libs/hwui/pipeline/skia/SkiaVulkanPipeline.cpp

+2 −1
Original line number Diff line number Diff line
@@ -75,7 +75,7 @@ IRenderPipeline::DrawResult SkiaVulkanPipeline::draw( 
        const LightGeometry& lightGeometry, LayerUpdateQueue* layerUpdateQueue,

        const Rect& contentDrawBounds, bool opaque, const LightInfo& lightInfo,

        const std::vector<sp<RenderNode>>& renderNodes, FrameInfoVisualizer* profiler,

        const HardwareBufferRenderParams& bufferParams) {

        const HardwareBufferRenderParams& bufferParams, std::mutex& profilerLock) {

    sk_sp<SkSurface> backBuffer;

    SkMatrix preTransform;

    if (mHardwareBuffer) {
@@ -103,6 +103,7 @@ IRenderPipeline::DrawResult SkiaVulkanPipeline::draw( 
    // Draw visual debugging features

    if (CC_UNLIKELY(Properties::showDirtyRegions ||

                    ProfileType::None != Properties::getProfileType())) {

        std::scoped_lock lock(profilerLock);

        SkCanvas* profileCanvas = backBuffer->getCanvas();

        SkAutoCanvasRestore saver(profileCanvas, true);

        profileCanvas->concat(preTransform);

libs/hwui/pipeline/skia/SkiaVulkanPipeline.h

+2 −1
Original line number Diff line number Diff line
@@ -42,7 +42,8 @@ public: 
            const LightGeometry& lightGeometry, LayerUpdateQueue* layerUpdateQueue,

            const Rect& contentDrawBounds, bool opaque, const LightInfo& lightInfo,

            const std::vector<sp<RenderNode> >& renderNodes, FrameInfoVisualizer* profiler,

            const renderthread::HardwareBufferRenderParams& bufferParams) override;

            const renderthread::HardwareBufferRenderParams& bufferParams,

            std::mutex& profilerLock) override;

    GrSurfaceOrigin getSurfaceOrigin() override { return kTopLeft_GrSurfaceOrigin; }

    bool swapBuffers(const renderthread::Frame& frame, IRenderPipeline::DrawResult& drawResult,

                     const SkRect& screenDirty, FrameInfo* currentFrameInfo,

libs/hwui/renderthread/CanvasContext.cpp

+9 −14
Original line number Diff line number Diff line
@@ -625,14 +625,9 @@ void CanvasContext::draw(bool solelyTextureViewUpdates) { 
    {

        // FrameInfoVisualizer accesses the frame events, which cannot be mutated mid-draw

        // or it can lead to memory corruption.

        // This lock is overly broad, but it's the quickest fix since this mutex is otherwise

        // not visible to IRenderPipeline much less FrameInfoVisualizer. And since this is

        // the thread we're primarily concerned about being responsive, this being too broad

        // shouldn't pose a performance issue.

        std::scoped_lock lock(mFrameMetricsReporterMutex);

        drawResult = mRenderPipeline->draw(frame, windowDirty, dirty, mLightGeometry,

                                           &mLayerUpdateQueue, mContentDrawBounds, mOpaque,

                                           mLightInfo, mRenderNodes, &(profiler()), mBufferParams);

        drawResult = mRenderPipeline->draw(

                frame, windowDirty, dirty, mLightGeometry, &mLayerUpdateQueue, mContentDrawBounds,

                mOpaque, mLightInfo, mRenderNodes, &(profiler()), mBufferParams, profilerLock());

    }



    uint64_t frameCompleteNr = getFrameNumber();
@@ -762,7 +757,7 @@ void CanvasContext::draw(bool solelyTextureViewUpdates) { 
            mCurrentFrameInfo->markFrameCompleted();

            mCurrentFrameInfo->set(FrameInfoIndex::GpuCompleted)

                    = mCurrentFrameInfo->get(FrameInfoIndex::FrameCompleted);

            std::scoped_lock lock(mFrameMetricsReporterMutex);

            std::scoped_lock lock(mFrameInfoMutex);

            mJankTracker.finishFrame(*mCurrentFrameInfo, mFrameMetricsReporter, frameCompleteNr,

                                     mSurfaceControlGenerationId);

        }
@@ -791,7 +786,7 @@ void CanvasContext::draw(bool solelyTextureViewUpdates) { 


void CanvasContext::reportMetricsWithPresentTime() {

    {  // acquire lock

        std::scoped_lock lock(mFrameMetricsReporterMutex);

        std::scoped_lock lock(mFrameInfoMutex);

        if (mFrameMetricsReporter == nullptr) {

            return;

        }
@@ -826,7 +821,7 @@ void CanvasContext::reportMetricsWithPresentTime() { 


    forthBehind->set(FrameInfoIndex::DisplayPresentTime) = presentTime;

    {  // acquire lock

        std::scoped_lock lock(mFrameMetricsReporterMutex);

        std::scoped_lock lock(mFrameInfoMutex);

        if (mFrameMetricsReporter != nullptr) {

            mFrameMetricsReporter->reportFrameMetrics(forthBehind->data(), true /*hasPresentTime*/,

                                                      frameNumber, surfaceControlId);
@@ -835,7 +830,7 @@ void CanvasContext::reportMetricsWithPresentTime() { 
}



void CanvasContext::addFrameMetricsObserver(FrameMetricsObserver* observer) {

    std::scoped_lock lock(mFrameMetricsReporterMutex);

    std::scoped_lock lock(mFrameInfoMutex);

    if (mFrameMetricsReporter.get() == nullptr) {

        mFrameMetricsReporter.reset(new FrameMetricsReporter());

    }
@@ -849,7 +844,7 @@ void CanvasContext::addFrameMetricsObserver(FrameMetricsObserver* observer) { 
}



void CanvasContext::removeFrameMetricsObserver(FrameMetricsObserver* observer) {

    std::scoped_lock lock(mFrameMetricsReporterMutex);

    std::scoped_lock lock(mFrameInfoMutex);

    if (mFrameMetricsReporter.get() != nullptr) {

        mFrameMetricsReporter->removeObserver(observer);

        if (!mFrameMetricsReporter->hasObservers()) {
@@ -886,7 +881,7 @@ void CanvasContext::onSurfaceStatsAvailable(void* context, int32_t surfaceContro 
    FrameInfo* frameInfo = instance->getFrameInfoFromLast4(frameNumber, surfaceControlId);



    if (frameInfo != nullptr) {

        std::scoped_lock lock(instance->mFrameMetricsReporterMutex);

        std::scoped_lock lock(instance->mFrameInfoMutex);

        frameInfo->set(FrameInfoIndex::FrameCompleted) = std::max(gpuCompleteTime,

                frameInfo->get(FrameInfoIndex::SwapBuffersCompleted));

        frameInfo->set(FrameInfoIndex::GpuCompleted) = std::max(