diff options
-rw-r--r-- | services/surfaceflinger/FrameTimeline/FrameTimeline.cpp | 13 | ||||
-rw-r--r-- | services/surfaceflinger/FrameTimeline/FrameTimeline.h | 10 | ||||
-rw-r--r-- | services/surfaceflinger/Scheduler/EventThread.cpp | 25 | ||||
-rw-r--r-- | services/surfaceflinger/Scheduler/EventThread.h | 11 | ||||
-rw-r--r-- | services/surfaceflinger/Scheduler/Scheduler.cpp | 5 | ||||
-rw-r--r-- | services/surfaceflinger/Scheduler/Scheduler.h | 4 | ||||
-rw-r--r-- | services/surfaceflinger/SurfaceFlinger.cpp | 30 | ||||
-rw-r--r-- | services/surfaceflinger/SurfaceFlinger.h | 2 | ||||
-rw-r--r-- | services/surfaceflinger/tests/unittests/EventThreadTest.cpp | 69 | ||||
-rw-r--r-- | services/surfaceflinger/tests/unittests/mock/MockEventThread.h | 1 |
10 files changed, 0 insertions, 170 deletions
diff --git a/services/surfaceflinger/FrameTimeline/FrameTimeline.cpp b/services/surfaceflinger/FrameTimeline/FrameTimeline.cpp index 008b0571c3..51d4078987 100644 --- a/services/surfaceflinger/FrameTimeline/FrameTimeline.cpp +++ b/services/surfaceflinger/FrameTimeline/FrameTimeline.cpp @@ -29,7 +29,6 @@ #include <cinttypes> #include <numeric> #include <unordered_set> -#include <vector> #include "../Jank/JankTracker.h" @@ -1005,11 +1004,6 @@ void FrameTimeline::setSfPresent(nsecs_t sfPresentTime, finalizeCurrentDisplayFrame(); } -const std::vector<std::shared_ptr<frametimeline::SurfaceFrame>>& FrameTimeline::getPresentFrames() - const { - return mPresentFrames; -} - void FrameTimeline::onCommitNotComposited() { SFTRACE_CALL(); std::scoped_lock lock(mMutex); @@ -1530,7 +1524,6 @@ void FrameTimeline::flushPendingPresentFences() { mPendingPresentFences.erase(mPendingPresentFences.begin()); } - mPresentFrames.clear(); for (size_t i = 0; i < mPendingPresentFences.size(); i++) { const auto& pendingPresentFence = mPendingPresentFences[i]; nsecs_t signalTime = Fence::SIGNAL_TIME_INVALID; @@ -1544,12 +1537,6 @@ void FrameTimeline::flushPendingPresentFences() { auto& displayFrame = pendingPresentFence.second; displayFrame->onPresent(signalTime, mPreviousActualPresentTime); - // Surface frames have been jank classified and can be provided to caller - // to detect if buffer stuffing is occurring. - for (const auto& frame : displayFrame->getSurfaceFrames()) { - mPresentFrames.push_back(frame); - } - mPreviousPredictionPresentTime = displayFrame->trace(mSurfaceFlingerPid, monoBootOffset, mPreviousPredictionPresentTime, mFilterFramesBeforeTraceStarts); diff --git a/services/surfaceflinger/FrameTimeline/FrameTimeline.h b/services/surfaceflinger/FrameTimeline/FrameTimeline.h index 9fedb57aca..fa83cd8523 100644 --- a/services/surfaceflinger/FrameTimeline/FrameTimeline.h +++ b/services/surfaceflinger/FrameTimeline/FrameTimeline.h @@ -331,11 +331,6 @@ public: virtual void setSfPresent(nsecs_t sfPresentTime, const std::shared_ptr<FenceTime>& presentFence, const std::shared_ptr<FenceTime>& gpuFence) = 0; - // Provides surface frames that have already been jank classified in the most recent - // flush of pending present fences. This allows buffer stuffing detection from SF. - virtual const std::vector<std::shared_ptr<frametimeline::SurfaceFrame>>& getPresentFrames() - const = 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; @@ -513,8 +508,6 @@ public: 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; - const std::vector<std::shared_ptr<frametimeline::SurfaceFrame>>& getPresentFrames() - const override; void onCommitNotComposited() override; void parseArgs(const Vector<String16>& args, std::string& result) override; void setMaxDisplayFrames(uint32_t size) override; @@ -562,9 +555,6 @@ private: // display frame, this is a good starting size for the vector so that we can avoid the // internal vector resizing that happens with push_back. static constexpr uint32_t kNumSurfaceFramesInitial = 10; - // Presented surface frames that have been jank classified and can - // indicate of potential buffer stuffing. - std::vector<std::shared_ptr<frametimeline::SurfaceFrame>> mPresentFrames; }; } // namespace impl diff --git a/services/surfaceflinger/Scheduler/EventThread.cpp b/services/surfaceflinger/Scheduler/EventThread.cpp index c37b9653cb..5390295e9a 100644 --- a/services/surfaceflinger/Scheduler/EventThread.cpp +++ b/services/surfaceflinger/Scheduler/EventThread.cpp @@ -512,14 +512,6 @@ void EventThread::onModeRejected(PhysicalDisplayId displayId, DisplayModeId mode mCondition.notify_all(); } -// Merge lists of buffer stuffed Uids -void EventThread::addBufferStuffedUids(BufferStuffingMap bufferStuffedUids) { - std::lock_guard<std::mutex> lock(mMutex); - for (auto& [uid, count] : bufferStuffedUids) { - mBufferStuffedUids.emplace_or_replace(uid, count); - } -} - void EventThread::threadMain(std::unique_lock<std::mutex>& lock) { DisplayEventConsumers consumers; @@ -761,10 +753,6 @@ void EventThread::generateFrameTimeline(VsyncEventData& outVsyncEventData, nsecs void EventThread::dispatchEvent(const DisplayEventReceiver::Event& event, const DisplayEventConsumers& consumers) { - // List of Uids that have been sent vsync data with queued buffer count. - // Used to keep track of which Uids can be removed from the map of - // buffer stuffed clients. - ftl::SmallVector<uid_t, 10> uidsPostedQueuedBuffers; for (const auto& consumer : consumers) { DisplayEventReceiver::Event copy = event; if (event.header.type == DisplayEventType::DISPLAY_EVENT_VSYNC) { @@ -774,13 +762,6 @@ void EventThread::dispatchEvent(const DisplayEventReceiver::Event& event, event.vsync.vsyncData.preferredExpectedPresentationTime(), event.vsync.vsyncData.preferredDeadlineTimestamp()); } - auto it = mBufferStuffedUids.find(consumer->mOwnerUid); - if (it != mBufferStuffedUids.end()) { - copy.vsync.vsyncData.numberQueuedBuffers = it->second; - uidsPostedQueuedBuffers.emplace_back(consumer->mOwnerUid); - } else { - copy.vsync.vsyncData.numberQueuedBuffers = 0; - } switch (consumer->postEvent(copy)) { case NO_ERROR: break; @@ -796,12 +777,6 @@ void EventThread::dispatchEvent(const DisplayEventReceiver::Event& event, removeDisplayEventConnectionLocked(consumer); } } - // The clients that have already received the queued buffer count - // can be removed from the buffer stuffed Uid list to avoid - // being sent duplicate messages. - for (auto uid : uidsPostedQueuedBuffers) { - mBufferStuffedUids.erase(uid); - } if (event.header.type == DisplayEventType::DISPLAY_EVENT_VSYNC && FlagManager::getInstance().vrr_config()) { mLastCommittedVsyncTime = diff --git a/services/surfaceflinger/Scheduler/EventThread.h b/services/surfaceflinger/Scheduler/EventThread.h index a91dde7430..612883a88b 100644 --- a/services/surfaceflinger/Scheduler/EventThread.h +++ b/services/surfaceflinger/Scheduler/EventThread.h @@ -56,7 +56,6 @@ using gui::VsyncEventData; // --------------------------------------------------------------------------- using FrameRateOverride = DisplayEventReceiver::Event::FrameRateOverride; -using BufferStuffingMap = ftl::SmallMap<uid_t, uint32_t, 10>; enum class VSyncRequest { None = -2, @@ -141,10 +140,6 @@ public: virtual void onHdcpLevelsChanged(PhysicalDisplayId displayId, int32_t connectedLevel, int32_t maxLevel) = 0; - - // An elevated number of queued buffers in the server is detected. This propagates a - // flag to Choreographer indicating that buffer stuffing recovery should begin. - virtual void addBufferStuffedUids(BufferStuffingMap bufferStuffedUids) = 0; }; struct IEventThreadCallback { @@ -199,8 +194,6 @@ public: void onHdcpLevelsChanged(PhysicalDisplayId displayId, int32_t connectedLevel, int32_t maxLevel) override; - void addBufferStuffedUids(BufferStuffingMap bufferStuffedUids) override; - private: friend EventThreadTest; @@ -241,10 +234,6 @@ private: scheduler::VSyncCallbackRegistration mVsyncRegistration GUARDED_BY(mMutex); frametimeline::TokenManager* const mTokenManager; - // All consumers that need to recover from buffer stuffing and the number - // of their queued buffers. - BufferStuffingMap mBufferStuffedUids GUARDED_BY(mMutex); - IEventThreadCallback& mCallback; std::thread mThread; diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp index 911d4894ce..c9d3b31061 100644 --- a/services/surfaceflinger/Scheduler/Scheduler.cpp +++ b/services/surfaceflinger/Scheduler/Scheduler.cpp @@ -960,11 +960,6 @@ bool Scheduler::updateFrameRateOverridesLocked(GlobalSignals consideredSignals, return mFrameRateOverrideMappings.updateFrameRateOverridesByContent(frameRateOverrides); } -void Scheduler::addBufferStuffedUids(BufferStuffingMap bufferStuffedUids) { - if (!mRenderEventThread) return; - mRenderEventThread->addBufferStuffedUids(std::move(bufferStuffedUids)); -} - void Scheduler::promotePacesetterDisplay(PhysicalDisplayId pacesetterId, PromotionParams params) { std::shared_ptr<VsyncSchedule> pacesetterVsyncSchedule; { diff --git a/services/surfaceflinger/Scheduler/Scheduler.h b/services/surfaceflinger/Scheduler/Scheduler.h index 81389e7362..61469c1b46 100644 --- a/services/surfaceflinger/Scheduler/Scheduler.h +++ b/services/surfaceflinger/Scheduler/Scheduler.h @@ -337,10 +337,6 @@ public: mPacesetterFrameDurationFractionToSkip = frameDurationFraction; } - // Propagates a flag to the EventThread indicating that buffer stuffing - // recovery should begin. - void addBufferStuffedUids(BufferStuffingMap bufferStuffedUids); - void setDebugPresentDelay(TimePoint delay) { mDebugPresentDelay = delay; } private: diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index c80270d4e7..ce7a720714 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -66,13 +66,11 @@ #include <ftl/concat.h> #include <ftl/fake_guard.h> #include <ftl/future.h> -#include <ftl/small_map.h> #include <ftl/unit.h> #include <gui/AidlUtil.h> #include <gui/BufferQueue.h> #include <gui/DebugEGLImageTracker.h> #include <gui/IProducerListener.h> -#include <gui/JankInfo.h> #include <gui/LayerMetadata.h> #include <gui/LayerState.h> #include <gui/Surface.h> @@ -3315,40 +3313,12 @@ void SurfaceFlinger::onCompositionPresented(PhysicalDisplayId pacesetterId, const TimePoint presentTime = TimePoint::now(); - // The Uids of layer owners that are in buffer stuffing mode, and their elevated - // buffer counts. Messages to start recovery are sent exclusively to these Uids. - BufferStuffingMap bufferStuffedUids; - // Set presentation information before calling Layer::releasePendingBuffer, such that jank // information from previous' frame classification is already available when sending jank info // to clients, so they get jank classification as early as possible. mFrameTimeline->setSfPresent(presentTime.ns(), pacesetterPresentFenceTime, pacesetterGpuCompositionDoneFenceTime); - // Find and register any layers that are in buffer stuffing mode - const auto& presentFrames = mFrameTimeline->getPresentFrames(); - - for (const auto& frame : presentFrames) { - const auto& layer = mLayerLifecycleManager.getLayerFromId(frame->getLayerId()); - if (!layer) continue; - uint32_t numberQueuedBuffers = layer->pendingBuffers ? layer->pendingBuffers->load() : 0; - int32_t jankType = frame->getJankType().value_or(JankType::None); - if (jankType & JankType::BufferStuffing && - layer->flags & layer_state_t::eRecoverableFromBufferStuffing) { - auto [it, wasEmplaced] = - bufferStuffedUids.try_emplace(layer->ownerUid.val(), numberQueuedBuffers); - // Update with maximum number of queued buffers, allows clients drawing - // multiple windows to account for the most severely stuffed window - if (!wasEmplaced && it->second < numberQueuedBuffers) { - it->second = numberQueuedBuffers; - } - } - } - - if (!bufferStuffedUids.empty()) { - mScheduler->addBufferStuffedUids(std::move(bufferStuffedUids)); - } - // We use the CompositionEngine::getLastFrameRefreshTimestamp() which might // be sampled a little later than when we started doing work for this frame, // but that should be okay since CompositorTiming has snapping logic. diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index 5464d0e19e..c472c4c6d4 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -1459,8 +1459,6 @@ private: // Flag used to set override desired display mode from backdoor bool mDebugDisplayModeSetByBackdoor = false; - // Tracks the number of maximum queued buffers by layer owner Uid. - using BufferStuffingMap = ftl::SmallMap<uid_t, uint32_t, 10>; BufferCountTracker mBufferCountTracker; std::unordered_map<DisplayId, sp<HdrLayerInfoReporter>> mHdrLayerInfoListeners diff --git a/services/surfaceflinger/tests/unittests/EventThreadTest.cpp b/services/surfaceflinger/tests/unittests/EventThreadTest.cpp index 6f15db8beb..76e01a6e7d 100644 --- a/services/surfaceflinger/tests/unittests/EventThreadTest.cpp +++ b/services/surfaceflinger/tests/unittests/EventThreadTest.cpp @@ -23,7 +23,6 @@ #include <gmock/gmock.h> #include <gtest/gtest.h> -#include <gui/DisplayEventReceiver.h> #include <log/log.h> #include <scheduler/VsyncConfig.h> #include <utils/Errors.h> @@ -112,8 +111,6 @@ protected: void expectOnExpectedPresentTimePosted(nsecs_t expectedPresentTime); void expectUidFrameRateMappingEventReceivedByConnection(PhysicalDisplayId expectedDisplayId, std::vector<FrameRateOverride>); - void expectQueuedBufferCountReceivedByConnection( - ConnectionEventRecorder& connectionEventRecorder, uint32_t expectedBufferCount); void onVSyncEvent(nsecs_t timestamp, nsecs_t expectedPresentationTime, nsecs_t deadlineTimestamp) { @@ -147,7 +144,6 @@ protected: sp<MockEventThreadConnection> mConnection; sp<MockEventThreadConnection> mThrottledConnection; std::unique_ptr<frametimeline::impl::TokenManager> mTokenManager; - std::vector<ConnectionEventRecorder*> mBufferStuffedConnectionRecorders; std::chrono::nanoseconds mVsyncPeriod; @@ -380,14 +376,6 @@ void EventThreadTest::expectUidFrameRateMappingEventReceivedByConnection( EXPECT_EQ(expectedDisplayId, event.header.displayId); } -void EventThreadTest::expectQueuedBufferCountReceivedByConnection( - ConnectionEventRecorder& connectionEventRecorder, uint32_t expectedBufferCount) { - auto args = connectionEventRecorder.waitForCall(); - ASSERT_TRUE(args.has_value()); - const auto& event = std::get<0>(args.value()); - EXPECT_EQ(expectedBufferCount, event.vsync.vsyncData.numberQueuedBuffers); -} - namespace { using namespace testing; @@ -880,63 +868,6 @@ TEST_F(EventThreadTest, postHcpLevelsChanged) { EXPECT_EQ(HDCP_V2, event.hdcpLevelsChange.maxLevel); } -TEST_F(EventThreadTest, connectionReceivesBufferStuffing) { - setupEventThread(); - - // Create a connection that will experience buffer stuffing. - ConnectionEventRecorder stuffedConnectionEventRecorder{0}; - sp<MockEventThreadConnection> stuffedConnection = - createConnection(stuffedConnectionEventRecorder, - gui::ISurfaceComposer::EventRegistration::modeChanged | - gui::ISurfaceComposer::EventRegistration::frameRateOverride, - 111); - - // Add a connection and buffer count to the list of stuffed Uids that will receive - // data in the next vsync event. - BufferStuffingMap bufferStuffedUids; - bufferStuffedUids.try_emplace(stuffedConnection->mOwnerUid, 3); - mThread->addBufferStuffedUids(bufferStuffedUids); - mBufferStuffedConnectionRecorders.emplace_back(&stuffedConnectionEventRecorder); - - // Signal that we want the next vsync event to be posted to two connections. - mThread->requestNextVsync(mConnection); - mThread->requestNextVsync(stuffedConnection); - onVSyncEvent(123, 456, 789); - - // Vsync event data contains number of queued buffers. - expectQueuedBufferCountReceivedByConnection(mConnectionEventCallRecorder, 0); - expectQueuedBufferCountReceivedByConnection(stuffedConnectionEventRecorder, 3); -} - -TEST_F(EventThreadTest, connectionsWithSameUidReceiveBufferStuffing) { - setupEventThread(); - - // Create a connection with the same Uid as another connection. - ConnectionEventRecorder secondConnectionEventRecorder{0}; - sp<MockEventThreadConnection> secondConnection = - createConnection(secondConnectionEventRecorder, - gui::ISurfaceComposer::EventRegistration::modeChanged | - gui::ISurfaceComposer::EventRegistration::frameRateOverride, - mConnectionUid); - - // Add connection Uid and buffer count to the list of stuffed Uids that will receive - // data in the next vsync event. - BufferStuffingMap bufferStuffedUids; - bufferStuffedUids.try_emplace(mConnectionUid, 3); - mThread->addBufferStuffedUids(bufferStuffedUids); - mBufferStuffedConnectionRecorders.emplace_back(&mConnectionEventCallRecorder); - mBufferStuffedConnectionRecorders.emplace_back(&secondConnectionEventRecorder); - - // Signal that we want the next vsync event to be posted to two connections. - mThread->requestNextVsync(mConnection); - mThread->requestNextVsync(secondConnection); - onVSyncEvent(123, 456, 789); - - // Vsync event data contains number of queued buffers. - expectQueuedBufferCountReceivedByConnection(mConnectionEventCallRecorder, 3); - expectQueuedBufferCountReceivedByConnection(secondConnectionEventRecorder, 3); -} - } // namespace } // namespace android diff --git a/services/surfaceflinger/tests/unittests/mock/MockEventThread.h b/services/surfaceflinger/tests/unittests/mock/MockEventThread.h index 3036fec456..cce4d2aba8 100644 --- a/services/surfaceflinger/tests/unittests/mock/MockEventThread.h +++ b/services/surfaceflinger/tests/unittests/mock/MockEventThread.h @@ -55,7 +55,6 @@ public: MOCK_METHOD(void, onHdcpLevelsChanged, (PhysicalDisplayId displayId, int32_t connectedLevel, int32_t maxLevel), (override)); - MOCK_METHOD(void, addBufferStuffedUids, (BufferStuffingMap), (override)); }; } // namespace android::mock |