Skip to content

Commit e524dd9

Browse files
author
Melody Hsu
committed
Recover from buffer stuffing for canned animations
Buffer stuffing occurs when SurfaceFlinger misses a frame, but the client continues to produce buffers at the same rate, causing a greater risk for jank to occur. Recovery is achieved for canned animations by adjusting the animation timeline on the client side so that SurfaceFlinger is no longer behind. Use SF backdoor command 1045 to inject jank. Usage: adb shell service call SurfaceFlinger 1045 f 1 Bug: b/294922229 Test: atest EventThreadTest Test: presubmit, manually check perfetto traces Flag: android.view.flags.buffer_stuffing_recovery Change-Id: I38f0eb3d6ef1331e07d6022fa3a0e16c556ba06f
1 parent 027013d commit e524dd9

15 files changed

Lines changed: 192 additions & 10 deletions

libs/gui/BufferStuffing.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
### Buffer Stuffing and Recovery ###
2+
3+
Buffer stuffing happens on the client side when SurfaceFlinger misses a frame, but the client continues producing buffers at the same rate. This could occur anytime when SurfaceFlinger does not meet the expected timeline’s deadline to finish composing a frame. As a result, SurfaceFlinger cannot yet release the buffer for the frame that it missed and the client has one less buffer to render into. The client may then run out of buffers or have to wait for buffer release callbacks, increasing the chances of janking when clients render multiple windows.
4+
5+
Recovery is implemented by first detecting when buffer stuffing occurs and ensuring that the elevated buffer counts in the server are from a relevant SurfaceControl (is a ViewRootImpl). Other SurfaceControl buffer producers such as games, media, and camera have other reasons for expectedly increased buffer counts, which do not need buffer stuffing recovery.
6+
7+
The actual recovery adjusts the animation timeline in the Choreographer so that the client deadlines for subsequent frames are moved forward in time by one frame. This approach adjusts the client buffer production timeline such that SurfaceFlinger does not fall behind when it misses a frame because the client will simply match its frame production rate with SurfaceFlinger. Ordinarily, buffer stuffing is problematic because the client continues producing buffers when SurfaceFlinger is behind. However, if the client delays producing its buffers to match SurfaceFlinger’s rate, the animation has new frame deadlines that can be reasonably met. The animation is effectively paused for one frame longer than originally intended, and continues the remainder of the animation normally.

libs/gui/include/gui/DisplayEventReceiver.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ class DisplayEventReceiver {
119119
HdcpLevelsChange hdcpLevelsChange;
120120
};
121121
};
122-
static_assert(sizeof(Event) == 216);
122+
static_assert(sizeof(Event) == 224);
123123

