Merge changes I3a501dd0,I23f3b85f into sc-dev
* changes:
Consider invalid present fence time in FrameTimeline
Call SurfaceFrame::onPresent if DisplayFrame predictions expired
diff --git a/services/surfaceflinger/FrameTimeline/FrameTimeline.cpp b/services/surfaceflinger/FrameTimeline/FrameTimeline.cpp
index 03e38f3..3442706 100644
--- a/services/surfaceflinger/FrameTimeline/FrameTimeline.cpp
+++ b/services/surfaceflinger/FrameTimeline/FrameTimeline.cpp
@@ -444,37 +444,26 @@
dumpTable(result, mPredictions, mActuals, indent, mPredictionState, baseTime);
}
-void SurfaceFrame::onPresent(nsecs_t presentTime, int32_t displayFrameJankType, Fps refreshRate,
- nsecs_t displayDeadlineDelta, nsecs_t displayPresentDelta) {
- std::scoped_lock lock(mMutex);
-
- if (mPresentState != PresentState::Presented) {
- // No need to update dropped buffers
+void SurfaceFrame::classifyJankLocked(int32_t displayFrameJankType, const Fps& refreshRate,
+ nsecs_t& deadlineDelta) {
+ if (mPredictionState == PredictionState::Expired ||
+ mActuals.presentTime == Fence::SIGNAL_TIME_INVALID) {
+ // Cannot do any classification for invalid present time.
+ // For prediction expired case, we do not know what happened here to classify this
+ // correctly. This could potentially be AppDeadlineMissed but that's assuming no app will
+ // request frames 120ms apart.
+ mJankType = JankType::Unknown;
+ deadlineDelta = -1;
return;
}
- mActuals.presentTime = presentTime;
- // Jank Analysis for SurfaceFrame
if (mPredictionState == PredictionState::None) {
// Cannot do jank classification on frames that don't have a token.
return;
}
- if (mPredictionState == PredictionState::Expired) {
- // We do not know what happened here to classify this correctly. This could
- // potentially be AppDeadlineMissed but that's assuming no app will request frames
- // 120ms apart.
- mJankType = JankType::Unknown;
- mFramePresentMetadata = FramePresentMetadata::UnknownPresent;
- mFrameReadyMetadata = FrameReadyMetadata::UnknownFinish;
- const constexpr nsecs_t kAppDeadlineDelta = -1;
- mTimeStats->incrementJankyFrames({refreshRate, mRenderRate, mOwnerUid, mLayerName,
- mJankType, displayDeadlineDelta, displayPresentDelta,
- kAppDeadlineDelta});
- return;
- }
+ deadlineDelta = mActuals.endTime - mPredictions.endTime;
const nsecs_t presentDelta = mActuals.presentTime - mPredictions.presentTime;
- const nsecs_t deadlineDelta = mActuals.endTime - mPredictions.endTime;
const nsecs_t deltaToVsync = refreshRate.getPeriodNsecs() > 0
? std::abs(presentDelta) % refreshRate.getPeriodNsecs()
: 0;
@@ -558,8 +547,28 @@
}
}
}
- mTimeStats->incrementJankyFrames({refreshRate, mRenderRate, mOwnerUid, mLayerName, mJankType,
- displayDeadlineDelta, displayPresentDelta, deadlineDelta});
+}
+
+void SurfaceFrame::onPresent(nsecs_t presentTime, int32_t displayFrameJankType, Fps refreshRate,
+ nsecs_t displayDeadlineDelta, nsecs_t displayPresentDelta) {
+ std::scoped_lock lock(mMutex);
+
+ if (mPresentState != PresentState::Presented) {
+ // No need to update dropped buffers
+ return;
+ }
+
+ mActuals.presentTime = presentTime;
+ nsecs_t deadlineDelta = 0;
+
+ classifyJankLocked(displayFrameJankType, refreshRate, deadlineDelta);
+
+ if (mPredictionState != PredictionState::None) {
+ // Only update janky frames if the app used vsync predictions
+ mTimeStats->incrementJankyFrames({refreshRate, mRenderRate, mOwnerUid, mLayerName,
+ mJankType, displayDeadlineDelta, displayPresentDelta,
+ deadlineDelta});
+ }
}
void SurfaceFrame::tracePredictions(int64_t displayFrameToken) const {
@@ -826,25 +835,28 @@
mSurfaceFlingerActuals.endTime = actualEndTime;
}
-void FrameTimeline::DisplayFrame::onPresent(nsecs_t signalTime) {
- mSurfaceFlingerActuals.presentTime = signalTime;
- if (mPredictionState == PredictionState::Expired) {
- // Cannot do jank classification with expired predictions
+void FrameTimeline::DisplayFrame::classifyJank(nsecs_t& deadlineDelta, nsecs_t& deltaToVsync) {
+ if (mPredictionState == PredictionState::Expired ||
+ mSurfaceFlingerActuals.presentTime == Fence::SIGNAL_TIME_INVALID) {
+ // Cannot do jank classification with expired predictions or invalid signal times.
mJankType = JankType::Unknown;
+ deadlineDelta = -1;
+ deltaToVsync = -1;
return;
}
// Delta between the expected present and the actual present
const nsecs_t presentDelta =
mSurfaceFlingerActuals.presentTime - mSurfaceFlingerPredictions.presentTime;
- const nsecs_t deadlineDelta =
+ deadlineDelta =
mSurfaceFlingerActuals.endTime - (mSurfaceFlingerPredictions.endTime - mHwcDuration);
// How far off was the presentDelta when compared to the vsyncPeriod. Used in checking if there
// was a prediction error or not.
- nsecs_t deltaToVsync = mRefreshRate.getPeriodNsecs() > 0
+ deltaToVsync = mRefreshRate.getPeriodNsecs() > 0
? std::abs(presentDelta) % mRefreshRate.getPeriodNsecs()
: 0;
+
if (std::abs(presentDelta) > mJankClassificationThresholds.presentThreshold) {
mFramePresentMetadata = presentDelta > 0 ? FramePresentMetadata::LatePresent
: FramePresentMetadata::EarlyPresent;
@@ -922,6 +934,14 @@
mJankType = JankType::Unknown;
}
}
+}
+
+void FrameTimeline::DisplayFrame::onPresent(nsecs_t signalTime) {
+ mSurfaceFlingerActuals.presentTime = signalTime;
+ nsecs_t deadlineDelta = 0;
+ nsecs_t deltaToVsync = 0;
+ classifyJank(deadlineDelta, deltaToVsync);
+
for (auto& surfaceFrame : mSurfaceFrames) {
surfaceFrame->onPresent(signalTime, mJankType, mRefreshRate, deadlineDelta, deltaToVsync);
}
@@ -1084,11 +1104,9 @@
continue;
}
}
- if (signalTime != Fence::SIGNAL_TIME_INVALID) {
- auto& displayFrame = pendingPresentFence.second;
- displayFrame->onPresent(signalTime);
- displayFrame->trace(mSurfaceFlingerPid);
- }
+ auto& displayFrame = pendingPresentFence.second;
+ displayFrame->onPresent(signalTime);
+ displayFrame->trace(mSurfaceFlingerPid);
mPendingPresentFences.erase(mPendingPresentFences.begin() + static_cast<int>(i));
--i;
diff --git a/services/surfaceflinger/FrameTimeline/FrameTimeline.h b/services/surfaceflinger/FrameTimeline/FrameTimeline.h
index 7c6a0cc..b66e02a 100644
--- a/services/surfaceflinger/FrameTimeline/FrameTimeline.h
+++ b/services/surfaceflinger/FrameTimeline/FrameTimeline.h
@@ -216,6 +216,8 @@
private:
void tracePredictions(int64_t displayFrameToken) const;
void traceActuals(int64_t displayFrameToken) const;
+ void classifyJankLocked(int32_t displayFrameJankType, const Fps& refreshRate,
+ nsecs_t& deadlineDelta) REQUIRES(mMutex);
const int64_t mToken;
const int32_t mInputEventId;
@@ -355,7 +357,7 @@
// Sets the token, vsyncPeriod, predictions and SF start time.
void onSfWakeUp(int64_t token, Fps refreshRate, std::optional<TimelineItem> predictions,
nsecs_t wakeUpTime);
- // Sets the appropriate metadata, classifies the jank and returns the classified jankType.
+ // Sets the appropriate metadata and classifies the jank.
void onPresent(nsecs_t signalTime);
// Adds the provided SurfaceFrame to the current display frame.
void addSurfaceFrame(std::shared_ptr<SurfaceFrame> surfaceFrame);
@@ -383,6 +385,7 @@
void dump(std::string& result, nsecs_t baseTime) const;
void tracePredictions(pid_t surfaceFlingerPid) const;
void traceActuals(pid_t surfaceFlingerPid) const;
+ void classifyJank(nsecs_t& deadlineDelta, nsecs_t& deltaToVsync);
int64_t mToken = FrameTimelineInfo::INVALID_VSYNC_ID;
diff --git a/services/surfaceflinger/tests/unittests/FrameTimelineTest.cpp b/services/surfaceflinger/tests/unittests/FrameTimelineTest.cpp
index f2cb951..81efe0b 100644
--- a/services/surfaceflinger/tests/unittests/FrameTimelineTest.cpp
+++ b/services/surfaceflinger/tests/unittests/FrameTimelineTest.cpp
@@ -420,7 +420,54 @@
EXPECT_EQ(getNumberOfDisplayFrames(), *maxDisplayFrames);
}
+TEST_F(FrameTimelineTest, presentFenceSignaled_invalidSignalTime) {
+ Fps refreshRate = Fps::fromPeriodNsecs(11);
+
+ auto presentFence1 = fenceFactory.createFenceTimeForTest(Fence::NO_FENCE);
+ int64_t surfaceFrameToken1 = mTokenManager->generateTokenForPredictions({10, 20, 60});
+ int64_t sfToken1 = mTokenManager->generateTokenForPredictions({52, 60, 60});
+
+ auto surfaceFrame1 =
+ mFrameTimeline->createSurfaceFrameForToken({surfaceFrameToken1, sInputEventId}, sPidOne,
+ sUidOne, sLayerIdOne, sLayerNameOne,
+ sLayerNameOne);
+ mFrameTimeline->setSfWakeUp(sfToken1, 52, refreshRate);
+ surfaceFrame1->setAcquireFenceTime(20);
+ surfaceFrame1->setPresentState(SurfaceFrame::PresentState::Presented);
+ mFrameTimeline->addSurfaceFrame(surfaceFrame1);
+
+ mFrameTimeline->setSfPresent(59, presentFence1);
+ presentFence1->signalForTest(-1);
+ addEmptyDisplayFrame();
+
+ auto displayFrame0 = getDisplayFrame(0);
+ EXPECT_EQ(displayFrame0->getActuals().presentTime, -1);
+ EXPECT_EQ(displayFrame0->getJankType(), JankType::Unknown);
+ EXPECT_EQ(surfaceFrame1->getActuals().presentTime, -1);
+ EXPECT_EQ(surfaceFrame1->getJankType(), JankType::Unknown);
+}
+
// Tests related to TimeStats
+TEST_F(FrameTimelineTest, presentFenceSignaled_doesNotReportForInvalidTokens) {
+ Fps refreshRate = Fps::fromPeriodNsecs(11);
+ EXPECT_CALL(*mTimeStats, incrementJankyFrames(_)).Times(0);
+ auto presentFence1 = fenceFactory.createFenceTimeForTest(Fence::NO_FENCE);
+ int64_t surfaceFrameToken1 = -1;
+ int64_t sfToken1 = -1;
+
+ auto surfaceFrame1 =
+ mFrameTimeline->createSurfaceFrameForToken({surfaceFrameToken1, sInputEventId}, sPidOne,
+ sUidOne, sLayerIdOne, sLayerNameOne,
+ sLayerNameOne);
+ mFrameTimeline->setSfWakeUp(sfToken1, 52, refreshRate);
+ surfaceFrame1->setAcquireFenceTime(20);
+ surfaceFrame1->setPresentState(SurfaceFrame::PresentState::Presented);
+ mFrameTimeline->addSurfaceFrame(surfaceFrame1);
+ presentFence1->signalForTest(70);
+
+ mFrameTimeline->setSfPresent(59, presentFence1);
+}
+
TEST_F(FrameTimelineTest, presentFenceSignaled_reportsLongSfCpu) {
Fps refreshRate = Fps::fromPeriodNsecs(11);
// Deadline delta is 2ms because, sf's adjusted deadline is 60 - composerTime(3) = 57ms.
@@ -603,6 +650,43 @@
EXPECT_EQ(surfaceFrame1->getJankType(), JankType::AppDeadlineMissed);
}
+TEST_F(FrameTimelineTest, presentFenceSignaled_displayFramePredictionExpiredPresentsSurfaceFrame) {
+ Fps refreshRate = Fps::fromPeriodNsecs(11);
+ Fps renderRate = Fps::fromPeriodNsecs(30);
+
+ EXPECT_CALL(*mTimeStats,
+ incrementJankyFrames(TimeStats::JankyFramesInfo{refreshRate, renderRate, sUidOne,
+ sLayerNameOne, JankType::Unknown,
+ -1, -1, 25}));
+ auto presentFence1 = fenceFactory.createFenceTimeForTest(Fence::NO_FENCE);
+ int64_t surfaceFrameToken1 = mTokenManager->generateTokenForPredictions({10, 20, 60});
+ int64_t sfToken1 = mTokenManager->generateTokenForPredictions({82, 90, 90});
+
+ auto surfaceFrame1 =
+ mFrameTimeline->createSurfaceFrameForToken({surfaceFrameToken1, sInputEventId}, sPidOne,
+ sUidOne, sLayerIdOne, sLayerNameOne,
+ sLayerNameOne);
+ surfaceFrame1->setAcquireFenceTime(45);
+ // Trigger a prediction expiry
+ flushTokens(systemTime() + maxTokenRetentionTime);
+ mFrameTimeline->setSfWakeUp(sfToken1, 52, refreshRate);
+
+ surfaceFrame1->setPresentState(SurfaceFrame::PresentState::Presented);
+ surfaceFrame1->setRenderRate(renderRate);
+ mFrameTimeline->addSurfaceFrame(surfaceFrame1);
+ presentFence1->signalForTest(90);
+ mFrameTimeline->setSfPresent(86, presentFence1);
+
+ auto displayFrame = getDisplayFrame(0);
+ EXPECT_EQ(displayFrame->getJankType(), JankType::Unknown);
+ EXPECT_EQ(displayFrame->getFrameStartMetadata(), FrameStartMetadata::UnknownStart);
+ EXPECT_EQ(displayFrame->getFrameReadyMetadata(), FrameReadyMetadata::UnknownFinish);
+ EXPECT_EQ(displayFrame->getFramePresentMetadata(), FramePresentMetadata::UnknownPresent);
+
+ EXPECT_EQ(surfaceFrame1->getActuals().presentTime, 90);
+ EXPECT_EQ(surfaceFrame1->getJankType(), JankType::Unknown);
+}
+
/*
* Tracing Tests
*