diff --git a/libs/hwui/Android.bp b/libs/hwui/Android.bp index 2c299fa323152..b931d030f2a02 100644 --- a/libs/hwui/Android.bp +++ b/libs/hwui/Android.bp @@ -587,6 +587,7 @@ cc_defaults { "HardwareBitmapUploader.cpp", "HWUIProperties.sysprop", "JankTracker.cpp", + "FrameMetricsReporter.cpp", "Layer.cpp", "LayerUpdateQueue.cpp", "ProfileData.cpp", @@ -691,6 +692,7 @@ cc_test { "tests/unit/FatVectorTests.cpp", "tests/unit/GraphicsStatsServiceTests.cpp", "tests/unit/JankTrackerTests.cpp", + "tests/unit/FrameMetricsReporterTests.cpp", "tests/unit/LayerUpdateQueueTests.cpp", "tests/unit/LinearAllocatorTests.cpp", "tests/unit/MatrixTests.cpp", diff --git a/libs/hwui/FrameMetricsObserver.h b/libs/hwui/FrameMetricsObserver.h index ef1f5aabcbd8b..498ec5793caba 100644 --- a/libs/hwui/FrameMetricsObserver.h +++ b/libs/hwui/FrameMetricsObserver.h @@ -26,6 +26,13 @@ public: virtual void notify(const int64_t* buffer) = 0; bool waitForPresentTime() const { return mWaitForPresentTime; }; + void reportMetricsFrom(uint64_t frameNumber, int32_t surfaceControlId) { + mAttachedFrameNumber = frameNumber; + mSurfaceControlId = surfaceControlId; + }; + uint64_t attachedFrameNumber() const { return mAttachedFrameNumber; }; + int32_t attachedSurfaceControlId() const { return mSurfaceControlId; }; + /** * Create a new metrics observer. An observer that watches present time gets notified at a * different time than the observer that doesn't. @@ -42,6 +49,22 @@ public: private: const bool mWaitForPresentTime; + + // The id of the surface control (mSurfaceControlGenerationId in CanvasContext) + // for which the mAttachedFrameNumber applies to. We rely on this value being + // an increasing counter. We will report metrics: + // - for all frames if the frame comes from a surface with a surfaceControlId + // that is strictly greater than mSurfaceControlId. + // - for all frames with a frame number greater than or equal to mAttachedFrameNumber + // if the frame comes from a surface with a surfaceControlId that is equal to the + // mSurfaceControlId. + // We will never report metrics if the frame comes from a surface with a surfaceControlId + // that is strictly smaller than mSurfaceControlId. + int32_t mSurfaceControlId; + + // The frame number the metrics observer was attached on. Metrics will be sent from this frame + // number (inclusive) onwards in the case that the surface id is equal to mSurfaceControlId. + uint64_t mAttachedFrameNumber; }; } // namespace uirenderer diff --git a/libs/hwui/FrameMetricsReporter.cpp b/libs/hwui/FrameMetricsReporter.cpp new file mode 100644 index 0000000000000..ee32ea17bfaf2 --- /dev/null +++ b/libs/hwui/FrameMetricsReporter.cpp @@ -0,0 +1,56 @@ +/* + * Copyright (C) 2021 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "FrameMetricsReporter.h" + +namespace android { +namespace uirenderer { + +void FrameMetricsReporter::reportFrameMetrics(const int64_t* stats, bool hasPresentTime, + uint64_t frameNumber, int32_t surfaceControlId) { + FatVector, 10> copy; + { + std::lock_guard lock(mObserversLock); + copy.reserve(mObservers.size()); + for (size_t i = 0; i < mObservers.size(); i++) { + auto observer = mObservers[i]; + + if (CC_UNLIKELY(surfaceControlId < observer->attachedSurfaceControlId())) { + // Don't notify if the metrics are from a frame that was run on an old + // surface (one from before the observer was attached). + ALOGV("skipped reporting metrics from old surface %d", surfaceControlId); + continue; + } else if (CC_UNLIKELY(surfaceControlId == observer->attachedSurfaceControlId() && + frameNumber < observer->attachedFrameNumber())) { + // Don't notify if the metrics are from a frame that was queued by the + // BufferQueueProducer on the render thread before the observer was attached. + ALOGV("skipped reporting metrics from old frame %ld", (long)frameNumber); + continue; + } + + const bool wantsPresentTime = observer->waitForPresentTime(); + if (hasPresentTime == wantsPresentTime) { + copy.push_back(observer); + } + } + } + for (size_t i = 0; i < copy.size(); i++) { + copy[i]->notify(stats); + } +} + +} // namespace uirenderer +} // namespace android diff --git a/libs/hwui/FrameMetricsReporter.h b/libs/hwui/FrameMetricsReporter.h index 0ac025fb01db8..7e51df7ce6fcc 100644 --- a/libs/hwui/FrameMetricsReporter.h +++ b/libs/hwui/FrameMetricsReporter.h @@ -63,23 +63,14 @@ public: * If an observer does not want present time, only notify when 'hasPresentTime' is false. * Never notify both types of observers from the same callback, because the callback with * 'hasPresentTime' is sent at a different time than the one without. + * + * The 'frameNumber' and 'surfaceControlId' associated to the frame whose's stats are being + * reported are used to determine whether or not the stats should be reported. We won't report + * stats of frames that are from "old" surfaces (i.e. with surfaceControlIds older than the one + * the observer was attached on) nor those that are from "old" frame numbers. */ - void reportFrameMetrics(const int64_t* stats, bool hasPresentTime) { - FatVector, 10> copy; - { - std::lock_guard lock(mObserversLock); - copy.reserve(mObservers.size()); - for (size_t i = 0; i < mObservers.size(); i++) { - const bool wantsPresentTime = mObservers[i]->waitForPresentTime(); - if (hasPresentTime == wantsPresentTime) { - copy.push_back(mObservers[i]); - } - } - } - for (size_t i = 0; i < copy.size(); i++) { - copy[i]->notify(stats); - } - } + void reportFrameMetrics(const int64_t* stats, bool hasPresentTime, uint64_t frameNumber, + int32_t surfaceControlId); private: FatVector, 10> mObservers GUARDED_BY(mObserversLock); diff --git a/libs/hwui/JankTracker.cpp b/libs/hwui/JankTracker.cpp index 34e5577066f9f..3e5cbb5fd7582 100644 --- a/libs/hwui/JankTracker.cpp +++ b/libs/hwui/JankTracker.cpp @@ -164,7 +164,8 @@ void JankTracker::calculateLegacyJank(FrameInfo& frame) REQUIRES(mDataMutex) { - lastFrameOffset + mFrameIntervalLegacy; } -void JankTracker::finishFrame(FrameInfo& frame, std::unique_ptr& reporter) { +void JankTracker::finishFrame(FrameInfo& frame, std::unique_ptr& reporter, + int64_t frameNumber, int32_t surfaceControlId) { std::lock_guard lock(mDataMutex); calculateLegacyJank(frame); @@ -253,7 +254,8 @@ void JankTracker::finishFrame(FrameInfo& frame, std::unique_ptrreportFrameMetrics(frame.data(), false /* hasPresentTime */); + reporter->reportFrameMetrics(frame.data(), false /* hasPresentTime */, frameNumber, + surfaceControlId); } } diff --git a/libs/hwui/JankTracker.h b/libs/hwui/JankTracker.h index bdb784dc87470..bcd031efa78d7 100644 --- a/libs/hwui/JankTracker.h +++ b/libs/hwui/JankTracker.h @@ -57,7 +57,8 @@ public: } FrameInfo* startFrame() { return &mFrames.next(); } - void finishFrame(FrameInfo& frame, std::unique_ptr& reporter); + void finishFrame(FrameInfo& frame, std::unique_ptr& reporter, + int64_t frameNumber, int32_t surfaceId); // Calculates the 'legacy' jank information, i.e. with outdated refresh rate information and // without GPU completion or deadlined information. diff --git a/libs/hwui/renderthread/CanvasContext.cpp b/libs/hwui/renderthread/CanvasContext.cpp index 0cd6ffba323c3..5fa09221d4936 100644 --- a/libs/hwui/renderthread/CanvasContext.cpp +++ b/libs/hwui/renderthread/CanvasContext.cpp @@ -203,9 +203,10 @@ void CanvasContext::setSurfaceControl(ASurfaceControl* surfaceControl) { mSurfaceControl = surfaceControl; mSurfaceControlGenerationId++; mExpectSurfaceStats = surfaceControl != nullptr; - if (mSurfaceControl != nullptr) { + if (mExpectSurfaceStats) { funcs.acquireFunc(mSurfaceControl); - funcs.registerListenerFunc(surfaceControl, this, &onSurfaceStatsAvailable); + funcs.registerListenerFunc(surfaceControl, mSurfaceControlGenerationId, this, + &onSurfaceStatsAvailable); } } @@ -218,7 +219,7 @@ void CanvasContext::setupPipelineSurface() { } - mFrameNumber = -1; + mFrameNumber = 0; if (mNativeSurface != nullptr && hasSurface) { mHaveNewSurface = true; @@ -512,7 +513,7 @@ nsecs_t CanvasContext::draw() { mContentDrawBounds, mOpaque, mLightInfo, mRenderNodes, &(profiler())); - int64_t frameCompleteNr = getFrameNumber(); + uint64_t frameCompleteNr = getFrameNumber(); waitOnFences(); @@ -580,7 +581,7 @@ nsecs_t CanvasContext::draw() { mCurrentFrameInfo->set(FrameInfoIndex::DequeueBufferDuration) = swap.dequeueDuration; mCurrentFrameInfo->set(FrameInfoIndex::QueueBufferDuration) = swap.queueDuration; mHaveNewSurface = false; - mFrameNumber = -1; + mFrameNumber = 0; } else { mCurrentFrameInfo->set(FrameInfoIndex::DequeueBufferDuration) = 0; mCurrentFrameInfo->set(FrameInfoIndex::QueueBufferDuration) = 0; @@ -613,16 +614,20 @@ nsecs_t CanvasContext::draw() { if (requireSwap) { if (mExpectSurfaceStats) { reportMetricsWithPresentTime(); - std::lock_guard lock(mLast4FrameInfosMutex); - std::pair& next = mLast4FrameInfos.next(); - next.first = mCurrentFrameInfo; - next.second = frameCompleteNr; + { // acquire lock + std::lock_guard lock(mLast4FrameMetricsInfosMutex); + FrameMetricsInfo& next = mLast4FrameMetricsInfos.next(); + next.frameInfo = mCurrentFrameInfo; + next.frameNumber = frameCompleteNr; + next.surfaceId = mSurfaceControlGenerationId; + } // release lock } else { mCurrentFrameInfo->markFrameCompleted(); mCurrentFrameInfo->set(FrameInfoIndex::GpuCompleted) = mCurrentFrameInfo->get(FrameInfoIndex::FrameCompleted); std::scoped_lock lock(mFrameMetricsReporterMutex); - mJankTracker.finishFrame(*mCurrentFrameInfo, mFrameMetricsReporter); + mJankTracker.finishFrame(*mCurrentFrameInfo, mFrameMetricsReporter, frameCompleteNr, + mSurfaceControlGenerationId); } } @@ -658,14 +663,18 @@ void CanvasContext::reportMetricsWithPresentTime() { ATRACE_CALL(); FrameInfo* forthBehind; int64_t frameNumber; + int32_t surfaceControlId; + { // acquire lock - std::scoped_lock lock(mLast4FrameInfosMutex); - if (mLast4FrameInfos.size() != mLast4FrameInfos.capacity()) { + std::scoped_lock lock(mLast4FrameMetricsInfosMutex); + if (mLast4FrameMetricsInfos.size() != mLast4FrameMetricsInfos.capacity()) { // Not enough frames yet return; } - // Surface object keeps stats for the last 8 frames. - std::tie(forthBehind, frameNumber) = mLast4FrameInfos.front(); + auto frameMetricsInfo = mLast4FrameMetricsInfos.front(); + forthBehind = frameMetricsInfo.frameInfo; + frameNumber = frameMetricsInfo.frameNumber; + surfaceControlId = frameMetricsInfo.surfaceId; } // release lock nsecs_t presentTime = 0; @@ -680,25 +689,51 @@ void CanvasContext::reportMetricsWithPresentTime() { { // acquire lock std::scoped_lock lock(mFrameMetricsReporterMutex); if (mFrameMetricsReporter != nullptr) { - mFrameMetricsReporter->reportFrameMetrics(forthBehind->data(), true /*hasPresentTime*/); + mFrameMetricsReporter->reportFrameMetrics(forthBehind->data(), true /*hasPresentTime*/, + frameNumber, surfaceControlId); } } // release lock } -FrameInfo* CanvasContext::getFrameInfoFromLast4(uint64_t frameNumber) { - std::scoped_lock lock(mLast4FrameInfosMutex); - for (size_t i = 0; i < mLast4FrameInfos.size(); i++) { - if (mLast4FrameInfos[i].second == frameNumber) { - return mLast4FrameInfos[i].first; +void CanvasContext::addFrameMetricsObserver(FrameMetricsObserver* observer) { + std::scoped_lock lock(mFrameMetricsReporterMutex); + if (mFrameMetricsReporter.get() == nullptr) { + mFrameMetricsReporter.reset(new FrameMetricsReporter()); + } + + // We want to make sure we aren't reporting frames that have already been queued by the + // BufferQueueProducer on the rendner thread but are still pending the callback to report their + // their frame metrics. + uint64_t nextFrameNumber = getFrameNumber(); + observer->reportMetricsFrom(nextFrameNumber, mSurfaceControlGenerationId); + mFrameMetricsReporter->addObserver(observer); +} + +void CanvasContext::removeFrameMetricsObserver(FrameMetricsObserver* observer) { + std::scoped_lock lock(mFrameMetricsReporterMutex); + if (mFrameMetricsReporter.get() != nullptr) { + mFrameMetricsReporter->removeObserver(observer); + if (!mFrameMetricsReporter->hasObservers()) { + mFrameMetricsReporter.reset(nullptr); } } +} + +FrameInfo* CanvasContext::getFrameInfoFromLast4(uint64_t frameNumber, uint32_t surfaceControlId) { + std::scoped_lock lock(mLast4FrameMetricsInfosMutex); + for (size_t i = 0; i < mLast4FrameMetricsInfos.size(); i++) { + if (mLast4FrameMetricsInfos[i].frameNumber == frameNumber && + mLast4FrameMetricsInfos[i].surfaceId == surfaceControlId) { + return mLast4FrameMetricsInfos[i].frameInfo; + } + } + return nullptr; } -void CanvasContext::onSurfaceStatsAvailable(void* context, ASurfaceControl* control, - ASurfaceControlStats* stats) { - - CanvasContext* instance = static_cast(context); +void CanvasContext::onSurfaceStatsAvailable(void* context, int32_t surfaceControlId, + ASurfaceControlStats* stats) { + auto* instance = static_cast(context); const ASurfaceControlFunctions& functions = instance->mRenderThread.getASurfaceControlFunctions(); @@ -706,14 +741,15 @@ void CanvasContext::onSurfaceStatsAvailable(void* context, ASurfaceControl* cont nsecs_t gpuCompleteTime = functions.getAcquireTimeFunc(stats); uint64_t frameNumber = functions.getFrameNumberFunc(stats); - FrameInfo* frameInfo = instance->getFrameInfoFromLast4(frameNumber); + FrameInfo* frameInfo = instance->getFrameInfoFromLast4(frameNumber, surfaceControlId); if (frameInfo != nullptr) { frameInfo->set(FrameInfoIndex::FrameCompleted) = std::max(gpuCompleteTime, frameInfo->get(FrameInfoIndex::SwapBuffersCompleted)); frameInfo->set(FrameInfoIndex::GpuCompleted) = gpuCompleteTime; std::scoped_lock lock(instance->mFrameMetricsReporterMutex); - instance->mJankTracker.finishFrame(*frameInfo, instance->mFrameMetricsReporter); + instance->mJankTracker.finishFrame(*frameInfo, instance->mFrameMetricsReporter, frameNumber, + surfaceControlId); } } @@ -854,9 +890,9 @@ void CanvasContext::enqueueFrameWork(std::function&& func) { mFrameFences.push_back(CommonPool::async(std::move(func))); } -int64_t CanvasContext::getFrameNumber() { - // mFrameNumber is reset to -1 when the surface changes or we swap buffers - if (mFrameNumber == -1 && mNativeSurface.get()) { +uint64_t CanvasContext::getFrameNumber() { + // mFrameNumber is reset to 0 when the surface changes or we swap buffers + if (mFrameNumber == 0 && mNativeSurface.get()) { mFrameNumber = ANativeWindow_getNextFrameId(mNativeSurface->getNativeWindow()); } return mFrameNumber; diff --git a/libs/hwui/renderthread/CanvasContext.h b/libs/hwui/renderthread/CanvasContext.h index ed4a6209285f8..0caf76a265390 100644 --- a/libs/hwui/renderthread/CanvasContext.h +++ b/libs/hwui/renderthread/CanvasContext.h @@ -167,29 +167,13 @@ public: void setContentDrawBounds(const Rect& bounds) { mContentDrawBounds = bounds; } - void addFrameMetricsObserver(FrameMetricsObserver* observer) { - std::scoped_lock lock(mFrameMetricsReporterMutex); - if (mFrameMetricsReporter.get() == nullptr) { - mFrameMetricsReporter.reset(new FrameMetricsReporter()); - } - - mFrameMetricsReporter->addObserver(observer); - } - - void removeFrameMetricsObserver(FrameMetricsObserver* observer) { - std::scoped_lock lock(mFrameMetricsReporterMutex); - if (mFrameMetricsReporter.get() != nullptr) { - mFrameMetricsReporter->removeObserver(observer); - if (!mFrameMetricsReporter->hasObservers()) { - mFrameMetricsReporter.reset(nullptr); - } - } - } + void addFrameMetricsObserver(FrameMetricsObserver* observer); + void removeFrameMetricsObserver(FrameMetricsObserver* observer); // Used to queue up work that needs to be completed before this frame completes void enqueueFrameWork(std::function&& func); - int64_t getFrameNumber(); + uint64_t getFrameNumber(); void waitOnFences(); @@ -212,8 +196,8 @@ public: SkISize getNextFrameSize() const; // Called when SurfaceStats are available. - static void onSurfaceStatsAvailable(void* context, ASurfaceControl* control, - ASurfaceControlStats* stats); + static void onSurfaceStatsAvailable(void* context, int32_t surfaceControlId, + ASurfaceControlStats* stats); void setASurfaceTransactionCallback( const std::function& callback) { @@ -254,7 +238,13 @@ private: */ void reportMetricsWithPresentTime(); - FrameInfo* getFrameInfoFromLast4(uint64_t frameNumber); + struct FrameMetricsInfo { + FrameInfo* frameInfo; + int64_t frameNumber; + int32_t surfaceId; + }; + + FrameInfo* getFrameInfoFromLast4(uint64_t frameNumber, uint32_t surfaceControlId); // The same type as Frame.mWidth and Frame.mHeight int32_t mLastFrameWidth = 0; @@ -266,7 +256,10 @@ private: // NULL to remove the reference ASurfaceControl* mSurfaceControl = nullptr; // id to track surface control changes and WebViewFunctor uses it to determine - // whether reparenting is needed + // whether reparenting is needed also used by FrameMetricsReporter to determine + // if a frame is from an "old" surface (i.e. one that existed before the + // observer was attched) and therefore shouldn't be reported. + // NOTE: It is important that this is an increasing counter. int32_t mSurfaceControlGenerationId = 0; // stopped indicates the CanvasContext will reject actual redraw operations, // and defer repaint until it is un-stopped @@ -288,7 +281,8 @@ private: // Need at least 4 because we do quad buffer. Add a 5th for good measure. RingBuffer mSwapHistory; - int64_t mFrameNumber = -1; + // Frame numbers start at 1, 0 means uninitialized + uint64_t mFrameNumber = 0; int64_t mDamageId = 0; // last vsync for a dropped frame due to stuffed queue @@ -308,10 +302,11 @@ private: FrameInfo* mCurrentFrameInfo = nullptr; - // List of frames that are awaiting GPU completion reporting - RingBuffer, 4> mLast4FrameInfos - GUARDED_BY(mLast4FrameInfosMutex); - std::mutex mLast4FrameInfosMutex; + // List of data of frames that are awaiting GPU completion reporting. Used to compute frame + // metrics and determine whether or not to report the metrics. + RingBuffer mLast4FrameMetricsInfos + GUARDED_BY(mLast4FrameMetricsInfosMutex); + std::mutex mLast4FrameMetricsInfosMutex; std::string mName; JankTracker mJankTracker; diff --git a/libs/hwui/renderthread/RenderThread.h b/libs/hwui/renderthread/RenderThread.h index 05d225b856dbf..0b81fc04edf56 100644 --- a/libs/hwui/renderthread/RenderThread.h +++ b/libs/hwui/renderthread/RenderThread.h @@ -83,8 +83,9 @@ typedef ASurfaceControl* (*ASC_create)(ASurfaceControl* parent, const char* debu typedef void (*ASC_acquire)(ASurfaceControl* control); typedef void (*ASC_release)(ASurfaceControl* control); -typedef void (*ASC_registerSurfaceStatsListener)(ASurfaceControl* control, void* context, - ASurfaceControl_SurfaceStatsListener func); +typedef void (*ASC_registerSurfaceStatsListener)(ASurfaceControl* control, int32_t id, + void* context, + ASurfaceControl_SurfaceStatsListener func); typedef void (*ASC_unregisterSurfaceStatsListener)(void* context, ASurfaceControl_SurfaceStatsListener func); diff --git a/libs/hwui/tests/unit/FrameMetricsReporterTests.cpp b/libs/hwui/tests/unit/FrameMetricsReporterTests.cpp new file mode 100644 index 0000000000000..fb04700bbf703 --- /dev/null +++ b/libs/hwui/tests/unit/FrameMetricsReporterTests.cpp @@ -0,0 +1,205 @@ +/* + * Copyright (C) 2021 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include +#include + +#include +#include +#include + +using namespace android; +using namespace android::uirenderer; + +using ::testing::NotNull; + +class TestFrameMetricsObserver : public FrameMetricsObserver { +public: + explicit TestFrameMetricsObserver(bool waitForPresentTime) + : FrameMetricsObserver(waitForPresentTime){}; + + MOCK_METHOD(void, notify, (const int64_t* buffer), (override)); +}; + +TEST(FrameMetricsReporter, reportsAllFramesIfNoFromFrameIsSpecified) { + auto reporter = std::make_shared(); + + auto observer = sp::make(false /*waitForPresentTime*/); + EXPECT_CALL(*observer, notify).Times(4); + + reporter->addObserver(observer.get()); + + const int64_t* stats; + bool hasPresentTime = false; + uint64_t frameNumber = 1; + int32_t surfaceControlId = 0; + reporter->reportFrameMetrics(stats, hasPresentTime, frameNumber, surfaceControlId); + + frameNumber = 10; + surfaceControlId = 0; + reporter->reportFrameMetrics(stats, hasPresentTime, frameNumber, surfaceControlId); + + frameNumber = 0; + surfaceControlId = 2; + reporter->reportFrameMetrics(stats, hasPresentTime, frameNumber, surfaceControlId); + + frameNumber = 10; + surfaceControlId = 2; + reporter->reportFrameMetrics(stats, hasPresentTime, frameNumber, surfaceControlId); +} + +TEST(FrameMetricsReporter, respectsWaitForPresentTimeUnset) { + auto reporter = std::make_shared(); + + auto observer = sp::make(false /*waitForPresentTime*/); + reporter->addObserver(observer.get()); + + const int64_t* stats; + bool hasPresentTime = false; + uint64_t frameNumber = 3; + int32_t surfaceControlId = 0; + + EXPECT_CALL(*observer, notify).Times(1); + reporter->reportFrameMetrics(stats, hasPresentTime, frameNumber, surfaceControlId); + + EXPECT_CALL(*observer, notify).Times(0); + hasPresentTime = true; + reporter->reportFrameMetrics(stats, hasPresentTime, frameNumber, surfaceControlId); +} + +TEST(FrameMetricsReporter, respectsWaitForPresentTimeSet) { + auto reporter = std::make_shared(); + + auto observer = sp::make(true /*waitForPresentTime*/); + reporter->addObserver(observer.get()); + + const int64_t* stats; + bool hasPresentTime = false; + uint64_t frameNumber = 3; + int32_t surfaceControlId = 0; + + EXPECT_CALL(*observer, notify).Times(0); + reporter->reportFrameMetrics(stats, hasPresentTime, frameNumber, surfaceControlId); + + EXPECT_CALL(*observer, notify).Times(1); + hasPresentTime = true; + reporter->reportFrameMetrics(stats, hasPresentTime, frameNumber, surfaceControlId); +} + +TEST(FrameMetricsReporter, reportsAllFramesAfterSpecifiedFromFrame) { + const int64_t* stats; + bool hasPresentTime = false; + + std::vector frameNumbers{0, 1, 10}; + std::vector surfaceControlIds{0, 1, 10}; + for (uint64_t frameNumber : frameNumbers) { + for (int32_t surfaceControlId : surfaceControlIds) { + auto reporter = std::make_shared(); + + auto observer = + sp::make(hasPresentTime /*waitForPresentTime*/); + observer->reportMetricsFrom(frameNumber, surfaceControlId); + reporter->addObserver(observer.get()); + + EXPECT_CALL(*observer, notify).Times(8); + reporter->reportFrameMetrics(stats, hasPresentTime, frameNumber, surfaceControlId); + reporter->reportFrameMetrics(stats, hasPresentTime, frameNumber + 1, surfaceControlId); + reporter->reportFrameMetrics(stats, hasPresentTime, frameNumber + 10, surfaceControlId); + reporter->reportFrameMetrics(stats, hasPresentTime, frameNumber, surfaceControlId + 1); + reporter->reportFrameMetrics(stats, hasPresentTime, frameNumber - 1, + surfaceControlId + 1); + reporter->reportFrameMetrics(stats, hasPresentTime, frameNumber + 1, + surfaceControlId + 1); + reporter->reportFrameMetrics(stats, hasPresentTime, frameNumber + 10, + surfaceControlId + 1); + reporter->reportFrameMetrics(stats, hasPresentTime, frameNumber + 10, + surfaceControlId + 10); + } + } +} + +TEST(FrameMetricsReporter, doesNotReportsFramesBeforeSpecifiedFromFrame) { + const int64_t* stats; + bool hasPresentTime = false; + + std::vector frameNumbers{1, 10}; + std::vector surfaceControlIds{0, 1, 10}; + for (uint64_t frameNumber : frameNumbers) { + for (int32_t surfaceControlId : surfaceControlIds) { + auto reporter = std::make_shared(); + + auto observer = + sp::make(hasPresentTime /*waitForPresentTime*/); + observer->reportMetricsFrom(frameNumber, surfaceControlId); + reporter->addObserver(observer.get()); + + EXPECT_CALL(*observer, notify).Times(0); + reporter->reportFrameMetrics(stats, hasPresentTime, frameNumber - 1, surfaceControlId); + if (surfaceControlId > 0) { + reporter->reportFrameMetrics(stats, hasPresentTime, frameNumber, + surfaceControlId - 1); + reporter->reportFrameMetrics(stats, hasPresentTime, frameNumber - 1, + surfaceControlId - 1); + } + } + } +} + +TEST(FrameMetricsReporter, canRemoveObservers) { + const int64_t* stats; + bool hasPresentTime = false; + uint64_t frameNumber = 3; + int32_t surfaceControlId = 0; + + auto reporter = std::make_shared(); + + auto observer = sp::make(hasPresentTime /*waitForPresentTime*/); + + observer->reportMetricsFrom(frameNumber, surfaceControlId); + reporter->addObserver(observer.get()); + + EXPECT_CALL(*observer, notify).Times(1); + reporter->reportFrameMetrics(stats, hasPresentTime, frameNumber, surfaceControlId); + + ASSERT_TRUE(reporter->removeObserver(observer.get())); + + EXPECT_CALL(*observer, notify).Times(0); + reporter->reportFrameMetrics(stats, hasPresentTime, frameNumber, surfaceControlId); +} + +TEST(FrameMetricsReporter, canSupportMultipleObservers) { + const int64_t* stats; + bool hasPresentTime = false; + uint64_t frameNumber = 3; + int32_t surfaceControlId = 0; + + auto reporter = std::make_shared(); + + auto observer1 = sp::make(hasPresentTime /*waitForPresentTime*/); + auto observer2 = sp::make(hasPresentTime /*waitForPresentTime*/); + observer1->reportMetricsFrom(frameNumber, surfaceControlId); + observer2->reportMetricsFrom(frameNumber + 10, surfaceControlId + 1); + reporter->addObserver(observer1.get()); + reporter->addObserver(observer2.get()); + + EXPECT_CALL(*observer1, notify).Times(1); + EXPECT_CALL(*observer2, notify).Times(0); + reporter->reportFrameMetrics(stats, hasPresentTime, frameNumber, surfaceControlId); + + EXPECT_CALL(*observer1, notify).Times(1); + EXPECT_CALL(*observer2, notify).Times(1); + reporter->reportFrameMetrics(stats, hasPresentTime, frameNumber + 10, surfaceControlId + 1); +} diff --git a/libs/hwui/tests/unit/JankTrackerTests.cpp b/libs/hwui/tests/unit/JankTrackerTests.cpp index f467ebf5d8881..5b397de36a869 100644 --- a/libs/hwui/tests/unit/JankTrackerTests.cpp +++ b/libs/hwui/tests/unit/JankTrackerTests.cpp @@ -34,6 +34,9 @@ TEST(JankTracker, noJank) { JankTracker jankTracker(&container); std::unique_ptr reporter = std::make_unique(); + uint64_t frameNumber = 0; + uint32_t surfaceId = 0; + FrameInfo* info = jankTracker.startFrame(); info->set(FrameInfoIndex::IntendedVsync) = 100_ms; info->set(FrameInfoIndex::Vsync) = 101_ms; @@ -42,7 +45,7 @@ TEST(JankTracker, noJank) { info->set(FrameInfoIndex::FrameCompleted) = 115_ms; info->set(FrameInfoIndex::FrameInterval) = 16_ms; info->set(FrameInfoIndex::FrameDeadline) = 120_ms; - jankTracker.finishFrame(*info, reporter); + jankTracker.finishFrame(*info, reporter, frameNumber, surfaceId); info = jankTracker.startFrame(); info->set(FrameInfoIndex::IntendedVsync) = 116_ms; @@ -52,7 +55,7 @@ TEST(JankTracker, noJank) { info->set(FrameInfoIndex::FrameCompleted) = 131_ms; info->set(FrameInfoIndex::FrameInterval) = 16_ms; info->set(FrameInfoIndex::FrameDeadline) = 136_ms; - jankTracker.finishFrame(*info, reporter); + jankTracker.finishFrame(*info, reporter, frameNumber, surfaceId); ASSERT_EQ(2, container.get()->totalFrameCount()); ASSERT_EQ(0, container.get()->jankFrameCount()); @@ -65,6 +68,9 @@ TEST(JankTracker, jank) { JankTracker jankTracker(&container); std::unique_ptr reporter = std::make_unique(); + uint64_t frameNumber = 0; + uint32_t surfaceId = 0; + FrameInfo* info = jankTracker.startFrame(); info->set(FrameInfoIndex::IntendedVsync) = 100_ms; info->set(FrameInfoIndex::Vsync) = 101_ms; @@ -73,7 +79,7 @@ TEST(JankTracker, jank) { info->set(FrameInfoIndex::FrameCompleted) = 121_ms; info->set(FrameInfoIndex::FrameInterval) = 16_ms; info->set(FrameInfoIndex::FrameDeadline) = 120_ms; - jankTracker.finishFrame(*info, reporter); + jankTracker.finishFrame(*info, reporter, frameNumber, surfaceId); ASSERT_EQ(1, container.get()->totalFrameCount()); ASSERT_EQ(1, container.get()->jankFrameCount()); @@ -85,6 +91,9 @@ TEST(JankTracker, legacyJankButNoRealJank) { JankTracker jankTracker(&container); std::unique_ptr reporter = std::make_unique(); + uint64_t frameNumber = 0; + uint32_t surfaceId = 0; + FrameInfo* info = jankTracker.startFrame(); info->set(FrameInfoIndex::IntendedVsync) = 100_ms; info->set(FrameInfoIndex::Vsync) = 101_ms; @@ -93,7 +102,7 @@ TEST(JankTracker, legacyJankButNoRealJank) { info->set(FrameInfoIndex::FrameCompleted) = 118_ms; info->set(FrameInfoIndex::FrameInterval) = 16_ms; info->set(FrameInfoIndex::FrameDeadline) = 120_ms; - jankTracker.finishFrame(*info, reporter); + jankTracker.finishFrame(*info, reporter, frameNumber, surfaceId); ASSERT_EQ(1, container.get()->totalFrameCount()); ASSERT_EQ(0, container.get()->jankFrameCount()); @@ -106,6 +115,9 @@ TEST(JankTracker, doubleStuffed) { JankTracker jankTracker(&container); std::unique_ptr reporter = std::make_unique(); + uint64_t frameNumber = 0; + uint32_t surfaceId = 0; + // First frame janks FrameInfo* info = jankTracker.startFrame(); info->set(FrameInfoIndex::IntendedVsync) = 100_ms; @@ -115,7 +127,7 @@ TEST(JankTracker, doubleStuffed) { info->set(FrameInfoIndex::FrameCompleted) = 121_ms; info->set(FrameInfoIndex::FrameInterval) = 16_ms; info->set(FrameInfoIndex::FrameDeadline) = 120_ms; - jankTracker.finishFrame(*info, reporter); + jankTracker.finishFrame(*info, reporter, frameNumber, surfaceId); ASSERT_EQ(1, container.get()->jankFrameCount()); @@ -128,7 +140,7 @@ TEST(JankTracker, doubleStuffed) { info->set(FrameInfoIndex::FrameCompleted) = 137_ms; info->set(FrameInfoIndex::FrameInterval) = 16_ms; info->set(FrameInfoIndex::FrameDeadline) = 136_ms; - jankTracker.finishFrame(*info, reporter); + jankTracker.finishFrame(*info, reporter, frameNumber, surfaceId); ASSERT_EQ(2, container.get()->totalFrameCount()); ASSERT_EQ(1, container.get()->jankFrameCount()); @@ -140,6 +152,9 @@ TEST(JankTracker, doubleStuffedThenPauseThenJank) { JankTracker jankTracker(&container); std::unique_ptr reporter = std::make_unique(); + uint64_t frameNumber = 0; + uint32_t surfaceId = 0; + // First frame janks FrameInfo* info = jankTracker.startFrame(); info->set(FrameInfoIndex::IntendedVsync) = 100_ms; @@ -149,7 +164,7 @@ TEST(JankTracker, doubleStuffedThenPauseThenJank) { info->set(FrameInfoIndex::FrameCompleted) = 121_ms; info->set(FrameInfoIndex::FrameInterval) = 16_ms; info->set(FrameInfoIndex::FrameDeadline) = 120_ms; - jankTracker.finishFrame(*info, reporter); + jankTracker.finishFrame(*info, reporter, frameNumber, surfaceId); ASSERT_EQ(1, container.get()->jankFrameCount()); @@ -162,7 +177,7 @@ TEST(JankTracker, doubleStuffedThenPauseThenJank) { info->set(FrameInfoIndex::FrameCompleted) = 137_ms; info->set(FrameInfoIndex::FrameInterval) = 16_ms; info->set(FrameInfoIndex::FrameDeadline) = 136_ms; - jankTracker.finishFrame(*info, reporter); + jankTracker.finishFrame(*info, reporter, frameNumber, surfaceId); ASSERT_EQ(1, container.get()->jankFrameCount()); @@ -175,8 +190,8 @@ TEST(JankTracker, doubleStuffedThenPauseThenJank) { info->set(FrameInfoIndex::FrameCompleted) = 169_ms; info->set(FrameInfoIndex::FrameInterval) = 16_ms; info->set(FrameInfoIndex::FrameDeadline) = 168_ms; - jankTracker.finishFrame(*info, reporter); + jankTracker.finishFrame(*info, reporter, frameNumber, surfaceId); ASSERT_EQ(3, container.get()->totalFrameCount()); ASSERT_EQ(2, container.get()->jankFrameCount()); -} \ No newline at end of file +} diff --git a/native/android/surface_control.cpp b/native/android/surface_control.cpp index 693a027bd0e2d..7f74dd4c33c31 100644 --- a/native/android/surface_control.cpp +++ b/native/android/surface_control.cpp @@ -146,28 +146,24 @@ struct ASurfaceControlStats { uint64_t frameNumber; }; -void ASurfaceControl_registerSurfaceStatsListener(ASurfaceControl* control, void* context, - ASurfaceControl_SurfaceStatsListener func) { - SurfaceStatsCallback callback = [func](void* callback_context, - nsecs_t, - const sp&, - const SurfaceStats& surfaceStats) { - +void ASurfaceControl_registerSurfaceStatsListener(ASurfaceControl* control, int32_t id, + void* context, + ASurfaceControl_SurfaceStatsListener func) { + SurfaceStatsCallback callback = [func, id](void* callback_context, nsecs_t, const sp&, + const SurfaceStats& surfaceStats) { ASurfaceControlStats aSurfaceControlStats; - ASurfaceControl* aSurfaceControl = - reinterpret_cast(surfaceStats.surfaceControl.get()); aSurfaceControlStats.acquireTime = surfaceStats.acquireTime; aSurfaceControlStats.previousReleaseFence = surfaceStats.previousReleaseFence; aSurfaceControlStats.frameNumber = surfaceStats.eventStats.frameNumber; - (*func)(callback_context, aSurfaceControl, &aSurfaceControlStats); + (*func)(callback_context, id, &aSurfaceControlStats); }; + TransactionCompletedListener::getInstance()->addSurfaceStatsListener(context, reinterpret_cast(func), ASurfaceControl_to_SurfaceControl(control), callback); } - void ASurfaceControl_unregisterSurfaceStatsListener(void* context, ASurfaceControl_SurfaceStatsListener func) { TransactionCompletedListener::getInstance()->removeSurfaceStatsListener(context, @@ -662,4 +658,4 @@ void ASurfaceTransaction_setOnCommit(ASurfaceTransaction* aSurfaceTransaction, v Transaction* transaction = ASurfaceTransaction_to_Transaction(aSurfaceTransaction); transaction->addTransactionCommittedCallback(callback, context); -} \ No newline at end of file +}