From c0086af0dd6fc979f1f03328db5a9e72feb4f258 Mon Sep 17 00:00:00 2001 From: Jorim Jaggi Date: Fri, 12 Feb 2021 18:18:11 +0100 Subject: Change hwui jank detection to use deadline & gpu completion (2/2) - Use GPU finish time as well as actual deadline to determine jank rate. - Use dynamic interval to adjust for 60/90hz switching - Move frame metrics reporting into JankTracker to adjust the deadline communicated to the app when in stuffing scenario. - Adjust double-stuffing detection to be a bit more readable. Test: GraphicsStatsValidationTest.java Test: adb shell dumpsys gfxinfo Test: FrameMetricsListenerTest Test: Log output of FrameMetricsObserver Bug: 169858044 Change-Id: Ia1cae9f0c5358d1cd3bf043289ea8b4d26154737 --- libs/gui/DisplayEventDispatcher.cpp | 1 + libs/gui/include/gui/DisplayEventDispatcher.h | 3 +++ libs/gui/include/gui/DisplayEventReceiver.h | 1 + libs/gui/tests/DisplayEventStructLayout_test.cpp | 3 ++- libs/nativedisplay/AChoreographer.cpp | 10 ++++++++- .../private/android/choreographer.h | 5 +++++ libs/nativedisplay/libnativedisplay.map.txt | 1 + services/surfaceflinger/Scheduler/EventThread.cpp | 14 +++++++++++-- services/surfaceflinger/Scheduler/EventThread.h | 4 +++- services/surfaceflinger/Scheduler/Scheduler.cpp | 24 ++++++++++++++++++++-- services/surfaceflinger/Scheduler/Scheduler.h | 1 + .../tests/unittests/EventThreadTest.cpp | 7 +++++-- 12 files changed, 65 insertions(+), 9 deletions(-) diff --git a/libs/gui/DisplayEventDispatcher.cpp b/libs/gui/DisplayEventDispatcher.cpp index 9cd3f631c8..e1b1efc0ed 100644 --- a/libs/gui/DisplayEventDispatcher.cpp +++ b/libs/gui/DisplayEventDispatcher.cpp @@ -152,6 +152,7 @@ bool DisplayEventDispatcher::processPendingEvents(nsecs_t* outTimestamp, *outCount = ev.vsync.count; outVsyncEventData->id = ev.vsync.vsyncId; outVsyncEventData->deadlineTimestamp = ev.vsync.deadlineTimestamp; + outVsyncEventData->frameInterval = ev.vsync.frameInterval; break; case DisplayEventReceiver::DISPLAY_EVENT_HOTPLUG: dispatchHotplug(ev.header.timestamp, ev.header.displayId, ev.hotplug.connected); diff --git a/libs/gui/include/gui/DisplayEventDispatcher.h b/libs/gui/include/gui/DisplayEventDispatcher.h index d74c2ba72b..4ade240dcf 100644 --- a/libs/gui/include/gui/DisplayEventDispatcher.h +++ b/libs/gui/include/gui/DisplayEventDispatcher.h @@ -30,6 +30,9 @@ struct VsyncEventData { // The deadline in CLOCK_MONOTONIC that the app needs to complete its // frame by (both on the CPU and the GPU) int64_t deadlineTimestamp = std::numeric_limits::max(); + + // The current frame interval in ns when this frame was scheduled. + int64_t frameInterval = 0; }; class DisplayEventDispatcher : public LooperCallback { diff --git a/libs/gui/include/gui/DisplayEventReceiver.h b/libs/gui/include/gui/DisplayEventReceiver.h index 7179a20d22..0dffbde88a 100644 --- a/libs/gui/include/gui/DisplayEventReceiver.h +++ b/libs/gui/include/gui/DisplayEventReceiver.h @@ -75,6 +75,7 @@ public: uint32_t count; nsecs_t expectedVSyncTimestamp __attribute__((aligned(8))); nsecs_t deadlineTimestamp __attribute__((aligned(8))); + nsecs_t frameInterval __attribute__((aligned(8))); int64_t vsyncId; }; diff --git a/libs/gui/tests/DisplayEventStructLayout_test.cpp b/libs/gui/tests/DisplayEventStructLayout_test.cpp index 7dbba31ba8..bcd39dbbf4 100644 --- a/libs/gui/tests/DisplayEventStructLayout_test.cpp +++ b/libs/gui/tests/DisplayEventStructLayout_test.cpp @@ -34,7 +34,8 @@ TEST(DisplayEventStructLayoutTest, TestEventAlignment) { CHECK_OFFSET(DisplayEventReceiver::Event::VSync, count, 0); CHECK_OFFSET(DisplayEventReceiver::Event::VSync, expectedVSyncTimestamp, 8); CHECK_OFFSET(DisplayEventReceiver::Event::VSync, deadlineTimestamp, 16); - CHECK_OFFSET(DisplayEventReceiver::Event::VSync, vsyncId, 24); + CHECK_OFFSET(DisplayEventReceiver::Event::VSync, frameInterval, 24); + CHECK_OFFSET(DisplayEventReceiver::Event::VSync, vsyncId, 32); CHECK_OFFSET(DisplayEventReceiver::Event::Hotplug, connected, 0); diff --git a/libs/nativedisplay/AChoreographer.cpp b/libs/nativedisplay/AChoreographer.cpp index dc2dd297c5..0edb213089 100644 --- a/libs/nativedisplay/AChoreographer.cpp +++ b/libs/nativedisplay/AChoreographer.cpp @@ -130,7 +130,7 @@ public: virtual ~Choreographer() override EXCLUDES(gChoreographers.lock); int64_t getVsyncId() const; int64_t getFrameDeadline() const; - + int64_t getFrameInterval() const; private: Choreographer(const Choreographer&) = delete; @@ -418,6 +418,10 @@ int64_t Choreographer::getFrameDeadline() const { return mLastVsyncEventData.deadlineTimestamp; } +int64_t Choreographer::getFrameInterval() const { + return mLastVsyncEventData.frameInterval; +} + } // namespace android using namespace android; @@ -501,6 +505,10 @@ int64_t AChoreographer_getFrameDeadline(const AChoreographer* choreographer) { return AChoreographer_to_Choreographer(choreographer)->getFrameDeadline(); } +int64_t AChoreographer_getFrameInterval(const AChoreographer* choreographer) { + return AChoreographer_to_Choreographer(choreographer)->getFrameInterval(); +} + } // namespace android /* Glue for the NDK interface */ diff --git a/libs/nativedisplay/include-private/private/android/choreographer.h b/libs/nativedisplay/include-private/private/android/choreographer.h index 0f4fd4521f..7d25ce8253 100644 --- a/libs/nativedisplay/include-private/private/android/choreographer.h +++ b/libs/nativedisplay/include-private/private/android/choreographer.h @@ -42,6 +42,11 @@ int64_t AChoreographer_getVsyncId(const AChoreographer* choreographer); // value. int64_t AChoreographer_getFrameDeadline(const AChoreographer* choreographer); +// Returns the current interval in ns between frames. +// Client are expected to call this function from their frame callback function. +// Calling this function from anywhere else will return an undefined value. +int64_t AChoreographer_getFrameInterval(const AChoreographer* choreographer); + // Trampoline functions allowing libandroid.so to define the NDK symbols without including // the entirety of libnativedisplay as a whole static lib. As libnativedisplay // maintains global state, libnativedisplay can never be directly statically diff --git a/libs/nativedisplay/libnativedisplay.map.txt b/libs/nativedisplay/libnativedisplay.map.txt index fda6a20a0b..9ed4915481 100644 --- a/libs/nativedisplay/libnativedisplay.map.txt +++ b/libs/nativedisplay/libnativedisplay.map.txt @@ -31,6 +31,7 @@ LIBNATIVEDISPLAY_PLATFORM { android::AChoreographer_signalRefreshRateCallbacks*; android::AChoreographer_getVsyncId*; android::AChoreographer_getFrameDeadline*; + android::AChoreographer_getFrameInterval*; android::ADisplay_acquirePhysicalDisplays*; android::ADisplay_release*; android::ADisplay_getMaxSupportedFps*; diff --git a/services/surfaceflinger/Scheduler/EventThread.cpp b/services/surfaceflinger/Scheduler/EventThread.cpp index 6553efe794..2321e2d082 100644 --- a/services/surfaceflinger/Scheduler/EventThread.cpp +++ b/services/surfaceflinger/Scheduler/EventThread.cpp @@ -217,12 +217,18 @@ namespace impl { EventThread::EventThread(std::unique_ptr vsyncSource, android::frametimeline::TokenManager* tokenManager, InterceptVSyncsCallback interceptVSyncsCallback, - ThrottleVsyncCallback throttleVsyncCallback) + ThrottleVsyncCallback throttleVsyncCallback, + GetVsyncPeriodFunction getVsyncPeriodFunction) : mVSyncSource(std::move(vsyncSource)), mTokenManager(tokenManager), mInterceptVSyncsCallback(std::move(interceptVSyncsCallback)), mThrottleVsyncCallback(std::move(throttleVsyncCallback)), + mGetVsyncPeriodFunction(std::move(getVsyncPeriodFunction)), mThreadName(mVSyncSource->getName()) { + + LOG_ALWAYS_FATAL_IF(getVsyncPeriodFunction == nullptr, + "getVsyncPeriodFunction must not be null"); + mVSyncSource->setCallback(this); mThread = std::thread([this]() NO_THREAD_SAFETY_ANALYSIS { @@ -565,7 +571,11 @@ bool EventThread::shouldConsumeEvent(const DisplayEventReceiver::Event& event, void EventThread::dispatchEvent(const DisplayEventReceiver::Event& event, const DisplayEventConsumers& consumers) { for (const auto& consumer : consumers) { - switch (consumer->postEvent(event)) { + DisplayEventReceiver::Event copy = event; + if (event.header.type == DisplayEventReceiver::DISPLAY_EVENT_VSYNC) { + copy.vsync.frameInterval = mGetVsyncPeriodFunction(consumer->mOwnerUid); + } + switch (consumer->postEvent(copy)) { case NO_ERROR: break; diff --git a/services/surfaceflinger/Scheduler/EventThread.h b/services/surfaceflinger/Scheduler/EventThread.h index 35406041a8..1e6793f77c 100644 --- a/services/surfaceflinger/Scheduler/EventThread.h +++ b/services/surfaceflinger/Scheduler/EventThread.h @@ -152,9 +152,10 @@ class EventThread : public android::EventThread, private VSyncSource::Callback { public: using InterceptVSyncsCallback = std::function; using ThrottleVsyncCallback = std::function; + using GetVsyncPeriodFunction = std::function; EventThread(std::unique_ptr, frametimeline::TokenManager*, InterceptVSyncsCallback, - ThrottleVsyncCallback); + ThrottleVsyncCallback, GetVsyncPeriodFunction); ~EventThread(); sp createEventConnection( @@ -210,6 +211,7 @@ private: const InterceptVSyncsCallback mInterceptVSyncsCallback; const ThrottleVsyncCallback mThrottleVsyncCallback; + const GetVsyncPeriodFunction mGetVsyncPeriodFunction; const char* const mThreadName; std::thread mThread; diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp index 8426737597..1d25c7247f 100644 --- a/services/surfaceflinger/Scheduler/Scheduler.cpp +++ b/services/surfaceflinger/Scheduler/Scheduler.cpp @@ -248,15 +248,34 @@ impl::EventThread::ThrottleVsyncCallback Scheduler::makeThrottleVsyncCallback() }; } +impl::EventThread::GetVsyncPeriodFunction Scheduler::makeGetVsyncPeriodFunction() const { + return [this](uid_t uid) { + nsecs_t basePeriod = mRefreshRateConfigs.getCurrentRefreshRate().getVsyncPeriod(); + const auto frameRate = getFrameRateOverride(uid); + if (!frameRate.has_value()) { + return basePeriod; + } + + const auto divider = scheduler::RefreshRateConfigs::getFrameRateDivider( + mRefreshRateConfigs.getCurrentRefreshRate().getFps(), *frameRate); + if (divider <= 1) { + return basePeriod; + } + return basePeriod * divider; + }; +} + Scheduler::ConnectionHandle Scheduler::createConnection( const char* connectionName, frametimeline::TokenManager* tokenManager, std::chrono::nanoseconds workDuration, std::chrono::nanoseconds readyDuration, impl::EventThread::InterceptVSyncsCallback interceptCallback) { auto vsyncSource = makePrimaryDispSyncSource(connectionName, workDuration, readyDuration); auto throttleVsync = makeThrottleVsyncCallback(); + auto getVsyncPeriod = makeGetVsyncPeriodFunction(); auto eventThread = std::make_unique(std::move(vsyncSource), tokenManager, std::move(interceptCallback), - std::move(throttleVsync)); + std::move(throttleVsync), + std::move(getVsyncPeriod)); return createConnection(std::move(eventThread)); } @@ -444,7 +463,8 @@ Scheduler::ConnectionHandle Scheduler::enableVSyncInjection(bool enable) { std::make_unique(std::move(vsyncSource), /*tokenManager=*/nullptr, impl::EventThread::InterceptVSyncsCallback(), - impl::EventThread::ThrottleVsyncCallback()); + impl::EventThread::ThrottleVsyncCallback(), + impl::EventThread::GetVsyncPeriodFunction()); // EventThread does not dispatch VSYNC unless the display is connected and powered on. eventThread->onHotplugReceived(PhysicalDisplayId::fromPort(0), true); diff --git a/services/surfaceflinger/Scheduler/Scheduler.h b/services/surfaceflinger/Scheduler/Scheduler.h index d4932e7f8e..4d1f3c6572 100644 --- a/services/surfaceflinger/Scheduler/Scheduler.h +++ b/services/surfaceflinger/Scheduler/Scheduler.h @@ -240,6 +240,7 @@ private: EXCLUDES(mFrameRateOverridesMutex); impl::EventThread::ThrottleVsyncCallback makeThrottleVsyncCallback() const; + impl::EventThread::GetVsyncPeriodFunction makeGetVsyncPeriodFunction() const; // Stores EventThread associated with a given VSyncSource, and an initial EventThreadConnection. struct Connection { diff --git a/services/surfaceflinger/tests/unittests/EventThreadTest.cpp b/services/surfaceflinger/tests/unittests/EventThreadTest.cpp index 7cc0032bc8..b4a1481e9c 100644 --- a/services/surfaceflinger/tests/unittests/EventThreadTest.cpp +++ b/services/surfaceflinger/tests/unittests/EventThreadTest.cpp @@ -44,7 +44,7 @@ namespace { constexpr PhysicalDisplayId INTERNAL_DISPLAY_ID(111); constexpr PhysicalDisplayId EXTERNAL_DISPLAY_ID(222); constexpr PhysicalDisplayId DISPLAY_ID_64BIT(0xabcd12349876fedcULL); - +constexpr std::chrono::duration VSYNC_PERIOD(16ms); class MockVSyncSource : public VSyncSource { public: const char* getName() const override { return "test"; } @@ -166,11 +166,14 @@ void EventThreadTest::createThread(std::unique_ptr source) { mThrottleVsyncCallRecorder.getInvocable()(expectedVsyncTimestamp, uid); return (uid == mThrottledConnectionUid); }; + const auto getVsyncPeriod = [](uid_t uid) { + return VSYNC_PERIOD.count(); + }; mThread = std::make_unique(std::move(source), /*tokenManager=*/nullptr, mInterceptVSyncCallRecorder.getInvocable(), - throttleVsync); + throttleVsync, getVsyncPeriod); // EventThread should register itself as VSyncSource callback. mCallback = expectVSyncSetCallbackCallReceived(); -- cgit v1.2.3-59-g8ed1b