From 12b511216b98edba27d9fbe36683a881e81d17c1 Mon Sep 17 00:00:00 2001 From: Ben Widawsky Date: Mon, 14 Oct 2024 11:07:47 -0700 Subject: SF: Add flag for FrameTracker removal Bug: 241394120 Test: builds Flag: com.android.graphics.surfaceflinger.flags.deprecate_frame_tracker Change-Id: Ib7fdfc31f92d292fdceb6027d22c9ed7fbeadb3e --- services/surfaceflinger/common/FlagManager.cpp | 2 ++ services/surfaceflinger/common/include/common/FlagManager.h | 1 + services/surfaceflinger/surfaceflinger_flags_new.aconfig | 11 +++++++++++ 3 files changed, 14 insertions(+) diff --git a/services/surfaceflinger/common/FlagManager.cpp b/services/surfaceflinger/common/FlagManager.cpp index 57ef4c7ac3..12616e3edb 100644 --- a/services/surfaceflinger/common/FlagManager.cpp +++ b/services/surfaceflinger/common/FlagManager.cpp @@ -158,6 +158,7 @@ void FlagManager::dump(std::string& result) const { DUMP_READ_ONLY_FLAG(true_hdr_screenshots); DUMP_READ_ONLY_FLAG(display_config_error_hal); DUMP_READ_ONLY_FLAG(connected_display_hdr); + DUMP_READ_ONLY_FLAG(deprecate_frame_tracker); #undef DUMP_READ_ONLY_FLAG #undef DUMP_SERVER_FLAG @@ -264,6 +265,7 @@ FLAG_MANAGER_READ_ONLY_FLAG(force_compile_graphite_renderengine, ""); FLAG_MANAGER_READ_ONLY_FLAG(true_hdr_screenshots, "debug.sf.true_hdr_screenshots"); FLAG_MANAGER_READ_ONLY_FLAG(display_config_error_hal, ""); FLAG_MANAGER_READ_ONLY_FLAG(connected_display_hdr, ""); +FLAG_MANAGER_READ_ONLY_FLAG(deprecate_frame_tracker, ""); /// Trunk stable server flags /// FLAG_MANAGER_SERVER_FLAG(refresh_rate_overlay_on_external_display, "") diff --git a/services/surfaceflinger/common/include/common/FlagManager.h b/services/surfaceflinger/common/include/common/FlagManager.h index 7716762685..f5bea7237f 100644 --- a/services/surfaceflinger/common/include/common/FlagManager.h +++ b/services/surfaceflinger/common/include/common/FlagManager.h @@ -96,6 +96,7 @@ public: bool true_hdr_screenshots() const; bool display_config_error_hal() const; bool connected_display_hdr() const; + bool deprecate_frame_tracker() const; protected: // overridden for unit tests diff --git a/services/surfaceflinger/surfaceflinger_flags_new.aconfig b/services/surfaceflinger/surfaceflinger_flags_new.aconfig index ce334e476f..014c736a6b 100644 --- a/services/surfaceflinger/surfaceflinger_flags_new.aconfig +++ b/services/surfaceflinger/surfaceflinger_flags_new.aconfig @@ -75,6 +75,17 @@ flag { } } # correct_dpi_with_display_size +flag { + name: "deprecate_frame_tracker" + namespace: "core_graphics" + description: "Deprecate using FrameTracker to accumulate and provide FrameStats" + bug: "241394120" + is_fixed_read_only: true + metadata { + purpose: PURPOSE_BUGFIX + } +} # deprecate_frame_tracker + flag { name: "deprecate_vsync_sf" namespace: "core_graphics" -- cgit v1.2.3-59-g8ed1b From ebdbead46c8dfd270481fcc714e798fc863cf9c5 Mon Sep 17 00:00:00 2001 From: Ben Widawsky Date: Thu, 24 Oct 2024 11:47:50 -0700 Subject: SF: Implement FrameStats directly in FrameTimeline Bug: 241394120 Test: builds, atest CtsUiAutomationTestCases:android.app.uiautomation.cts.UiAutomationTest#testWindowContentFrameStats$ Flag: com.android.graphics.surfaceflinger.flags.deprecate_frame_tracker Change-Id: I3f12d504e821a6ba15a26604eb56f317c9d59a18 --- .../surfaceflinger/FrameTimeline/FrameTimeline.cpp | 29 +++++++++++ .../surfaceflinger/FrameTimeline/FrameTimeline.h | 15 ++++-- services/surfaceflinger/Layer.cpp | 59 +++++++++++++++++----- services/surfaceflinger/Layer.h | 11 +++- .../tests/unittests/MessageQueueTest.cpp | 6 +-- 5 files changed, 101 insertions(+), 19 deletions(-) diff --git a/services/surfaceflinger/FrameTimeline/FrameTimeline.cpp b/services/surfaceflinger/FrameTimeline/FrameTimeline.cpp index 47b811b721..c13e444a99 100644 --- a/services/surfaceflinger/FrameTimeline/FrameTimeline.cpp +++ b/services/surfaceflinger/FrameTimeline/FrameTimeline.cpp @@ -378,6 +378,11 @@ void SurfaceFrame::setAcquireFenceTime(nsecs_t acquireFenceTime) { } } +void SurfaceFrame::setDesiredPresentTime(nsecs_t desiredPresentTime) { + std::scoped_lock lock(mMutex); + mActuals.desiredPresentTime = desiredPresentTime; +} + void SurfaceFrame::setDropTime(nsecs_t dropTime) { std::scoped_lock lock(mMutex); mDropTime = dropTime; @@ -1456,6 +1461,30 @@ float FrameTimeline::computeFps(const std::unordered_set& layerIds) { static_cast(totalPresentToPresentWalls); } +void FrameTimeline::generateFrameStats(int32_t layer, size_t count, FrameStats* outStats) const { + std::scoped_lock lock(mMutex); + + // TODO: Include FPS calculation here + for (auto displayFrame : mDisplayFrames) { + if (!count--) { + break; + } + + if (displayFrame->getActuals().presentTime <= 0) { + continue; + } + + for (const auto& surfaceFrame : displayFrame->getSurfaceFrames()) { + if (surfaceFrame->getLayerId() == layer) { + outStats->actualPresentTimesNano.push_back(surfaceFrame->getActuals().presentTime); + outStats->desiredPresentTimesNano.push_back( + surfaceFrame->getActuals().desiredPresentTime); + outStats->frameReadyTimesNano.push_back(surfaceFrame->getActuals().endTime); + } + } + } +} + std::optional FrameTimeline::getFirstSignalFenceIndex() const { for (size_t i = 0; i < mPendingPresentFences.size(); i++) { const auto& [fence, _] = mPendingPresentFences[i]; diff --git a/services/surfaceflinger/FrameTimeline/FrameTimeline.h b/services/surfaceflinger/FrameTimeline/FrameTimeline.h index cffb61ee10..6cda309440 100644 --- a/services/surfaceflinger/FrameTimeline/FrameTimeline.h +++ b/services/surfaceflinger/FrameTimeline/FrameTimeline.h @@ -85,16 +85,20 @@ enum class FrameStartMetadata : int8_t { */ struct TimelineItem { TimelineItem(const nsecs_t startTime = 0, const nsecs_t endTime = 0, - const nsecs_t presentTime = 0) - : startTime(startTime), endTime(endTime), presentTime(presentTime) {} + const nsecs_t presentTime = 0, const nsecs_t desiredPresentTime = 0) + : startTime(startTime), + endTime(endTime), + presentTime(presentTime), + desiredPresentTime(desiredPresentTime) {} nsecs_t startTime; nsecs_t endTime; nsecs_t presentTime; + nsecs_t desiredPresentTime; bool operator==(const TimelineItem& other) const { return startTime == other.startTime && endTime == other.endTime && - presentTime == other.presentTime; + presentTime == other.presentTime && desiredPresentTime != other.desiredPresentTime; } bool operator!=(const TimelineItem& other) const { return !(*this == other); } @@ -183,6 +187,7 @@ public: void setActualStartTime(nsecs_t actualStartTime); void setActualQueueTime(nsecs_t actualQueueTime); void setAcquireFenceTime(nsecs_t acquireFenceTime); + void setDesiredPresentTime(nsecs_t desiredPresentTime); void setDropTime(nsecs_t dropTime); void setPresentState(PresentState presentState, nsecs_t lastLatchTime = 0); void setRenderRate(Fps renderRate); @@ -341,6 +346,9 @@ public: // containing at least one layer ID. virtual float computeFps(const std::unordered_set& layerIds) = 0; + // Supports the legacy FrameStats interface + virtual void generateFrameStats(int32_t layer, size_t count, FrameStats* outStats) const = 0; + // Restores the max number of display frames to default. Called by SF backdoor. virtual void reset() = 0; }; @@ -501,6 +509,7 @@ public: void parseArgs(const Vector& args, std::string& result) override; void setMaxDisplayFrames(uint32_t size) override; float computeFps(const std::unordered_set& layerIds) override; + void generateFrameStats(int32_t layer, size_t count, FrameStats* outStats) const override; void reset() override; // Sets up the perfetto tracing backend and data source. diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp index 20ba45f96e..195461f47e 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -154,7 +154,7 @@ Layer::Layer(const surfaceflinger::LayerCreationArgs& args) mDrawingState.metadata = args.metadata; mDrawingState.frameTimelineInfo = {}; mDrawingState.postTime = -1; - mFrameTracker.setDisplayRefreshPeriod( + mDeprecatedFrameTracker.setDisplayRefreshPeriod( args.flinger->mScheduler->getPacesetterVsyncPeriod().ns()); mOwnerUid = args.ownerUid; @@ -472,6 +472,9 @@ std::shared_ptr Layer::createSurfaceFrameForTransac getSequence(), mName, mTransactionName, /*isBuffer*/ false, gameMode); + // Buffer hasn't yet been latched, so use mDrawingState + surfaceFrame->setDesiredPresentTime(mDrawingState.desiredPresentTime); + surfaceFrame->setActualStartTime(info.startTimeNanos); // For Transactions, the post time is considered to be both queue and acquire fence time. surfaceFrame->setActualQueueTime(postTime); @@ -490,6 +493,8 @@ std::shared_ptr Layer::createSurfaceFrameForBuffer( mFlinger->mFrameTimeline->createSurfaceFrameForToken(info, mOwnerPid, mOwnerUid, getSequence(), mName, debugName, /*isBuffer*/ true, gameMode); + // Buffer hasn't yet been latched, so use mDrawingState + surfaceFrame->setDesiredPresentTime(mDrawingState.desiredPresentTime); surfaceFrame->setActualStartTime(info.startTimeNanos); // For buffers, acquire fence time will set during latch. surfaceFrame->setActualQueueTime(queueTime); @@ -514,6 +519,8 @@ void Layer::setFrameTimelineVsyncForSkippedFrames(const FrameTimelineInfo& info, mOwnerPid, mOwnerUid, getSequence(), mName, debugName, /*isBuffer*/ false, gameMode); + // Buffer hasn't yet been latched, so use mDrawingState + surfaceFrame->setDesiredPresentTime(mDrawingState.desiredPresentTime); surfaceFrame->setActualStartTime(skippedFrameTimelineInfo.skippedFrameStartTimeNanos); // For Transactions, the post time is considered to be both queue and acquire fence time. surfaceFrame->setActualQueueTime(postTime); @@ -605,15 +612,42 @@ void Layer::miniDump(std::string& result, const frontend::LayerSnapshot& snapsho } void Layer::dumpFrameStats(std::string& result) const { - mFrameTracker.dumpStats(result); + if (FlagManager::getInstance().deprecate_frame_tracker()) { + FrameStats fs = FrameStats(); + getFrameStats(&fs); + for (auto desired = fs.desiredPresentTimesNano.begin(), + actual = fs.actualPresentTimesNano.begin(), + ready = fs.frameReadyTimesNano.begin(); + desired != fs.desiredPresentTimesNano.end() && + actual != fs.actualPresentTimesNano.end() && ready != fs.frameReadyTimesNano.end(); + ++desired, ++actual, ++ready) { + result.append(std::format("{}\t{}\t{}\n", *desired, *actual, *ready)); + } + + result.push_back('\n'); + } else { + mDeprecatedFrameTracker.dumpStats(result); + } } void Layer::clearFrameStats() { - mFrameTracker.clearStats(); + if (FlagManager::getInstance().deprecate_frame_tracker()) { + mFrameStatsHistorySize = 0; + } else { + mDeprecatedFrameTracker.clearStats(); + } } void Layer::getFrameStats(FrameStats* outStats) const { - mFrameTracker.getStats(outStats); + if (FlagManager::getInstance().deprecate_frame_tracker()) { + if (auto ftl = getTimeline()) { + float fps = ftl->get().computeFps({getSequence()}); + ftl->get().generateFrameStats(getSequence(), mFrameStatsHistorySize, outStats); + outStats->refreshPeriodNano = Fps::fromValue(fps).getPeriodNsecs(); + } + } else { + mDeprecatedFrameTracker.getStats(outStats); + } } void Layer::onDisconnect() { @@ -1348,9 +1382,9 @@ void Layer::onCompositionPresented(const DisplayDevice* display, handle->compositorTiming = compositorTiming; } - // Update mFrameTracker. + // Update mDeprecatedFrameTracker. nsecs_t desiredPresentTime = mBufferInfo.mDesiredPresentTime; - mFrameTracker.setDesiredPresentTime(desiredPresentTime); + mDeprecatedFrameTracker.setDesiredPresentTime(desiredPresentTime); const int32_t layerId = getSequence(); mFlinger->mTimeStats->setDesiredTime(layerId, mCurrentFrameNumber, desiredPresentTime); @@ -1370,15 +1404,15 @@ void Layer::onCompositionPresented(const DisplayDevice* display, } } + // The SurfaceFrame's AcquireFence is the same as this. std::shared_ptr frameReadyFence = mBufferInfo.mFenceTime; if (frameReadyFence->isValid()) { - mFrameTracker.setFrameReadyFence(std::move(frameReadyFence)); + mDeprecatedFrameTracker.setFrameReadyFence(std::move(frameReadyFence)); } else { // There was no fence for this frame, so assume that it was ready // to be presented at the desired present time. - mFrameTracker.setFrameReadyTime(desiredPresentTime); + mDeprecatedFrameTracker.setFrameReadyTime(desiredPresentTime); } - if (display) { const auto activeMode = display->refreshRateSelector().getActiveMode(); const Fps refreshRate = activeMode.fps; @@ -1393,7 +1427,7 @@ void Layer::onCompositionPresented(const DisplayDevice* display, mFlinger->mFrameTracer->traceFence(layerId, getCurrentBufferId(), mCurrentFrameNumber, presentFence, FrameTracer::FrameEvent::PRESENT_FENCE); - mFrameTracker.setActualPresentFence(std::shared_ptr(presentFence)); + mDeprecatedFrameTracker.setActualPresentFence(std::shared_ptr(presentFence)); } else if (const auto displayId = PhysicalDisplayId::tryCast(display->getId()); displayId && mFlinger->getHwComposer().isConnected(*displayId)) { // The HWC doesn't support present fences, so use the present timestamp instead. @@ -1414,11 +1448,12 @@ void Layer::onCompositionPresented(const DisplayDevice* display, mFlinger->mFrameTracer->traceTimestamp(layerId, getCurrentBufferId(), mCurrentFrameNumber, actualPresentTime, FrameTracer::FrameEvent::PRESENT_FENCE); - mFrameTracker.setActualPresentTime(actualPresentTime); + mDeprecatedFrameTracker.setActualPresentTime(actualPresentTime); } } - mFrameTracker.advanceFrame(); + mFrameStatsHistorySize++; + mDeprecatedFrameTracker.advanceFrame(); mBufferInfo.mFrameLatencyNeeded = false; } diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h index a2716c6b1f..c234a75693 100644 --- a/services/surfaceflinger/Layer.h +++ b/services/surfaceflinger/Layer.h @@ -18,6 +18,7 @@ #include #include +#include #include #include #include @@ -44,6 +45,7 @@ #include #include +#include #include #include @@ -433,8 +435,12 @@ protected: uint32_t mTransactionFlags{0}; + // Leverages FrameTimeline to generate FrameStats. Since FrameTimeline already has the data, + // statistical history needs to only be tracked by count of frames. + // TODO: Deprecate the '--latency-clear' and get rid of this. + std::atomic mFrameStatsHistorySize; // Timestamp history for UIAutomation. Thread safe. - FrameTracker mFrameTracker; + FrameTracker mDeprecatedFrameTracker; // main thread sp mSidebandStream; @@ -556,6 +562,9 @@ private: std::vector>> mLayerFEs; bool mHandleAlive = false; + std::optional> getTimeline() const { + return *mFlinger->mFrameTimeline; + } }; std::ostream& operator<<(std::ostream& stream, const Layer::FrameRate& rate); diff --git a/services/surfaceflinger/tests/unittests/MessageQueueTest.cpp b/services/surfaceflinger/tests/unittests/MessageQueueTest.cpp index 71f9f88ba7..908637ae76 100644 --- a/services/surfaceflinger/tests/unittests/MessageQueueTest.cpp +++ b/services/surfaceflinger/tests/unittests/MessageQueueTest.cpp @@ -159,9 +159,9 @@ TEST_F(MessageQueueTest, commitTwiceWithCallback) { constexpr VsyncId vsyncId{42}; EXPECT_CALL(mTokenManager, - generateTokenForPredictions(frametimeline::TimelineItem(kStartTime.ns(), - kEndTime.ns(), - kPresentTime.ns()))) + generateTokenForPredictions( + frametimeline::TimelineItem(kStartTime.ns(), kEndTime.ns(), + kPresentTime.ns(), kPresentTime.ns()))) .WillOnce(Return(ftl::to_underlying(vsyncId))); EXPECT_CALL(*mEventQueue.mHandler, dispatchFrame(vsyncId, kPresentTime)).Times(1); EXPECT_NO_FATAL_FAILURE( -- cgit v1.2.3-59-g8ed1b