From d0b44a52f815fb20cb470aca8ac17da498797c1e Mon Sep 17 00:00:00 2001 From: John Dias Date: Tue, 11 Jun 2019 18:16:08 -0700 Subject: sf: optimize luma sampling code Removing some math from the luma-sampling code reduces its running time by about half (~.5ms -> ~.25ms at 1.1MHz on a little core in some experiments). Bug: 127973145 Bug: 134922154 Test: trace and compare sampleArea times when scrolling in news app Change-Id: Ie53d9595bea6685cf45f53972b42daa5e32fcc8e --- services/surfaceflinger/RegionSamplingThread.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'services/surfaceflinger/RegionSamplingThread.cpp') diff --git a/services/surfaceflinger/RegionSamplingThread.cpp b/services/surfaceflinger/RegionSamplingThread.cpp index 7fa33f597c..fdc68aa8d2 100644 --- a/services/surfaceflinger/RegionSamplingThread.cpp +++ b/services/surfaceflinger/RegionSamplingThread.cpp @@ -258,7 +258,7 @@ void RegionSamplingThread::binderDied(const wp& who) { namespace { // Using Rec. 709 primaries -float getLuma(float r, float g, float b) { +inline float getLuma(float r, float g, float b) { constexpr auto rec709_red_primary = 0.2126f; constexpr auto rec709_green_primary = 0.7152f; constexpr auto rec709_blue_primary = 0.0722f; @@ -289,10 +289,10 @@ float sampleArea(const uint32_t* data, int32_t width, int32_t height, int32_t st const uint32_t* rowBase = data + row * stride; for (int32_t column = area.left; column < area.right; ++column) { uint32_t pixel = rowBase[column]; - const float r = (pixel & 0xFF) / 255.0f; - const float g = ((pixel >> 8) & 0xFF) / 255.0f; - const float b = ((pixel >> 16) & 0xFF) / 255.0f; - const uint8_t luma = std::round(getLuma(r, g, b) * 255.0f); + const float r = pixel & 0xFF; + const float g = (pixel >> 8) & 0xFF; + const float b = (pixel >> 16) & 0xFF; + const uint8_t luma = std::round(getLuma(r, g, b)); ++brightnessBuckets[luma]; if (brightnessBuckets[luma] > majoritySampleNum) return luma / 255.0f; } -- cgit v1.2.3-59-g8ed1b From 84be783ff179fd6589bd01091f08f174f1f6a95d Mon Sep 17 00:00:00 2001 From: John Dias Date: Tue, 18 Jun 2019 17:05:26 -0700 Subject: SF: delay region sampling when short on time In a number of janky traces, particularly at high frame rates, we've seen the surfaceflinger thread overrunning its time slot. In some of those cases, the surfaceflinger thread is doing region-sampling. This change causes region-sampling to check how much time is left until the next vsync before deciding whether to sample this frame. If low on time, it will defer the sampling to a later frame. Bug: 133779857 Test: trace inspection from scrolling in various apps Change-Id: I92c2368e80033c1ba6e27f947a456d14db02064c --- services/surfaceflinger/RegionSamplingThread.cpp | 23 +++++++++++++++++++---- services/surfaceflinger/RegionSamplingThread.h | 2 +- 2 files changed, 20 insertions(+), 5 deletions(-) (limited to 'services/surfaceflinger/RegionSamplingThread.cpp') diff --git a/services/surfaceflinger/RegionSamplingThread.cpp b/services/surfaceflinger/RegionSamplingThread.cpp index 66906e950c..4fca63aa96 100644 --- a/services/surfaceflinger/RegionSamplingThread.cpp +++ b/services/surfaceflinger/RegionSamplingThread.cpp @@ -44,11 +44,14 @@ constexpr auto lumaSamplingStepTag = "LumaSamplingStep"; enum class samplingStep { noWorkNeeded, idleTimerWaiting, + waitForQuietFrame, waitForZeroPhase, waitForSamplePhase, sample }; +constexpr auto timeForRegionSampling = 5000000ns; +constexpr auto maxRegionSamplingSkips = 10; constexpr auto defaultRegionSamplingOffset = -3ms; constexpr auto defaultRegionSamplingPeriod = 100ms; constexpr auto defaultRegionSamplingTimerTimeout = 100ms; @@ -215,9 +218,9 @@ void RegionSamplingThread::removeListener(const sp& lis void RegionSamplingThread::checkForStaleLuma() { std::lock_guard lock(mThreadControlMutex); - if (mDiscardedFrames) { + if (mDiscardedFrames > 0) { ATRACE_INT(lumaSamplingStepTag, static_cast(samplingStep::waitForZeroPhase)); - mDiscardedFrames = false; + mDiscardedFrames = 0; mPhaseCallback->startVsyncListener(); } } @@ -235,13 +238,25 @@ void RegionSamplingThread::doSample() { auto now = std::chrono::nanoseconds(systemTime(SYSTEM_TIME_MONOTONIC)); if (lastSampleTime + mTunables.mSamplingPeriod > now) { ATRACE_INT(lumaSamplingStepTag, static_cast(samplingStep::idleTimerWaiting)); - mDiscardedFrames = true; + if (mDiscardedFrames == 0) mDiscardedFrames++; return; } + if (mDiscardedFrames < maxRegionSamplingSkips) { + // If there is relatively little time left for surfaceflinger + // until the next vsync deadline, defer this sampling work + // to a later frame, when hopefully there will be more time. + DisplayStatInfo stats; + mScheduler.getDisplayStatInfo(&stats); + if (std::chrono::nanoseconds(stats.vsyncTime) - now < timeForRegionSampling) { + ATRACE_INT(lumaSamplingStepTag, static_cast(samplingStep::waitForQuietFrame)); + mDiscardedFrames++; + return; + } + } ATRACE_INT(lumaSamplingStepTag, static_cast(samplingStep::sample)); - mDiscardedFrames = false; + mDiscardedFrames = 0; lastSampleTime = now; mIdleTimer.reset(); diff --git a/services/surfaceflinger/RegionSamplingThread.h b/services/surfaceflinger/RegionSamplingThread.h index 3c6fcf3872..96ffe207e4 100644 --- a/services/surfaceflinger/RegionSamplingThread.h +++ b/services/surfaceflinger/RegionSamplingThread.h @@ -117,7 +117,7 @@ private: std::condition_variable_any mCondition; bool mRunning GUARDED_BY(mThreadControlMutex) = true; bool mSampleRequested GUARDED_BY(mThreadControlMutex) = false; - bool mDiscardedFrames GUARDED_BY(mThreadControlMutex) = false; + uint32_t mDiscardedFrames GUARDED_BY(mThreadControlMutex) = 0; std::chrono::nanoseconds lastSampleTime GUARDED_BY(mThreadControlMutex); std::mutex mSamplingMutex; -- cgit v1.2.3-59-g8ed1b From 769ab6f949369468ee6355d00e490ad14a2f3721 Mon Sep 17 00:00:00 2001 From: Kevin DuBois Date: Wed, 19 Jun 2019 08:13:28 -0700 Subject: SF: do not extend DisplayDevice lifetime DisplayDevice lifetime was extended, resulting in stack corruption due to lifetime issue Fixes: 135211720 Test: 1h45m continual rotation with PIP video playing without crashing Test: Functional testing to make sure b/132394665 is still fixed. Change-Id: I1c4b3946a6da7994ff442a2345c1c878aa899ee4 --- services/surfaceflinger/RegionSamplingThread.cpp | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) (limited to 'services/surfaceflinger/RegionSamplingThread.cpp') diff --git a/services/surfaceflinger/RegionSamplingThread.cpp b/services/surfaceflinger/RegionSamplingThread.cpp index 8b85d4cdbf..72bbf5dc26 100644 --- a/services/surfaceflinger/RegionSamplingThread.cpp +++ b/services/surfaceflinger/RegionSamplingThread.cpp @@ -342,9 +342,19 @@ void RegionSamplingThread::captureSample() { } const auto device = mFlinger.getDefaultDisplayDevice(); - const auto display = device->getCompositionDisplay(); - const auto state = display->getState(); - const auto orientation = static_cast(state.orientation); + const auto orientation = [](uint32_t orientation) { + switch (orientation) { + default: + case DisplayState::eOrientationDefault: + return ui::Transform::ROT_0; + case DisplayState::eOrientation90: + return ui::Transform::ROT_90; + case DisplayState::eOrientation180: + return ui::Transform::ROT_180; + case DisplayState::eOrientation270: + return ui::Transform::ROT_270; + } + }(device->getOrientation()); std::vector descriptors; Region sampleRegion; -- cgit v1.2.3-59-g8ed1b