Return janktype properly for dropped frames
In SurfaceFrame::getJankType(), a dropped frame won't have any present
time set and will return std::nullopt currently. This behavior can
result in a memory leak in releasePendingBuffer. Whenever SurfaceFrames
are created, they are added to a deque 'mPendingJankClassification'.
This deque is popped from the front as and when the frames are
presented. However, if a dropped frame is in the front of the deque, the
deque keeps growing in size to infinity. Since getJankType() reports a
std::nullopt, it assumes that the frame hasn't been presented yet and
keeps waiting for it.
Bug: 179701581
Test: libsurfaceflinger_unittest
Change-Id: I8ad1ed07222d39e4f98e4f7f3178db9fe52ea712
diff --git a/services/surfaceflinger/FrameTimeline/FrameTimeline.cpp b/services/surfaceflinger/FrameTimeline/FrameTimeline.cpp
index eb0c724..0a40388 100644
--- a/services/surfaceflinger/FrameTimeline/FrameTimeline.cpp
+++ b/services/surfaceflinger/FrameTimeline/FrameTimeline.cpp
@@ -327,7 +327,12 @@
std::optional<int32_t> SurfaceFrame::getJankType() const {
std::scoped_lock lock(mMutex);
+ if (mPresentState == PresentState::Dropped) {
+ // Return no jank if it's a dropped frame since we cannot attribute a jank to a it.
+ return JankType::None;
+ }
if (mActuals.presentTime == 0) {
+ // Frame hasn't been presented yet.
return std::nullopt;
}
return mJankType;
diff --git a/services/surfaceflinger/tests/unittests/TransactionSurfaceFrameTest.cpp b/services/surfaceflinger/tests/unittests/TransactionSurfaceFrameTest.cpp
index aa6798d..a6a465e 100644
--- a/services/surfaceflinger/tests/unittests/TransactionSurfaceFrameTest.cpp
+++ b/services/surfaceflinger/tests/unittests/TransactionSurfaceFrameTest.cpp
@@ -328,6 +328,41 @@
EXPECT_EQ(PresentState::Presented, bufferlessSurfaceFrame2->getPresentState());
EXPECT_EQ(12, bufferlessSurfaceFrame2->getActuals().endTime);
}
+
+ void PendingSurfaceFramesRemovedAfterClassification() {
+ sp<BufferStateLayer> layer = createBufferStateLayer();
+
+ sp<Fence> fence1(new Fence());
+ auto acquireFence1 = fenceFactory.createFenceTimeForTest(fence1);
+ sp<GraphicBuffer> buffer1{new GraphicBuffer(1, 1, HAL_PIXEL_FORMAT_RGBA_8888, 1, 0)};
+ layer->setBuffer(buffer1, fence1, 10, 20, false, mClientCache, 1, std::nullopt,
+ {/*vsyncId*/ 1, /*inputEventId*/ 0});
+ ASSERT_NE(nullptr, layer->mCurrentState.bufferSurfaceFrameTX);
+ const auto droppedSurfaceFrame = layer->mCurrentState.bufferSurfaceFrameTX;
+
+ sp<Fence> fence2(new Fence());
+ auto acquireFence2 = fenceFactory.createFenceTimeForTest(fence2);
+ sp<GraphicBuffer> buffer2{new GraphicBuffer(1, 1, HAL_PIXEL_FORMAT_RGBA_8888, 1, 0)};
+ layer->setBuffer(buffer2, fence2, 10, 20, false, mClientCache, 1, std::nullopt,
+ {/*vsyncId*/ 1, /*inputEventId*/ 0});
+ acquireFence2->signalForTest(12);
+
+ ASSERT_NE(nullptr, layer->mCurrentState.bufferSurfaceFrameTX);
+ auto& presentedSurfaceFrame = layer->mCurrentState.bufferSurfaceFrameTX;
+
+ commitTransaction(layer.get());
+ bool computeVisisbleRegions;
+ layer->updateTexImage(computeVisisbleRegions, 15, 0);
+
+ // Both the droppedSurfaceFrame and presentedSurfaceFrame should be in
+ // pendingJankClassifications.
+ EXPECT_EQ(2u, layer->mPendingJankClassifications.size());
+ presentedSurfaceFrame->onPresent(20, JankType::None, Fps::fromPeriodNsecs(11),
+ /*displayDeadlineDelta*/ 0, /*displayPresentDelta*/ 0);
+ layer->releasePendingBuffer(25);
+
+ EXPECT_EQ(0u, layer->mPendingJankClassifications.size());
+ }
};
TEST_F(TransactionSurfaceFrameTest, PresentedBufferlessSurfaceFrame) {
@@ -364,4 +399,7 @@
MergePendingStates_BufferlessSurfaceFramesWithOverlappingToken();
}
+TEST_F(TransactionSurfaceFrameTest, PendingSurfaceFramesRemovedAfterClassification) {
+ PendingSurfaceFramesRemovedAfterClassification();
+}
} // namespace android
\ No newline at end of file