124124
public:
125125
/*

libs/gui/include/gui/LayerState.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,10 @@ struct layer_state_t {
170170
// Sets a property on this layer indicating that its visible region should be considered
171171
// when computing TrustedPresentation Thresholds.
172172
eCanOccludePresentation = 0x1000,
173+
// Indicates that the SurfaceControl should recover from buffer stuffing when
174+
// possible. This is the case when the SurfaceControl is the root SurfaceControl
175+
// owned by ViewRootImpl.
176+
eRecoverableFromBufferStuffing = 0x2000,
173177
};
174178

175179
enum {

libs/gui/include/gui/VsyncEventData.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,9 @@ struct VsyncEventData {
3636
// Size of frame timelines provided by the platform; max is kFrameTimelinesCapacity.
3737
uint32_t frameTimelinesLength;
3838

39+
// Number of queued buffers to indicate if buffer stuffing mode is detected.
40+
uint32_t numberQueuedBuffers;
41+
3942
struct alignas(8) FrameTimeline {
4043
// The Vsync Id corresponsing to this vsync event. This will be used to
4144
// populate ISurfaceComposer::setFrameTimelineVsync and

libs/gui/tests/DisplayEventStructLayout_test.cpp

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,15 +36,16 @@ TEST(DisplayEventStructLayoutTest, TestEventAlignment) {
3636
CHECK_OFFSET(DisplayEventReceiver::Event::VSync, vsyncData.frameInterval, 8);
3737
CHECK_OFFSET(DisplayEventReceiver::Event::VSync, vsyncData.preferredFrameTimelineIndex, 16);
3838
CHECK_OFFSET(DisplayEventReceiver::Event::VSync, vsyncData.frameTimelinesLength, 20);
39-
CHECK_OFFSET(DisplayEventReceiver::Event::VSync, vsyncData.frameTimelines, 24);
40-
CHECK_OFFSET(DisplayEventReceiver::Event::VSync, vsyncData.frameTimelines[0].vsyncId, 24);
39+
CHECK_OFFSET(DisplayEventReceiver::Event::VSync, vsyncData.numberQueuedBuffers, 24);
40+
CHECK_OFFSET(DisplayEventReceiver::Event::VSync, vsyncData.frameTimelines, 32);
41+
CHECK_OFFSET(DisplayEventReceiver::Event::VSync, vsyncData.frameTimelines[0].vsyncId, 32);
4142
CHECK_OFFSET(DisplayEventReceiver::Event::VSync, vsyncData.frameTimelines[0].deadlineTimestamp,
42-
32);
43+
40);
4344
CHECK_OFFSET(DisplayEventReceiver::Event::VSync,
44-
vsyncData.frameTimelines[0].expectedPresentationTime, 40);
45+
vsyncData.frameTimelines[0].expectedPresentationTime, 48);
4546
// Also test the offsets of the last frame timeline. A loop is not used because the non-const
4647
// index cannot be used in static_assert.
47-
const int lastFrameTimelineOffset = /* Start of array */ 24 +
48+
const int lastFrameTimelineOffset = /* Start of array */ 32 +
4849
(VsyncEventData::kFrameTimelinesCapacity - 1) * /* Size of FrameTimeline */ 24;
4950
CHECK_OFFSET(DisplayEventReceiver::Event::VSync,
5051
vsyncData.frameTimelines[VsyncEventData::kFrameTimelinesCapacity - 1].vsyncId,

services/surfaceflinger/FrameTimeline/FrameTimeline.cpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
#include <cinttypes>
3030
#include <numeric>
3131
#include <unordered_set>
32+
#include <vector>
3233

3334
#include "../Jank/JankTracker.h"
3435

@@ -997,6 +998,11 @@ void FrameTimeline::setSfPresent(nsecs_t sfPresentTime,
997998
finalizeCurrentDisplayFrame();
998999
}
9991000

1001+
const std::vector<std::shared_ptr<frametimeline::SurfaceFrame>>& FrameTimeline::getPresentFrames()
1002+
const {
1003+
return mPresentFrames;
1004+
}
1005+
10001006
void FrameTimeline::onCommitNotComposited() {
10011007
SFTRACE_CALL();
10021008
std::scoped_lock lock(mMutex);
@@ -1492,6 +1498,7 @@ void FrameTimeline::flushPendingPresentFences() {
14921498
mPendingPresentFences.erase(mPendingPresentFences.begin());
14931499
}
14941500

1501+
mPresentFrames.clear();
14951502
for (size_t i = 0; i < mPendingPresentFences.size(); i++) {
14961503
const auto& pendingPresentFence = mPendingPresentFences[i];
14971504
nsecs_t signalTime = Fence::SIGNAL_TIME_INVALID;
@@ -1504,6 +1511,13 @@ void FrameTimeline::flushPendingPresentFences() {
15041511

15051512
auto& displayFrame = pendingPresentFence.second;
15061513
displayFrame->onPresent(signalTime, mPreviousActualPresentTime);
1514+
1515+
// Surface frames have been jank classified and can be provided to caller
1516+
// to detect if buffer stuffing is occurring.
1517+
for (const auto& frame : displayFrame->getSurfaceFrames()) {
1518+
mPresentFrames.push_back(frame);
1519+
}
1520+
15071521
mPreviousPredictionPresentTime =
15081522
displayFrame->trace(mSurfaceFlingerPid, monoBootOffset,
15091523
mPreviousPredictionPresentTime, mFilterFramesBeforeTraceStarts);

services/surfaceflinger/FrameTimeline/FrameTimeline.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -323,6 +323,11 @@ class FrameTimeline {
323323
virtual void setSfPresent(nsecs_t sfPresentTime, const std::shared_ptr<FenceTime>& presentFence,
324324
const std::shared_ptr<FenceTime>& gpuFence) = 0;
325325

326+
// Provides surface frames that have already been jank classified in the most recent
327+
// flush of pending present fences. This allows buffer stuffing detection from SF.
328+
virtual const std::vector<std::shared_ptr<frametimeline::SurfaceFrame>>& getPresentFrames()
329+
const = 0;
330+
326331
// Tells FrameTimeline that a frame was committed but not composited. This is used to flush
327332
// all the associated surface frames.
328333
virtual void onCommitNotComposited() = 0;
@@ -497,6 +502,8 @@ class FrameTimeline : public android::frametimeline::FrameTimeline {
497502
void setSfWakeUp(int64_t token, nsecs_t wakeupTime, Fps refreshRate, Fps renderRate) override;
498503
void setSfPresent(nsecs_t sfPresentTime, const std::shared_ptr<FenceTime>& presentFence,
499504
const std::shared_ptr<FenceTime>& gpuFence = FenceTime::NO_FENCE) override;
505+
const std::vector<std::shared_ptr<frametimeline::SurfaceFrame>>& getPresentFrames()
506+
const override;
500507
void onCommitNotComposited() override;
501508
void parseArgs(const Vector<String16>& args, std::string& result) override;
502509
void setMaxDisplayFrames(uint32_t size) override;
@@ -543,6 +550,9 @@ class FrameTimeline : public android::frametimeline::FrameTimeline {
543550
// display frame, this is a good starting size for the vector so that we can avoid the
544551
// internal vector resizing that happens with push_back.
545552
static constexpr uint32_t kNumSurfaceFramesInitial = 10;
553+
// Presented surface frames that have been jank classified and can
554+
// indicate of potential buffer stuffing.
555+
std::vector<std::shared_ptr<frametimeline::SurfaceFrame>> mPresentFrames;
546556
};
547557

548558
} // namespace impl

services/surfaceflinger/Scheduler/EventThread.cpp

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,8 @@
4444

4545
#include <common/FlagManager.h>
4646
#include <scheduler/VsyncConfig.h>
47-
#include "DisplayHardware/DisplayMode.h"
4847
#include "FrameTimeline.h"
4948
#include "VSyncDispatch.h"
50-
#include "VSyncTracker.h"
5149

5250
#include "EventThread.h"
5351

@@ -472,6 +470,14 @@ void EventThread::onHdcpLevelsChanged(PhysicalDisplayId displayId, int32_t conne
472470
mCondition.notify_all();
473471
}
474472

473+
// Merge lists of buffer stuffed Uids
474+
void EventThread::addBufferStuffedUids(BufferStuffingMap bufferStuffedUids) {
475+
std::lock_guard<std::mutex> lock(mMutex);
476+
for (auto& [uid, count] : bufferStuffedUids) {
477+
mBufferStuffedUids.emplace_or_replace(uid, count);
478+
}
479+
}
480+
475481
void EventThread::threadMain(std::unique_lock<std::mutex>& lock) {
476482
DisplayEventConsumers consumers;
477483

@@ -701,6 +707,10 @@ void EventThread::generateFrameTimeline(VsyncEventData& outVsyncEventData, nsecs
701707

702708
void EventThread::dispatchEvent(const DisplayEventReceiver::Event& event,
703709
const DisplayEventConsumers& consumers) {
710+
// List of Uids that have been sent vsync data with queued buffer count.
711+
// Used to keep track of which Uids can be removed from the map of
712+
// buffer stuffed clients.
713+
ftl::SmallVector<uid_t, 10> uidsPostedQueuedBuffers;
704714
for (const auto& consumer : consumers) {
705715
DisplayEventReceiver::Event copy = event;
706716
if (event.header.type == DisplayEventReceiver::DISPLAY_EVENT_VSYNC) {
@@ -710,6 +720,13 @@ void EventThread::dispatchEvent(const DisplayEventReceiver::Event& event,
710720
event.vsync.vsyncData.preferredExpectedPresentationTime(),
711721
event.vsync.vsyncData.preferredDeadlineTimestamp());
712722
}
723+
auto it = mBufferStuffedUids.find(consumer->mOwnerUid);
724+
if (it != mBufferStuffedUids.end()) {
725+
copy.vsync.vsyncData.numberQueuedBuffers = it->second;
726+
uidsPostedQueuedBuffers.emplace_back(consumer->mOwnerUid);
727+
} else {
728+
copy.vsync.vsyncData.numberQueuedBuffers = 0;
729+
}
713730
switch (consumer->postEvent(copy)) {
714731
case NO_ERROR:
715732
break;
@@ -725,6 +742,12 @@ void EventThread::dispatchEvent(const DisplayEventReceiver::Event& event,
725742
removeDisplayEventConnectionLocked(consumer);
726743
}
727744
}
745+
// The clients that have already received the queued buffer count
746+
// can be removed from the buffer stuffed Uid list to avoid
747+
// being sent duplicate messages.
748+
for (auto uid : uidsPostedQueuedBuffers) {
749+
mBufferStuffedUids.erase(uid);
750+
}
728751
if (event.header.type == DisplayEventReceiver::DISPLAY_EVENT_VSYNC &&
729752
FlagManager::getInstance().vrr_config()) {
730753
mLastCommittedVsyncTime =

services/surfaceflinger/Scheduler/EventThread.h

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@
3232
#include <thread>
3333
#include <vector>
3434

35-
#include "DisplayHardware/DisplayMode.h"
3635
#include "TracedOrdinal.h"
3736
#include "VSyncDispatch.h"
3837
#include "VsyncSchedule.h"
@@ -55,6 +54,7 @@ using gui::VsyncEventData;
5554
// ---------------------------------------------------------------------------
5655

5756
using FrameRateOverride = DisplayEventReceiver::Event::FrameRateOverride;
57+
using BufferStuffingMap = ftl::SmallMap<uid_t, uint32_t, 10>;
5858

5959
enum class VSyncRequest {
6060
None = -2,
@@ -134,6 +134,10 @@ class EventThread {
134134

135135
virtual void onHdcpLevelsChanged(PhysicalDisplayId displayId, int32_t connectedLevel,
136136
int32_t maxLevel) = 0;
137+
138+
// An elevated number of queued buffers in the server is detected. This propagates a
139+
// flag to Choreographer indicating that buffer stuffing recovery should begin.
140+
virtual void addBufferStuffedUids(BufferStuffingMap bufferStuffedUids);
137141
};
138142

139143
struct IEventThreadCallback {
@@ -184,6 +188,8 @@ class EventThread : public android::EventThread {
184188
void onHdcpLevelsChanged(PhysicalDisplayId displayId, int32_t connectedLevel,
185189
int32_t maxLevel) override;
186190

191+
void addBufferStuffedUids(BufferStuffingMap bufferStuffedUids) override;
192+
187193
private:
188194
friend EventThreadTest;
189195

@@ -224,6 +230,10 @@ class EventThread : public android::EventThread {
224230
scheduler::VSyncCallbackRegistration mVsyncRegistration GUARDED_BY(mMutex);
225231
frametimeline::TokenManager* const mTokenManager;
226232

233+
// All consumers that need to recover from buffer stuffing and the number
234+
// of their queued buffers.
235+
BufferStuffingMap mBufferStuffedUids GUARDED_BY(mMutex);
236+
227237
IEventThreadCallback& mCallback;
228238

229239
std::thread mThread;

services/surfaceflinger/Scheduler/Scheduler.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -940,6 +940,11 @@ bool Scheduler::updateFrameRateOverridesLocked(GlobalSignals consideredSignals,
940940
return mFrameRateOverrideMappings.updateFrameRateOverridesByContent(frameRateOverrides);
941941
}
942942

943+
void Scheduler::addBufferStuffedUids(BufferStuffingMap bufferStuffedUids) {
944+
if (!mRenderEventThread) return;
945+
mRenderEventThread->addBufferStuffedUids(std::move(bufferStuffedUids));
946+
}
947+
943948
void Scheduler::promotePacesetterDisplay(PhysicalDisplayId pacesetterId, PromotionParams params) {
944949
std::shared_ptr<VsyncSchedule> pacesetterVsyncSchedule;
945950
{

0 commit comments

Comments
 (0)