Jank callback API refactor.
Removes the old work-arounds for missing jank callbacks.
Removes the jank data from the transaction completed callback.
Adds new function to ISurfaceComposer to register jank listeners.
With the new API, jank data is only sent over binder periodically
(every ~50 frames) and on a background thread. It is also only tracked
for layers where there is a listener registered.
Test: manual, libsurfaceflinger_unittest
Bug: http://b/336461947
Flag: EXEMPT refactor
Change-Id: I3238ce604571832523525cf098832c7352879826
diff --git a/services/surfaceflinger/Android.bp b/services/surfaceflinger/Android.bp
index 1b6c598..a37433c 100644
--- a/services/surfaceflinger/Android.bp
+++ b/services/surfaceflinger/Android.bp
@@ -85,7 +85,7 @@
"libui",
"libutils",
"libSurfaceFlingerProp",
- "libaconfig_storage_read_api_cc"
+ "libaconfig_storage_read_api_cc",
],
static_libs: [
"iinputflinger_aidl_lib_static",
@@ -187,6 +187,7 @@
"FrameTracker.cpp",
"HdrLayerInfoReporter.cpp",
"HdrSdrRatioOverlay.cpp",
+ "Jank/JankTracker.cpp",
"WindowInfosListenerInvoker.cpp",
"Layer.cpp",
"LayerFE.cpp",
diff --git a/services/surfaceflinger/FrameTimeline/FrameTimeline.cpp b/services/surfaceflinger/FrameTimeline/FrameTimeline.cpp
index 2596a25..f671006 100644
--- a/services/surfaceflinger/FrameTimeline/FrameTimeline.cpp
+++ b/services/surfaceflinger/FrameTimeline/FrameTimeline.cpp
@@ -30,6 +30,8 @@
#include <numeric>
#include <unordered_set>
+#include "../Jank/JankTracker.h"
+
namespace android::frametimeline {
using base::StringAppendF;
@@ -685,6 +687,13 @@
mTimeStats->incrementJankyFrames({refreshRate, mRenderRate, mOwnerUid, mLayerName,
mGameMode, mJankType, displayDeadlineDelta,
displayPresentDelta, deadlineDelta});
+
+ gui::JankData jd;
+ jd.frameVsyncId = mToken;
+ jd.jankType = mJankType;
+ jd.frameIntervalNs =
+ (mRenderRate ? *mRenderRate : mDisplayFrameRenderRate).getPeriodNsecs();
+ JankTracker::onJankData(mLayerId, jd);
}
}
diff --git a/services/surfaceflinger/Jank/JankTracker.cpp b/services/surfaceflinger/Jank/JankTracker.cpp
new file mode 100644
index 0000000..8e0e084
--- /dev/null
+++ b/services/surfaceflinger/Jank/JankTracker.cpp
@@ -0,0 +1,206 @@
+/*
+ * Copyright 2024 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 "JankTracker.h"
+
+#include <android/gui/IJankListener.h>
+#include "BackgroundExecutor.h"
+
+namespace android {
+
+namespace {
+
+constexpr size_t kJankDataBatchSize = 50;
+
+} // anonymous namespace
+
+std::atomic<size_t> JankTracker::sListenerCount(0);
+std::atomic<bool> JankTracker::sCollectAllJankDataForTesting(false);
+
+JankTracker::~JankTracker() {}
+
+void JankTracker::addJankListener(int32_t layerId, sp<IBinder> listener) {
+ // Increment right away, so that if an onJankData call comes in before the background thread has
+ // added this listener, it will not drop the data.
+ sListenerCount++;
+
+ BackgroundExecutor::getLowPriorityInstance().sendCallbacks(
+ {[layerId, listener = std::move(listener)]() {
+ JankTracker& tracker = getInstance();
+ const std::lock_guard<std::mutex> _l(tracker.mLock);
+ tracker.addJankListenerLocked(layerId, listener);
+ }});
+}
+
+void JankTracker::flushJankData(int32_t layerId) {
+ BackgroundExecutor::getLowPriorityInstance().sendCallbacks(
+ {[layerId]() { getInstance().doFlushJankData(layerId); }});
+}
+
+void JankTracker::removeJankListener(int32_t layerId, sp<IBinder> listener, int64_t afterVsync) {
+ BackgroundExecutor::getLowPriorityInstance().sendCallbacks(
+ {[layerId, listener = std::move(listener), afterVsync]() {
+ JankTracker& tracker = getInstance();
+ const std::lock_guard<std::mutex> _l(tracker.mLock);
+ tracker.markJankListenerForRemovalLocked(layerId, listener, afterVsync);
+ }});
+}
+
+void JankTracker::onJankData(int32_t layerId, gui::JankData data) {
+ if (sListenerCount == 0) {
+ return;
+ }
+
+ BackgroundExecutor::getLowPriorityInstance().sendCallbacks(
+ {[layerId, data = std::move(data)]() {
+ JankTracker& tracker = getInstance();
+
+ tracker.mLock.lock();
+ bool hasListeners = tracker.mJankListeners.count(layerId) > 0;
+ tracker.mLock.unlock();
+
+ if (!hasListeners && !sCollectAllJankDataForTesting) {
+ return;
+ }
+
+ tracker.mJankDataLock.lock();
+ tracker.mJankData.emplace(layerId, data);
+ size_t count = tracker.mJankData.count(layerId);
+ tracker.mJankDataLock.unlock();
+
+ if (count >= kJankDataBatchSize && !sCollectAllJankDataForTesting) {
+ tracker.doFlushJankData(layerId);
+ }
+ }});
+}
+
+void JankTracker::addJankListenerLocked(int32_t layerId, sp<IBinder> listener) {
+ for (auto it = mJankListeners.find(layerId); it != mJankListeners.end(); it++) {
+ if (it->second.mListener == listener) {
+ // Undo the duplicate increment in addJankListener.
+ sListenerCount--;
+ return;
+ }
+ }
+
+ mJankListeners.emplace(layerId, std::move(listener));
+}
+
+void JankTracker::doFlushJankData(int32_t layerId) {
+ std::vector<gui::JankData> jankData;
+ int64_t maxVsync = transferAvailableJankData(layerId, jankData);
+
+ std::vector<sp<IBinder>> toSend;
+
+ mLock.lock();
+ for (auto it = mJankListeners.find(layerId); it != mJankListeners.end();) {
+ if (!jankData.empty()) {
+ toSend.emplace_back(it->second.mListener);
+ }
+
+ int64_t removeAfter = it->second.mRemoveAfter;
+ if (removeAfter != -1 && removeAfter <= maxVsync) {
+ it = mJankListeners.erase(it);
+ sListenerCount--;
+ } else {
+ it++;
+ }
+ }
+ mLock.unlock();
+
+ for (const auto& listener : toSend) {
+ binder::Status status = interface_cast<gui::IJankListener>(listener)->onJankData(jankData);
+ if (status.exceptionCode() == binder::Status::EX_NULL_POINTER) {
+ // Remove any listeners, where the App side has gone away, without
+ // deregistering.
+ dropJankListener(layerId, listener);
+ }
+ }
+}
+
+void JankTracker::markJankListenerForRemovalLocked(int32_t layerId, sp<IBinder> listener,
+ int64_t afterVysnc) {
+ for (auto it = mJankListeners.find(layerId); it != mJankListeners.end(); it++) {
+ if (it->second.mListener == listener) {
+ it->second.mRemoveAfter = std::max(static_cast<int64_t>(0), afterVysnc);
+ return;
+ }
+ }
+}
+
+int64_t JankTracker::transferAvailableJankData(int32_t layerId,
+ std::vector<gui::JankData>& outJankData) {
+ const std::lock_guard<std::mutex> _l(mJankDataLock);
+ int64_t maxVsync = 0;
+ auto range = mJankData.equal_range(layerId);
+ for (auto it = range.first; it != range.second;) {
+ maxVsync = std::max(it->second.frameVsyncId, maxVsync);
+ outJankData.emplace_back(std::move(it->second));
+ it = mJankData.erase(it);
+ }
+ return maxVsync;
+}
+
+void JankTracker::dropJankListener(int32_t layerId, sp<IBinder> listener) {
+ const std::lock_guard<std::mutex> _l(mLock);
+ for (auto it = mJankListeners.find(layerId); it != mJankListeners.end(); it++) {
+ if (it->second.mListener == listener) {
+ mJankListeners.erase(it);
+ sListenerCount--;
+ return;
+ }
+ }
+}
+
+void JankTracker::clearAndStartCollectingAllJankDataForTesting() {
+ BackgroundExecutor::getLowPriorityInstance().flushQueue();
+
+ // Clear all past tracked jank data.
+ JankTracker& tracker = getInstance();
+ const std::lock_guard<std::mutex> _l(tracker.mJankDataLock);
+ tracker.mJankData.clear();
+
+ // Pretend there's at least one listener.
+ sListenerCount++;
+ sCollectAllJankDataForTesting = true;
+}
+
+std::vector<gui::JankData> JankTracker::getCollectedJankDataForTesting(int32_t layerId) {
+ JankTracker& tracker = getInstance();
+ const std::lock_guard<std::mutex> _l(tracker.mJankDataLock);
+
+ auto range = tracker.mJankData.equal_range(layerId);
+ std::vector<gui::JankData> result;
+ std::transform(range.first, range.second, std::back_inserter(result),
+ [](std::pair<int32_t, gui::JankData> layerIdToJankData) {
+ return layerIdToJankData.second;
+ });
+
+ return result;
+}
+
+void JankTracker::clearAndStopCollectingAllJankDataForTesting() {
+ // Undo startCollectingAllJankDataForTesting.
+ sListenerCount--;
+ sCollectAllJankDataForTesting = false;
+
+ // Clear all tracked jank data.
+ JankTracker& tracker = getInstance();
+ const std::lock_guard<std::mutex> _l(tracker.mJankDataLock);
+ tracker.mJankData.clear();
+}
+
+} // namespace android
diff --git a/services/surfaceflinger/Jank/JankTracker.h b/services/surfaceflinger/Jank/JankTracker.h
new file mode 100644
index 0000000..5917358
--- /dev/null
+++ b/services/surfaceflinger/Jank/JankTracker.h
@@ -0,0 +1,99 @@
+/*
+ * Copyright 2024 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.
+ */
+
+#pragma once
+
+#include <cstdint>
+#include <mutex>
+#include <unordered_map>
+
+#include <android/gui/JankData.h>
+#include <binder/IBinder.h>
+#include <utils/Mutex.h>
+
+namespace android {
+namespace frametimeline {
+class FrameTimelineTest;
+}
+
+/**
+ * JankTracker maintains a backlog of frame jank classification and manages and notififies any
+ * registered jank data listeners.
+ */
+class JankTracker {
+public:
+ ~JankTracker();
+
+ static void addJankListener(int32_t layerId, sp<IBinder> listener);
+ static void flushJankData(int32_t layerId);
+ static void removeJankListener(int32_t layerId, sp<IBinder> listener, int64_t afterVysnc);
+
+ static void onJankData(int32_t layerId, gui::JankData data);
+
+protected:
+ // The following methods can be used to force the tracker to collect all jank data and not
+ // flush it for a short time period and should *only* be used for testing. Every call to
+ // clearAndStartCollectingAllJankDataForTesting needs to be followed by a call to
+ // clearAndStopCollectingAllJankDataForTesting.
+ static void clearAndStartCollectingAllJankDataForTesting();
+ static std::vector<gui::JankData> getCollectedJankDataForTesting(int32_t layerId);
+ static void clearAndStopCollectingAllJankDataForTesting();
+
+ friend class frametimeline::FrameTimelineTest;
+
+private:
+ JankTracker() {}
+ JankTracker(const JankTracker&) = delete;
+ JankTracker(JankTracker&&) = delete;
+
+ JankTracker& operator=(const JankTracker&) = delete;
+ JankTracker& operator=(JankTracker&&) = delete;
+
+ static JankTracker& getInstance() {
+ static JankTracker instance;
+ return instance;
+ }
+
+ void addJankListenerLocked(int32_t layerId, sp<IBinder> listener) REQUIRES(mLock);
+ void doFlushJankData(int32_t layerId);
+ void markJankListenerForRemovalLocked(int32_t layerId, sp<IBinder> listener, int64_t afterVysnc)
+ REQUIRES(mLock);
+
+ int64_t transferAvailableJankData(int32_t layerId, std::vector<gui::JankData>& jankData);
+ void dropJankListener(int32_t layerId, sp<IBinder> listener);
+
+ struct Listener {
+ sp<IBinder> mListener;
+ int64_t mRemoveAfter;
+
+ Listener(sp<IBinder>&& listener) : mListener(listener), mRemoveAfter(-1) {}
+ };
+
+ // We keep track of the current listener count, so that the onJankData call, which is on the
+ // main thread, can short-curcuit the scheduling on the background thread (which involves
+ // locking) if there are no listeners registered, which is the most common case.
+ static std::atomic<size_t> sListenerCount;
+ static std::atomic<bool> sCollectAllJankDataForTesting;
+
+ std::mutex mLock;
+ std::unordered_multimap<int32_t, Listener> mJankListeners GUARDED_BY(mLock);
+ std::mutex mJankDataLock;
+ std::unordered_multimap<int32_t, gui::JankData> mJankData GUARDED_BY(mJankDataLock);
+
+ friend class JankTrackerTest;
+};
+
+} // namespace android
diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index d27bfd2..e758741 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -767,41 +767,6 @@
return (p != nullptr) ? p->isSecure() : false;
}
-void Layer::transferAvailableJankData(const std::deque<sp<CallbackHandle>>& handles,
- std::vector<JankData>& jankData) {
- if (mPendingJankClassifications.empty() ||
- !mPendingJankClassifications.front()->getJankType()) {
- return;
- }
-
- bool includeJankData = false;
- for (const auto& handle : handles) {
- for (const auto& cb : handle->callbackIds) {
- if (cb.includeJankData) {
- includeJankData = true;
- break;
- }
- }
-
- if (includeJankData) {
- jankData.reserve(mPendingJankClassifications.size());
- break;
- }
- }
-
- while (!mPendingJankClassifications.empty() &&
- mPendingJankClassifications.front()->getJankType()) {
- if (includeJankData) {
- std::shared_ptr<frametimeline::SurfaceFrame> surfaceFrame =
- mPendingJankClassifications.front();
- jankData.emplace_back(JankData(surfaceFrame->getToken(),
- surfaceFrame->getJankType().value(),
- surfaceFrame->getRenderRate().getPeriodNsecs()));
- }
- mPendingJankClassifications.pop_front();
- }
-}
-
// ----------------------------------------------------------------------------
// transaction
// ----------------------------------------------------------------------------
@@ -1436,7 +1401,6 @@
if (fps) {
surfaceFrame->setRenderRate(*fps);
}
- onSurfaceFrameCreated(surfaceFrame);
return surfaceFrame;
}
@@ -1453,7 +1417,6 @@
if (fps) {
surfaceFrame->setRenderRate(*fps);
}
- onSurfaceFrameCreated(surfaceFrame);
return surfaceFrame;
}
@@ -1479,7 +1442,6 @@
if (fps) {
surfaceFrame->setRenderRate(*fps);
}
- onSurfaceFrameCreated(surfaceFrame);
addSurfaceFrameDroppedForBuffer(surfaceFrame, postTime);
}
@@ -2942,25 +2904,6 @@
}
}
-void Layer::onSurfaceFrameCreated(
- const std::shared_ptr<frametimeline::SurfaceFrame>& surfaceFrame) {
- while (mPendingJankClassifications.size() >= kPendingClassificationMaxSurfaceFrames) {
- // Too many SurfaceFrames pending classification. The front of the deque is probably not
- // tracked by FrameTimeline and will never be presented. This will only result in a memory
- // leak.
- if (hasBufferOrSidebandStreamInDrawing()) {
- // Only log for layers with a buffer, since we expect the jank data to be drained for
- // these, while there may be no jank listeners for bufferless layers.
- ALOGW("Removing the front of pending jank deque from layer - %s to prevent memory leak",
- mName.c_str());
- std::string miniDump = mPendingJankClassifications.front()->miniDump();
- ALOGD("Head SurfaceFrame mini dump\n%s", miniDump.c_str());
- }
- mPendingJankClassifications.pop_front();
- }
- mPendingJankClassifications.emplace_back(surfaceFrame);
-}
-
void Layer::releasePendingBuffer(nsecs_t dequeueReadyTime) {
for (const auto& handle : mDrawingState.callbackHandles) {
handle->transformHint = mTransformHint;
@@ -2978,10 +2921,7 @@
}
}
- std::vector<JankData> jankData;
- transferAvailableJankData(mDrawingState.callbackHandles, jankData);
- mFlinger->getTransactionCallbackInvoker().addCallbackHandles(mDrawingState.callbackHandles,
- jankData);
+ mFlinger->getTransactionCallbackInvoker().addCallbackHandles(mDrawingState.callbackHandles);
mDrawingState.callbackHandles = {};
}
@@ -3449,9 +3389,7 @@
if (!remainingHandles.empty()) {
// Notify the transaction completed threads these handles are done. These are only the
// handles that were not added to the mDrawingState, which will be notified later.
- std::vector<JankData> jankData;
- transferAvailableJankData(remainingHandles, jankData);
- mFlinger->getTransactionCallbackInvoker().addCallbackHandles(remainingHandles, jankData);
+ mFlinger->getTransactionCallbackInvoker().addCallbackHandles(remainingHandles);
}
mReleasePreviousBuffer = false;
diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h
index b9fcd5c..d3b56f8 100644
--- a/services/surfaceflinger/Layer.h
+++ b/services/surfaceflinger/Layer.h
@@ -980,7 +980,6 @@
void preparePerFrameBufferCompositionState();
void preparePerFrameEffectsCompositionState();
void gatherBufferInfo();
- void onSurfaceFrameCreated(const std::shared_ptr<frametimeline::SurfaceFrame>&);
bool isClonedFromAlive() { return getClonedFrom() != nullptr; }
@@ -1195,11 +1194,6 @@
bool hasSomethingToDraw() const { return hasEffect() || hasBufferOrSidebandStream(); }
- // Fills the provided vector with the currently available JankData and removes the processed
- // JankData from the pending list.
- void transferAvailableJankData(const std::deque<sp<CallbackHandle>>& handles,
- std::vector<JankData>& jankData);
-
bool shouldOverrideChildrenFrameRate() const {
return getDrawingState().frameRateSelectionStrategy ==
FrameRateSelectionStrategy::OverrideChildren;
@@ -1268,10 +1262,6 @@
// time.
std::variant<nsecs_t, sp<Fence>> mCallbackHandleAcquireTimeOrFence = -1;
- std::deque<std::shared_ptr<android::frametimeline::SurfaceFrame>> mPendingJankClassifications;
- // An upper bound on the number of SurfaceFrames in the pending classifications deque.
- static constexpr int kPendingClassificationMaxSurfaceFrames = 50;
-
const std::string mBlastTransactionName{"BufferTX - " + mName};
// This integer is incremented everytime a buffer arrives at the server for this layer,
// and decremented when a buffer is dropped or latched. When changed the integer is exported
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 5b40acf..989020d 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -143,6 +143,7 @@
#include "FrontEnd/LayerLog.h"
#include "FrontEnd/LayerSnapshot.h"
#include "HdrLayerInfoReporter.h"
+#include "Jank/JankTracker.h"
#include "Layer.h"
#include "LayerProtoHelper.h"
#include "LayerRenderArea.h"
@@ -5507,10 +5508,8 @@
}
if (layer == nullptr) {
for (auto& [listener, callbackIds] : s.listeners) {
- mTransactionCallbackInvoker.addCallbackHandle(sp<CallbackHandle>::make(listener,
- callbackIds,
- s.surface),
- std::vector<JankData>());
+ mTransactionCallbackInvoker.addCallbackHandle(
+ sp<CallbackHandle>::make(listener, callbackIds, s.surface));
}
return 0;
}
@@ -5868,10 +5867,8 @@
}
if (layer == nullptr) {
for (auto& [listener, callbackIds] : s.listeners) {
- mTransactionCallbackInvoker.addCallbackHandle(sp<CallbackHandle>::make(listener,
- callbackIds,
- s.surface),
- std::vector<JankData>());
+ mTransactionCallbackInvoker.addCallbackHandle(
+ sp<CallbackHandle>::make(listener, callbackIds, s.surface));
}
return 0;
}
@@ -6114,6 +6111,8 @@
mTransactionHandler.onLayerDestroyed(layerId);
}
+ JankTracker::flushJankData(layerId);
+
Mutex::Autolock lock(mStateLock);
markLayerPendingRemovalLocked(layer);
layer->onHandleDestroyed();
@@ -10463,6 +10462,28 @@
return ::android::binder::Status::ok();
}
+binder::Status SurfaceComposerAIDL::addJankListener(const sp<IBinder>& layerHandle,
+ const sp<gui::IJankListener>& listener) {
+ sp<Layer> layer = LayerHandle::getLayer(layerHandle);
+ if (layer == nullptr) {
+ return binder::Status::fromExceptionCode(binder::Status::EX_NULL_POINTER);
+ }
+ JankTracker::addJankListener(layer->sequence, IInterface::asBinder(listener));
+ return binder::Status::ok();
+}
+
+binder::Status SurfaceComposerAIDL::flushJankData(int32_t layerId) {
+ JankTracker::flushJankData(layerId);
+ return binder::Status::ok();
+}
+
+binder::Status SurfaceComposerAIDL::removeJankListener(int32_t layerId,
+ const sp<gui::IJankListener>& listener,
+ int64_t afterVsync) {
+ JankTracker::removeJankListener(layerId, IInterface::asBinder(listener), afterVsync);
+ return binder::Status::ok();
+}
+
status_t SurfaceComposerAIDL::checkAccessPermission(bool usePermissionCache) {
if (!mFlinger->callingThreadHasUnscopedSurfaceFlingerAccess(usePermissionCache)) {
IPCThreadState* ipc = IPCThreadState::self();
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index ee541c4..6838994 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -27,6 +27,7 @@
#include <android/gui/BnSurfaceComposer.h>
#include <android/gui/DisplayStatInfo.h>
#include <android/gui/DisplayState.h>
+#include <android/gui/IJankListener.h>
#include <android/gui/ISurfaceComposerClient.h>
#include <cutils/atomic.h>
#include <cutils/compiler.h>
@@ -1693,6 +1694,11 @@
int pid, std::optional<gui::StalledTransactionInfo>* outInfo) override;
binder::Status getSchedulingPolicy(gui::SchedulingPolicy* outPolicy) override;
binder::Status notifyShutdown() override;
+ binder::Status addJankListener(const sp<IBinder>& layer,
+ const sp<gui::IJankListener>& listener) override;
+ binder::Status flushJankData(int32_t layerId) override;
+ binder::Status removeJankListener(int32_t layerId, const sp<gui::IJankListener>& listener,
+ int64_t afterVsync) override;
private:
static const constexpr bool kUsePermissionCache = true;
diff --git a/services/surfaceflinger/TransactionCallbackInvoker.cpp b/services/surfaceflinger/TransactionCallbackInvoker.cpp
index 222ae30..b9555c0 100644
--- a/services/surfaceflinger/TransactionCallbackInvoker.cpp
+++ b/services/surfaceflinger/TransactionCallbackInvoker.cpp
@@ -64,13 +64,12 @@
if (handles.empty()) {
return NO_ERROR;
}
- const std::vector<JankData>& jankData = std::vector<JankData>();
for (const auto& handle : handles) {
if (!containsOnCommitCallbacks(handle->callbackIds)) {
outRemainingHandles.push_back(handle);
continue;
}
- status_t err = addCallbackHandle(handle, jankData);
+ status_t err = addCallbackHandle(handle);
if (err != NO_ERROR) {
return err;
}
@@ -80,12 +79,12 @@
}
status_t TransactionCallbackInvoker::addCallbackHandles(
- const std::deque<sp<CallbackHandle>>& handles, const std::vector<JankData>& jankData) {
+ const std::deque<sp<CallbackHandle>>& handles) {
if (handles.empty()) {
return NO_ERROR;
}
for (const auto& handle : handles) {
- status_t err = addCallbackHandle(handle, jankData);
+ status_t err = addCallbackHandle(handle);
if (err != NO_ERROR) {
return err;
}
@@ -111,8 +110,7 @@
return NO_ERROR;
}
-status_t TransactionCallbackInvoker::addCallbackHandle(const sp<CallbackHandle>& handle,
- const std::vector<JankData>& jankData) {
+status_t TransactionCallbackInvoker::addCallbackHandle(const sp<CallbackHandle>& handle) {
// If we can't find the transaction stats something has gone wrong. The client should call
// startRegistration before trying to add a callback handle.
TransactionStats* transactionStats;
@@ -151,8 +149,7 @@
handle->previousReleaseFence,
handle->transformHint,
handle->currentMaxAcquiredBufferCount,
- eventStats, jankData,
- handle->previousReleaseCallbackId);
+ eventStats, handle->previousReleaseCallbackId);
}
return NO_ERROR;
}
diff --git a/services/surfaceflinger/TransactionCallbackInvoker.h b/services/surfaceflinger/TransactionCallbackInvoker.h
index cb7150b..7853a9f 100644
--- a/services/surfaceflinger/TransactionCallbackInvoker.h
+++ b/services/surfaceflinger/TransactionCallbackInvoker.h
@@ -63,8 +63,7 @@
class TransactionCallbackInvoker {
public:
- status_t addCallbackHandles(const std::deque<sp<CallbackHandle>>& handles,
- const std::vector<JankData>& jankData);
+ status_t addCallbackHandles(const std::deque<sp<CallbackHandle>>& handles);
status_t addOnCommitCallbackHandles(const std::deque<sp<CallbackHandle>>& handles,
std::deque<sp<CallbackHandle>>& outRemainingHandles);
@@ -77,9 +76,7 @@
mCompletedTransactions.clear();
}
- status_t addCallbackHandle(const sp<CallbackHandle>& handle,
- const std::vector<JankData>& jankData);
-
+ status_t addCallbackHandle(const sp<CallbackHandle>& handle);
private:
status_t findOrCreateTransactionStats(const sp<IBinder>& listener,
diff --git a/services/surfaceflinger/tests/unittests/Android.bp b/services/surfaceflinger/tests/unittests/Android.bp
index 98d5754..bc35639 100644
--- a/services/surfaceflinger/tests/unittests/Android.bp
+++ b/services/surfaceflinger/tests/unittests/Android.bp
@@ -84,6 +84,7 @@
"FrameTimelineTest.cpp",
"GameModeTest.cpp",
"HWComposerTest.cpp",
+ "JankTrackerTest.cpp",
"OneShotTimerTest.cpp",
"LayerHistoryTest.cpp",
"LayerHistoryIntegrationTest.cpp",
diff --git a/services/surfaceflinger/tests/unittests/FrameTimelineTest.cpp b/services/surfaceflinger/tests/unittests/FrameTimelineTest.cpp
index dac9265..25437ca 100644
--- a/services/surfaceflinger/tests/unittests/FrameTimelineTest.cpp
+++ b/services/surfaceflinger/tests/unittests/FrameTimelineTest.cpp
@@ -15,6 +15,8 @@
*/
#include <common/test/FlagUtils.h>
+#include "BackgroundExecutor.h"
+#include "Jank/JankTracker.h"
#include "com_android_graphics_surfaceflinger_flags.h"
#include "gmock/gmock-spec-builders.h"
#include "mock/MockTimeStats.h"
@@ -90,8 +92,12 @@
mTraceCookieCounter = &mFrameTimeline->mTraceCookieCounter;
maxDisplayFrames = &mFrameTimeline->mMaxDisplayFrames;
maxTokens = mTokenManager->kMaxTokens;
+
+ JankTracker::clearAndStartCollectingAllJankDataForTesting();
}
+ void TearDown() override { JankTracker::clearAndStopCollectingAllJankDataForTesting(); }
+
// Each tracing session can be used for a single block of Start -> Stop.
static std::unique_ptr<perfetto::TracingSession> getTracingSessionForTest() {
perfetto::TraceConfig cfg;
@@ -175,6 +181,16 @@
[&](FrameTimelineDataSource::TraceContext ctx) { ctx.Flush(); });
}
+ std::vector<gui::JankData> getLayerOneJankData() {
+ BackgroundExecutor::getLowPriorityInstance().flushQueue();
+ return JankTracker::getCollectedJankDataForTesting(sLayerIdOne);
+ }
+
+ std::vector<gui::JankData> getLayerTwoJankData() {
+ BackgroundExecutor::getLowPriorityInstance().flushQueue();
+ return JankTracker::getCollectedJankDataForTesting(sLayerIdTwo);
+ }
+
std::shared_ptr<mock::TimeStats> mTimeStats;
std::unique_ptr<impl::FrameTimeline> mFrameTimeline;
impl::TokenManager* mTokenManager;
@@ -339,6 +355,9 @@
EXPECT_NE(surfaceFrame1->getJankSeverityType(), std::nullopt);
EXPECT_NE(surfaceFrame2->getJankType(), std::nullopt);
EXPECT_NE(surfaceFrame2->getJankSeverityType(), std::nullopt);
+
+ EXPECT_EQ(getLayerOneJankData().size(), 1u);
+ EXPECT_EQ(getLayerTwoJankData().size(), 1u);
}
TEST_F(FrameTimelineTest, displayFrameSkippedComposition) {
@@ -446,6 +465,8 @@
// The window should have slided by 1 now and the previous 0th display frame
// should have been removed from the deque
EXPECT_EQ(compareTimelineItems(displayFrame0->getActuals(), TimelineItem(52, 57, 62)), true);
+
+ EXPECT_EQ(getLayerOneJankData().size(), *maxDisplayFrames);
}
TEST_F(FrameTimelineTest, surfaceFrameEndTimeAcquireFenceAfterQueue) {
@@ -575,6 +596,8 @@
presentFence1->signalForTest(70);
mFrameTimeline->setSfPresent(59, presentFence1);
+
+ EXPECT_EQ(getLayerOneJankData().size(), 0u);
}
TEST_F(FrameTimelineTest, presentFenceSignaled_reportsLongSfCpu) {
@@ -603,6 +626,10 @@
presentFence1->signalForTest(70);
mFrameTimeline->setSfPresent(62, presentFence1);
+
+ auto jankData = getLayerOneJankData();
+ EXPECT_EQ(jankData.size(), 1u);
+ EXPECT_EQ(jankData[0].jankType, JankType::SurfaceFlingerCpuDeadlineMissed);
}
TEST_F(FrameTimelineTest, presentFenceSignaled_reportsLongSfGpu) {
@@ -633,6 +660,10 @@
presentFence1->signalForTest(70);
mFrameTimeline->setSfPresent(59, presentFence1, gpuFence1);
+
+ auto jankData = getLayerOneJankData();
+ EXPECT_EQ(jankData.size(), 1u);
+ EXPECT_EQ(jankData[0].jankType, JankType::SurfaceFlingerGpuDeadlineMissed);
}
TEST_F(FrameTimelineTest, presentFenceSignaled_reportsDisplayMiss) {
@@ -661,6 +692,10 @@
mFrameTimeline->setSfPresent(56, presentFence1);
EXPECT_EQ(surfaceFrame1->getJankType(), JankType::DisplayHAL);
EXPECT_EQ(surfaceFrame1->getJankSeverityType(), JankSeverityType::Full);
+
+ auto jankData = getLayerOneJankData();
+ EXPECT_EQ(jankData.size(), 1u);
+ EXPECT_EQ(jankData[0].jankType, JankType::DisplayHAL);
}
TEST_F(FrameTimelineTest, presentFenceSignaled_reportsAppMiss) {
@@ -691,6 +726,10 @@
EXPECT_EQ(surfaceFrame1->getJankType(), JankType::AppDeadlineMissed);
EXPECT_EQ(surfaceFrame1->getJankSeverityType(), JankSeverityType::Partial);
+
+ auto jankData = getLayerOneJankData();
+ EXPECT_EQ(jankData.size(), 1u);
+ EXPECT_EQ(jankData[0].jankType, JankType::AppDeadlineMissed);
}
TEST_F(FrameTimelineTest, presentFenceSignaled_reportsSfScheduling) {
@@ -721,6 +760,10 @@
EXPECT_EQ(surfaceFrame1->getJankType(), JankType::SurfaceFlingerScheduling);
EXPECT_EQ(surfaceFrame1->getJankSeverityType(), JankSeverityType::Full);
+
+ auto jankData = getLayerOneJankData();
+ EXPECT_EQ(jankData.size(), 1u);
+ EXPECT_EQ(jankData[0].jankType, JankType::SurfaceFlingerScheduling);
}
TEST_F(FrameTimelineTest, presentFenceSignaled_reportsSfPredictionError) {
@@ -751,6 +794,10 @@
EXPECT_EQ(surfaceFrame1->getJankType(), JankType::PredictionError);
EXPECT_EQ(surfaceFrame1->getJankSeverityType(), JankSeverityType::Partial);
+
+ auto jankData = getLayerOneJankData();
+ EXPECT_EQ(jankData.size(), 1u);
+ EXPECT_EQ(jankData[0].jankType, JankType::PredictionError);
}
TEST_F(FrameTimelineTest, presentFenceSignaled_reportsAppBufferStuffing) {
@@ -782,6 +829,10 @@
EXPECT_EQ(surfaceFrame1->getJankType(), JankType::BufferStuffing);
EXPECT_EQ(surfaceFrame1->getJankSeverityType(), JankSeverityType::Full);
+
+ auto jankData = getLayerOneJankData();
+ EXPECT_EQ(jankData.size(), 1u);
+ EXPECT_EQ(jankData[0].jankType, JankType::BufferStuffing);
}
TEST_F(FrameTimelineTest, presentFenceSignaled_reportsAppMissWithRenderRate) {
@@ -814,6 +865,10 @@
EXPECT_EQ(surfaceFrame1->getJankType(), JankType::AppDeadlineMissed);
EXPECT_EQ(surfaceFrame1->getJankSeverityType(), JankSeverityType::Full);
+
+ auto jankData = getLayerOneJankData();
+ EXPECT_EQ(jankData.size(), 1u);
+ EXPECT_EQ(jankData[0].jankType, JankType::AppDeadlineMissed);
}
TEST_F(FrameTimelineTest, presentFenceSignaled_displayFramePredictionExpiredPresentsSurfaceFrame) {
@@ -858,6 +913,10 @@
EXPECT_EQ(surfaceFrame1->getActuals().presentTime, 90);
EXPECT_EQ(surfaceFrame1->getJankType(), JankType::Unknown | JankType::AppDeadlineMissed);
EXPECT_EQ(surfaceFrame1->getJankSeverityType(), JankSeverityType::Full);
+
+ auto jankData = getLayerOneJankData();
+ EXPECT_EQ(jankData.size(), 1u);
+ EXPECT_EQ(jankData[0].jankType, JankType::Unknown | JankType::AppDeadlineMissed);
}
/*
@@ -1789,6 +1848,10 @@
EXPECT_EQ(displayFrame->getFrameReadyMetadata(), FrameReadyMetadata::OnTimeFinish);
EXPECT_EQ(displayFrame->getJankType(), JankType::None);
EXPECT_EQ(displayFrame->getJankSeverityType(), JankSeverityType::None);
+
+ auto jankData = getLayerOneJankData();
+ EXPECT_EQ(jankData.size(), 1u);
+ EXPECT_EQ(jankData[0].jankType, JankType::None);
}
TEST_F(FrameTimelineTest, jankClassification_displayFrameOnTimeFinishEarlyPresent) {
diff --git a/services/surfaceflinger/tests/unittests/JankTrackerTest.cpp b/services/surfaceflinger/tests/unittests/JankTrackerTest.cpp
new file mode 100644
index 0000000..2941a14
--- /dev/null
+++ b/services/surfaceflinger/tests/unittests/JankTrackerTest.cpp
@@ -0,0 +1,216 @@
+/*
+ * Copyright 2024 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 <android/gui/BnJankListener.h>
+#include <binder/IInterface.h>
+#include "BackgroundExecutor.h"
+#include "Jank/JankTracker.h"
+
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+
+namespace android {
+
+namespace {
+
+using namespace testing;
+
+class MockJankListener : public gui::BnJankListener {
+public:
+ MockJankListener() = default;
+ ~MockJankListener() override = default;
+
+ MOCK_METHOD(binder::Status, onJankData, (const std::vector<gui::JankData>& jankData),
+ (override));
+};
+
+} // anonymous namespace
+
+class JankTrackerTest : public Test {
+public:
+ JankTrackerTest() {}
+
+ void SetUp() override { mListener = sp<StrictMock<MockJankListener>>::make(); }
+
+ void addJankListener(int32_t layerId) {
+ JankTracker::addJankListener(layerId, IInterface::asBinder(mListener));
+ }
+
+ void removeJankListener(int32_t layerId, int64_t after) {
+ JankTracker::removeJankListener(layerId, IInterface::asBinder(mListener), after);
+ }
+
+ void addJankData(int32_t layerId, int jankType) {
+ gui::JankData data;
+ data.frameVsyncId = mVsyncId++;
+ data.jankType = jankType;
+ data.frameIntervalNs = 8333333;
+ JankTracker::onJankData(layerId, data);
+ }
+
+ void flushBackgroundThread() { BackgroundExecutor::getLowPriorityInstance().flushQueue(); }
+
+ size_t listenerCount() { return JankTracker::sListenerCount; }
+
+ std::vector<gui::JankData> getCollectedJankData(int32_t layerId) {
+ return JankTracker::getCollectedJankDataForTesting(layerId);
+ }
+
+ sp<StrictMock<MockJankListener>> mListener = nullptr;
+ int64_t mVsyncId = 1000;
+};
+
+TEST_F(JankTrackerTest, jankDataIsTrackedAndPropagated) {
+ ASSERT_EQ(listenerCount(), 0u);
+
+ EXPECT_CALL(*mListener.get(), onJankData(SizeIs(3)))
+ .WillOnce([](const std::vector<gui::JankData>& jankData) {
+ EXPECT_EQ(jankData[0].frameVsyncId, 1000);
+ EXPECT_EQ(jankData[0].jankType, 1);
+ EXPECT_EQ(jankData[0].frameIntervalNs, 8333333);
+
+ EXPECT_EQ(jankData[1].frameVsyncId, 1001);
+ EXPECT_EQ(jankData[1].jankType, 2);
+ EXPECT_EQ(jankData[1].frameIntervalNs, 8333333);
+
+ EXPECT_EQ(jankData[2].frameVsyncId, 1002);
+ EXPECT_EQ(jankData[2].jankType, 3);
+ EXPECT_EQ(jankData[2].frameIntervalNs, 8333333);
+ return binder::Status::ok();
+ });
+ EXPECT_CALL(*mListener.get(), onJankData(SizeIs(2)))
+ .WillOnce([](const std::vector<gui::JankData>& jankData) {
+ EXPECT_EQ(jankData[0].frameVsyncId, 1003);
+ EXPECT_EQ(jankData[0].jankType, 4);
+ EXPECT_EQ(jankData[0].frameIntervalNs, 8333333);
+
+ EXPECT_EQ(jankData[1].frameVsyncId, 1004);
+ EXPECT_EQ(jankData[1].jankType, 5);
+ EXPECT_EQ(jankData[1].frameIntervalNs, 8333333);
+
+ return binder::Status::ok();
+ });
+
+ addJankListener(123);
+ addJankData(123, 1);
+ addJankData(123, 2);
+ addJankData(123, 3);
+ JankTracker::flushJankData(123);
+ addJankData(123, 4);
+ removeJankListener(123, mVsyncId);
+ addJankData(123, 5);
+ JankTracker::flushJankData(123);
+ addJankData(123, 6);
+ JankTracker::flushJankData(123);
+ removeJankListener(123, 0);
+
+ flushBackgroundThread();
+}
+
+TEST_F(JankTrackerTest, jankDataIsAutomaticallyFlushedInBatches) {
+ ASSERT_EQ(listenerCount(), 0u);
+
+ // needs to be larger than kJankDataBatchSize in JankTracker.cpp.
+ constexpr size_t kNumberOfJankDataToSend = 234;
+
+ size_t jankDataReceived = 0;
+ size_t numBatchesReceived = 0;
+
+ EXPECT_CALL(*mListener.get(), onJankData(_))
+ .WillRepeatedly([&](const std::vector<gui::JankData>& jankData) {
+ jankDataReceived += jankData.size();
+ numBatchesReceived++;
+ return binder::Status::ok();
+ });
+
+ addJankListener(123);
+ for (size_t i = 0; i < kNumberOfJankDataToSend; i++) {
+ addJankData(123, 0);
+ }
+
+ flushBackgroundThread();
+ // Check that we got some data, without explicitly flushing.
+ EXPECT_GT(jankDataReceived, 0u);
+ EXPECT_GT(numBatchesReceived, 0u);
+ EXPECT_LT(numBatchesReceived, jankDataReceived); // batches should be > size 1.
+
+ removeJankListener(123, 0);
+ JankTracker::flushJankData(123);
+ flushBackgroundThread();
+ EXPECT_EQ(jankDataReceived, kNumberOfJankDataToSend);
+}
+
+TEST_F(JankTrackerTest, jankListenerIsRemovedWhenReturningNullError) {
+ ASSERT_EQ(listenerCount(), 0u);
+
+ EXPECT_CALL(*mListener.get(), onJankData(SizeIs(3)))
+ .WillOnce(Return(binder::Status::fromExceptionCode(binder::Status::EX_NULL_POINTER)));
+
+ addJankListener(123);
+ addJankData(123, 1);
+ addJankData(123, 2);
+ addJankData(123, 3);
+ JankTracker::flushJankData(123);
+ addJankData(123, 4);
+ addJankData(123, 5);
+ JankTracker::flushJankData(123);
+ flushBackgroundThread();
+
+ EXPECT_EQ(listenerCount(), 0u);
+}
+
+TEST_F(JankTrackerTest, jankDataIsDroppedIfNobodyIsListening) {
+ ASSERT_EQ(listenerCount(), 0u);
+
+ addJankData(123, 1);
+ addJankData(123, 2);
+ addJankData(123, 3);
+ flushBackgroundThread();
+
+ EXPECT_EQ(getCollectedJankData(123).size(), 0u);
+}
+
+TEST_F(JankTrackerTest, listenerCountTracksRegistrations) {
+ ASSERT_EQ(listenerCount(), 0u);
+
+ addJankListener(123);
+ addJankListener(456);
+ flushBackgroundThread();
+ EXPECT_EQ(listenerCount(), 2u);
+
+ removeJankListener(123, 0);
+ JankTracker::flushJankData(123);
+ removeJankListener(456, 0);
+ JankTracker::flushJankData(456);
+ flushBackgroundThread();
+ EXPECT_EQ(listenerCount(), 0u);
+}
+
+TEST_F(JankTrackerTest, listenerCountIsAccurateOnDuplicateRegistration) {
+ ASSERT_EQ(listenerCount(), 0u);
+
+ addJankListener(123);
+ addJankListener(123);
+ flushBackgroundThread();
+ EXPECT_EQ(listenerCount(), 1u);
+
+ removeJankListener(123, 0);
+ JankTracker::flushJankData(123);
+ flushBackgroundThread();
+ EXPECT_EQ(listenerCount(), 0u);
+}
+
+} // namespace android
\ No newline at end of file
diff --git a/services/surfaceflinger/tests/unittests/TransactionSurfaceFrameTest.cpp b/services/surfaceflinger/tests/unittests/TransactionSurfaceFrameTest.cpp
index 46733b9..0745f87 100644
--- a/services/surfaceflinger/tests/unittests/TransactionSurfaceFrameTest.cpp
+++ b/services/surfaceflinger/tests/unittests/TransactionSurfaceFrameTest.cpp
@@ -302,58 +302,6 @@
EXPECT_EQ(PresentState::Presented, bufferSurfaceFrameTX->getPresentState());
}
- void PendingSurfaceFramesRemovedAfterClassification() {
- sp<Layer> layer = createLayer();
-
- sp<Fence> fence1(sp<Fence>::make());
- auto acquireFence1 = fenceFactory.createFenceTimeForTest(fence1);
- BufferData bufferData;
- bufferData.acquireFence = fence1;
- bufferData.frameNumber = 1;
- bufferData.flags |= BufferData::BufferDataChange::fenceChanged;
- bufferData.flags |= BufferData::BufferDataChange::frameNumberChanged;
- std::shared_ptr<renderengine::ExternalTexture> externalTexture1 = std::make_shared<
- renderengine::mock::FakeExternalTexture>(1U /*width*/, 1U /*height*/,
- 1ULL /* bufferId */,
- HAL_PIXEL_FORMAT_RGBA_8888,
- 0ULL /*usage*/);
- FrameTimelineInfo ftInfo;
- ftInfo.vsyncId = 1;
- ftInfo.inputEventId = 0;
- layer->setBuffer(externalTexture1, bufferData, 10, 20, false, ftInfo);
- ASSERT_NE(nullptr, layer->mDrawingState.bufferSurfaceFrameTX);
- const auto droppedSurfaceFrame = layer->mDrawingState.bufferSurfaceFrameTX;
-
- sp<Fence> fence2(sp<Fence>::make());
- auto acquireFence2 = fenceFactory.createFenceTimeForTest(fence2);
- bufferData.acquireFence = fence2;
- bufferData.frameNumber = 1;
- bufferData.flags |= BufferData::BufferDataChange::fenceChanged;
- bufferData.flags |= BufferData::BufferDataChange::frameNumberChanged;
- std::shared_ptr<renderengine::ExternalTexture> externalTexture2 = std::make_shared<
- renderengine::mock::FakeExternalTexture>(1U /*width*/, 1U /*height*/,
- 1ULL /* bufferId */,
- HAL_PIXEL_FORMAT_RGBA_8888,
- 0ULL /*usage*/);
- layer->setBuffer(externalTexture2, bufferData, 10, 20, false, ftInfo);
- acquireFence2->signalForTest(12);
-
- ASSERT_NE(nullptr, layer->mDrawingState.bufferSurfaceFrameTX);
- auto presentedSurfaceFrame = layer->mDrawingState.bufferSurfaceFrameTX;
-
- commitTransaction(layer.get());
- layer->updateTexImage(15);
-
- // Both the droppedSurfaceFrame and presentedSurfaceFrame should be in
- // pendingJankClassifications.
- EXPECT_EQ(2u, layer->mPendingJankClassifications.size());
- presentedSurfaceFrame->onPresent(20, JankType::None, 90_Hz, 90_Hz,
- /*displayDeadlineDelta*/ 0, /*displayPresentDelta*/ 0);
- layer->releasePendingBuffer(25);
-
- EXPECT_EQ(0u, layer->mPendingJankClassifications.size());
- }
-
void BufferSurfaceFrame_ReplaceValidTokenBufferWithInvalidTokenBuffer() {
sp<Layer> layer = createLayer();
@@ -445,8 +393,7 @@
void MultipleCommitsBeforeLatch() {
sp<Layer> layer = createLayer();
- uint32_t surfaceFramesPendingClassification = 0;
- std::vector<std::shared_ptr<frametimeline::SurfaceFrame>> bufferlessSurfaceFrames;
+ std::vector<std::shared_ptr<frametimeline::SurfaceFrame>> surfaceFrames;
for (int i = 0; i < 10; i += 2) {
sp<Fence> fence(sp<Fence>::make());
BufferData bufferData;
@@ -469,51 +416,43 @@
layer->setFrameTimelineVsyncForBufferlessTransaction(ftInfo2, 10);
ASSERT_NE(nullptr, layer->mDrawingState.bufferSurfaceFrameTX);
EXPECT_EQ(1u, layer->mDrawingState.bufferlessSurfaceFramesTX.size());
- auto& bufferlessSurfaceFrame =
- layer->mDrawingState.bufferlessSurfaceFramesTX.at(/*vsyncId*/ 2);
- bufferlessSurfaceFrames.push_back(bufferlessSurfaceFrame);
+
+ surfaceFrames.push_back(layer->mDrawingState.bufferSurfaceFrameTX);
+ surfaceFrames.push_back(
+ layer->mDrawingState.bufferlessSurfaceFramesTX.at(/*vsyncId*/ 2));
commitTransaction(layer.get());
- surfaceFramesPendingClassification += 2;
- EXPECT_EQ(surfaceFramesPendingClassification,
- layer->mPendingJankClassifications.size());
}
auto presentedBufferSurfaceFrame = layer->mDrawingState.bufferSurfaceFrameTX;
layer->updateTexImage(15);
// BufferlessSurfaceFrames are immediately set to presented and added to the DisplayFrame.
// Since we don't have access to DisplayFrame here, trigger an onPresent directly.
- for (auto& surfaceFrame : bufferlessSurfaceFrames) {
- surfaceFrame->onPresent(20, JankType::None, 90_Hz, 90_Hz,
- /*displayDeadlineDelta*/ 0, /*displayPresentDelta*/ 0);
+ // The odd indices are the bufferless frames.
+ for (uint32_t i = 1; i < 10; i += 2) {
+ surfaceFrames[i]->onPresent(20, JankType::None, 90_Hz, 90_Hz,
+ /*displayDeadlineDelta*/ 0, /*displayPresentDelta*/ 0);
}
presentedBufferSurfaceFrame->onPresent(20, JankType::None, 90_Hz, 90_Hz,
/*displayDeadlineDelta*/ 0,
/*displayPresentDelta*/ 0);
- // There should be 10 bufferlessSurfaceFrames and 1 bufferSurfaceFrame
- ASSERT_EQ(10u, surfaceFramesPendingClassification);
- ASSERT_EQ(surfaceFramesPendingClassification, layer->mPendingJankClassifications.size());
-
// For the frames upto 8, the bufferSurfaceFrame should have been dropped while the
// bufferlessSurfaceFrame presented
for (uint32_t i = 0; i < 8; i += 2) {
- auto& bufferSurfaceFrame = layer->mPendingJankClassifications[i];
- auto& bufferlessSurfaceFrame = layer->mPendingJankClassifications[i + 1];
+ auto bufferSurfaceFrame = surfaceFrames[i];
+ auto bufferlessSurfaceFrame = surfaceFrames[i + 1];
EXPECT_EQ(bufferSurfaceFrame->getPresentState(), PresentState::Dropped);
EXPECT_EQ(bufferlessSurfaceFrame->getPresentState(), PresentState::Presented);
}
{
- auto& bufferSurfaceFrame = layer->mPendingJankClassifications[8u];
- auto& bufferlessSurfaceFrame = layer->mPendingJankClassifications[9u];
+ auto bufferSurfaceFrame = surfaceFrames[8];
+ auto bufferlessSurfaceFrame = surfaceFrames[9];
EXPECT_EQ(bufferSurfaceFrame->getPresentState(), PresentState::Presented);
EXPECT_EQ(bufferlessSurfaceFrame->getPresentState(), PresentState::Presented);
}
layer->releasePendingBuffer(25);
-
- // There shouldn't be any pending classifications. Everything should have been cleared.
- EXPECT_EQ(0u, layer->mPendingJankClassifications.size());
}
};
@@ -541,10 +480,6 @@
MultipleSurfaceFramesPresentedTogether();
}
-TEST_F(TransactionSurfaceFrameTest, PendingSurfaceFramesRemovedAfterClassification) {
- PendingSurfaceFramesRemovedAfterClassification();
-}
-
TEST_F(TransactionSurfaceFrameTest,
BufferSurfaceFrame_ReplaceValidTokenBufferWithInvalidTokenBuffer) {
BufferSurfaceFrame_ReplaceValidTokenBufferWithInvalidTokenBuffer();