SF: explicitly mark surface frame without a composite as non janky
otherwise these surface frames waits in the current display frame
for the next composite and they are marked as janky, even though
they had no affect on the displayed frame.
Bug: 340633280
Test: android.platform.test.scenario.sysui.people.PeopleSpaceActivityMicrobenchmark#startPeopleSpaceActivity
Change-Id: I2774e88627e39bce18c04af4b20e2f6973121a42
diff --git a/services/surfaceflinger/FrameTimeline/FrameTimeline.cpp b/services/surfaceflinger/FrameTimeline/FrameTimeline.cpp
index d0e2d7a..8369a1e 100644
--- a/services/surfaceflinger/FrameTimeline/FrameTimeline.cpp
+++ b/services/surfaceflinger/FrameTimeline/FrameTimeline.cpp
@@ -681,6 +681,15 @@
}
}
+void SurfaceFrame::onCommitNotComposited(Fps refreshRate, Fps displayFrameRenderRate) {
+ std::scoped_lock lock(mMutex);
+
+ mDisplayFrameRenderRate = displayFrameRenderRate;
+ mActuals.presentTime = mPredictions.presentTime;
+ nsecs_t deadlineDelta = 0;
+ classifyJankLocked(JankType::None, refreshRate, displayFrameRenderRate, deadlineDelta);
+}
+
void SurfaceFrame::tracePredictions(int64_t displayFrameToken, nsecs_t monoBootOffset) const {
int64_t expectedTimelineCookie = mTraceCookieCounter.getCookieForTracing();
@@ -912,6 +921,15 @@
finalizeCurrentDisplayFrame();
}
+void FrameTimeline::onCommitNotComposited() {
+ ATRACE_CALL();
+ std::scoped_lock lock(mMutex);
+ mCurrentDisplayFrame->onCommitNotComposited();
+ mCurrentDisplayFrame.reset();
+ mCurrentDisplayFrame = std::make_shared<DisplayFrame>(mTimeStats, mJankClassificationThresholds,
+ &mTraceCookieCounter);
+}
+
void FrameTimeline::DisplayFrame::addSurfaceFrame(std::shared_ptr<SurfaceFrame> surfaceFrame) {
mSurfaceFrames.push_back(surfaceFrame);
}
@@ -1094,6 +1112,12 @@
}
}
+void FrameTimeline::DisplayFrame::onCommitNotComposited() {
+ for (auto& surfaceFrame : mSurfaceFrames) {
+ surfaceFrame->onCommitNotComposited(mRefreshRate, mRenderRate);
+ }
+}
+
void FrameTimeline::DisplayFrame::tracePredictions(pid_t surfaceFlingerPid,
nsecs_t monoBootOffset) const {
int64_t expectedTimelineCookie = mTraceCookieCounter.getCookieForTracing();
diff --git a/services/surfaceflinger/FrameTimeline/FrameTimeline.h b/services/surfaceflinger/FrameTimeline/FrameTimeline.h
index a76f7d4..21bc95a 100644
--- a/services/surfaceflinger/FrameTimeline/FrameTimeline.h
+++ b/services/surfaceflinger/FrameTimeline/FrameTimeline.h
@@ -204,6 +204,8 @@
void onPresent(nsecs_t presentTime, int32_t displayFrameJankType, Fps refreshRate,
Fps displayFrameRenderRate, nsecs_t displayDeadlineDelta,
nsecs_t displayPresentDelta);
+ // Sets the frame as none janky as there was no real display frame.
+ void onCommitNotComposited(Fps refreshRate, Fps displayFrameRenderRate);
// All the timestamps are dumped relative to the baseTime
void dump(std::string& result, const std::string& indent, nsecs_t baseTime) const;
// Dumps only the layer, token, is buffer, jank metadata, prediction and present states.
@@ -318,6 +320,10 @@
virtual void setSfPresent(nsecs_t sfPresentTime, const std::shared_ptr<FenceTime>& presentFence,
const std::shared_ptr<FenceTime>& gpuFence) = 0;
+ // Tells FrameTimeline that a frame was committed but not composited. This is used to flush
+ // all the associated surface frames.
+ virtual void onCommitNotComposited() = 0;
+
// Args:
// -jank : Dumps only the Display Frames that are either janky themselves
// or contain janky Surface Frames.
@@ -390,6 +396,8 @@
std::optional<TimelineItem> predictions, nsecs_t wakeUpTime);
// Sets the appropriate metadata and classifies the jank.
void onPresent(nsecs_t signalTime, nsecs_t previousPresentTime);
+ // Flushes all the surface frames as those were not generating any actual display frames.
+ void onCommitNotComposited();
// Adds the provided SurfaceFrame to the current display frame.
void addSurfaceFrame(std::shared_ptr<SurfaceFrame> surfaceFrame);
@@ -475,6 +483,7 @@
void setSfWakeUp(int64_t token, nsecs_t wakeupTime, Fps refreshRate, Fps renderRate) override;
void setSfPresent(nsecs_t sfPresentTime, const std::shared_ptr<FenceTime>& presentFence,
const std::shared_ptr<FenceTime>& gpuFence = FenceTime::NO_FENCE) override;
+ void onCommitNotComposited() override;
void parseArgs(const Vector<String16>& args, std::string& result) override;
void setMaxDisplayFrames(uint32_t size) override;
float computeFps(const std::unordered_set<int32_t>& layerIds) override;
diff --git a/services/surfaceflinger/Scheduler/ISchedulerCallback.h b/services/surfaceflinger/Scheduler/ISchedulerCallback.h
index 9f4f5b6..43cdb5e 100644
--- a/services/surfaceflinger/Scheduler/ISchedulerCallback.h
+++ b/services/surfaceflinger/Scheduler/ISchedulerCallback.h
@@ -32,6 +32,7 @@
virtual void onChoreographerAttached() = 0;
virtual void onExpectedPresentTimePosted(TimePoint, ftl::NonNull<DisplayModePtr>,
Fps renderRate) = 0;
+ virtual void onCommitNotComposited(PhysicalDisplayId pacesetterDisplayId) = 0;
protected:
~ISchedulerCallback() = default;
diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp
index 005ec05..f72e7a0 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.cpp
+++ b/services/surfaceflinger/Scheduler/Scheduler.cpp
@@ -221,6 +221,7 @@
if (FlagManager::getInstance().vrr_config()) {
compositor.sendNotifyExpectedPresentHint(pacesetterPtr->displayId);
}
+ mSchedulerCallback.onCommitNotComposited(pacesetterPtr->displayId);
return;
}
}
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index b3ddb57..410c420 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -4405,6 +4405,12 @@
scheduleNotifyExpectedPresentHint(displayId);
}
+void SurfaceFlinger::onCommitNotComposited(PhysicalDisplayId pacesetterDisplayId) {
+ if (FlagManager::getInstance().commit_not_composited()) {
+ mFrameTimeline->onCommitNotComposited();
+ }
+}
+
void SurfaceFlinger::initScheduler(const sp<const DisplayDevice>& display) {
using namespace scheduler;
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 8474515..edcc8d3 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -700,6 +700,8 @@
void onChoreographerAttached() override;
void onExpectedPresentTimePosted(TimePoint expectedPresentTime, ftl::NonNull<DisplayModePtr>,
Fps renderRate) override;
+ void onCommitNotComposited(PhysicalDisplayId pacesetterDisplayId) override
+ REQUIRES(kMainThreadContext);
// ICEPowerCallback overrides:
void notifyCpuLoadUp() override;
diff --git a/services/surfaceflinger/common/FlagManager.cpp b/services/surfaceflinger/common/FlagManager.cpp
index 45b3290..e73cf5d 100644
--- a/services/surfaceflinger/common/FlagManager.cpp
+++ b/services/surfaceflinger/common/FlagManager.cpp
@@ -144,6 +144,7 @@
DUMP_READ_ONLY_FLAG(deprecate_vsync_sf);
DUMP_READ_ONLY_FLAG(allow_n_vsyncs_in_targeter);
DUMP_READ_ONLY_FLAG(detached_mirror);
+ DUMP_READ_ONLY_FLAG(commit_not_composited);
#undef DUMP_READ_ONLY_FLAG
#undef DUMP_SERVER_FLAG
@@ -238,6 +239,7 @@
FLAG_MANAGER_READ_ONLY_FLAG(deprecate_vsync_sf, "");
FLAG_MANAGER_READ_ONLY_FLAG(allow_n_vsyncs_in_targeter, "");
FLAG_MANAGER_READ_ONLY_FLAG(detached_mirror, "");
+FLAG_MANAGER_READ_ONLY_FLAG(commit_not_composited, "");
/// 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 592e774..327cc4a 100644
--- a/services/surfaceflinger/common/include/common/FlagManager.h
+++ b/services/surfaceflinger/common/include/common/FlagManager.h
@@ -83,6 +83,7 @@
bool deprecate_vsync_sf() const;
bool allow_n_vsyncs_in_targeter() const;
bool detached_mirror() const;
+ bool commit_not_composited() const;
protected:
// overridden for unit tests
diff --git a/services/surfaceflinger/surfaceflinger_flags_new.aconfig b/services/surfaceflinger/surfaceflinger_flags_new.aconfig
index 4d3195d..f1ec3e1 100644
--- a/services/surfaceflinger/surfaceflinger_flags_new.aconfig
+++ b/services/surfaceflinger/surfaceflinger_flags_new.aconfig
@@ -22,6 +22,17 @@
} # ce_fence_promise
flag {
+ name: "commit_not_composited"
+ namespace: "core_graphics"
+ description: "mark frames as non janky if the transaction resulted in no composition"
+ bug: "340633280"
+ is_fixed_read_only: true
+ metadata {
+ purpose: PURPOSE_BUGFIX
+ }
+ } # commit_not_composited
+
+ flag {
name: "deprecate_vsync_sf"
namespace: "core_graphics"
description: "Depracate eVsyncSourceSurfaceFlinger and use vsync_app everywhere"
diff --git a/services/surfaceflinger/tests/unittests/FrameTimelineTest.cpp b/services/surfaceflinger/tests/unittests/FrameTimelineTest.cpp
index ddc3967..9fd687c 100644
--- a/services/surfaceflinger/tests/unittests/FrameTimelineTest.cpp
+++ b/services/surfaceflinger/tests/unittests/FrameTimelineTest.cpp
@@ -341,6 +341,57 @@
EXPECT_NE(surfaceFrame2->getJankSeverityType(), std::nullopt);
}
+TEST_F(FrameTimelineTest, displayFrameSkippedComposition) {
+ // Layer specific increment
+ EXPECT_CALL(*mTimeStats, incrementJankyFrames(_)).Times(1);
+ auto presentFence1 = fenceFactory.createFenceTimeForTest(Fence::NO_FENCE);
+ int64_t surfaceFrameToken1 = mTokenManager->generateTokenForPredictions({10, 20, 30});
+ int64_t sfToken1 = mTokenManager->generateTokenForPredictions({22, 26, 30});
+ FrameTimelineInfo ftInfo;
+ ftInfo.vsyncId = surfaceFrameToken1;
+ ftInfo.inputEventId = sInputEventId;
+ auto surfaceFrame1 =
+ mFrameTimeline->createSurfaceFrameForToken(ftInfo, sPidOne, sUidOne, sLayerIdOne,
+ sLayerNameOne, sLayerNameOne,
+ /*isBuffer*/ true, sGameMode);
+ auto surfaceFrame2 =
+ mFrameTimeline->createSurfaceFrameForToken(ftInfo, sPidOne, sUidOne, sLayerIdTwo,
+ sLayerNameTwo, sLayerNameTwo,
+ /*isBuffer*/ true, sGameMode);
+
+ mFrameTimeline->setSfWakeUp(sfToken1, 22, RR_11, RR_11);
+ surfaceFrame1->setPresentState(SurfaceFrame::PresentState::Presented);
+ mFrameTimeline->addSurfaceFrame(surfaceFrame1);
+ mFrameTimeline->onCommitNotComposited();
+
+ EXPECT_EQ(surfaceFrame1->getActuals().presentTime, 30);
+ ASSERT_NE(surfaceFrame1->getJankType(), std::nullopt);
+ EXPECT_EQ(*surfaceFrame1->getJankType(), JankType::None);
+ ASSERT_NE(surfaceFrame1->getJankSeverityType(), std::nullopt);
+ EXPECT_EQ(*surfaceFrame1->getJankSeverityType(), JankSeverityType::None);
+
+ mFrameTimeline->setSfWakeUp(sfToken1, 22, RR_11, RR_11);
+ surfaceFrame2->setPresentState(SurfaceFrame::PresentState::Presented);
+ mFrameTimeline->addSurfaceFrame(surfaceFrame2);
+ mFrameTimeline->setSfPresent(26, presentFence1);
+
+ auto displayFrame = getDisplayFrame(0);
+ auto& presentedSurfaceFrame2 = getSurfaceFrame(0, 0);
+ presentFence1->signalForTest(42);
+
+ // Fences haven't been flushed yet, so it should be 0
+ EXPECT_EQ(displayFrame->getActuals().presentTime, 0);
+ EXPECT_EQ(presentedSurfaceFrame2.getActuals().presentTime, 0);
+
+ addEmptyDisplayFrame();
+
+ // Fences have flushed, so the present timestamps should be updated
+ EXPECT_EQ(displayFrame->getActuals().presentTime, 42);
+ EXPECT_EQ(presentedSurfaceFrame2.getActuals().presentTime, 42);
+ EXPECT_NE(surfaceFrame2->getJankType(), std::nullopt);
+ EXPECT_NE(surfaceFrame2->getJankSeverityType(), std::nullopt);
+}
+
TEST_F(FrameTimelineTest, displayFramesSlidingWindowMovesAfterLimit) {
// Insert kMaxDisplayFrames' count of DisplayFrames to fill the deque
int frameTimeFactor = 0;
diff --git a/services/surfaceflinger/tests/unittests/mock/MockSchedulerCallback.h b/services/surfaceflinger/tests/unittests/mock/MockSchedulerCallback.h
index 4ca0542..dec5fa5 100644
--- a/services/surfaceflinger/tests/unittests/mock/MockSchedulerCallback.h
+++ b/services/surfaceflinger/tests/unittests/mock/MockSchedulerCallback.h
@@ -30,6 +30,7 @@
MOCK_METHOD(void, onChoreographerAttached, (), (override));
MOCK_METHOD(void, onExpectedPresentTimePosted, (TimePoint, ftl::NonNull<DisplayModePtr>, Fps),
(override));
+ MOCK_METHOD(void, onCommitNotComposited, (PhysicalDisplayId), (override));
};
struct NoOpSchedulerCallback final : ISchedulerCallback {
@@ -39,6 +40,7 @@
void triggerOnFrameRateOverridesChanged() override {}
void onChoreographerAttached() override {}
void onExpectedPresentTimePosted(TimePoint, ftl::NonNull<DisplayModePtr>, Fps) override {}
+ void onCommitNotComposited(PhysicalDisplayId) override {}
};
} // namespace android::scheduler::mock