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/libs/gui/ITransactionCompletedListener.cpp b/libs/gui/ITransactionCompletedListener.cpp
index f5d19aa..83fc827 100644
--- a/libs/gui/ITransactionCompletedListener.cpp
+++ b/libs/gui/ITransactionCompletedListener.cpp
@@ -43,12 +43,6 @@
} // Anonymous namespace
-namespace { // Anonymous
-
-constexpr int32_t kSerializedCallbackTypeOnCompelteWithJankData = 2;
-
-} // Anonymous namespace
-
status_t FrameEventHistoryStats::writeToParcel(Parcel* output) const {
status_t err = output->writeUint64(frameNumber);
if (err != NO_ERROR) return err;
@@ -119,23 +113,6 @@
return err;
}
-JankData::JankData()
- : frameVsyncId(FrameTimelineInfo::INVALID_VSYNC_ID), jankType(JankType::None) {}
-
-status_t JankData::writeToParcel(Parcel* output) const {
- SAFE_PARCEL(output->writeInt64, frameVsyncId);
- SAFE_PARCEL(output->writeInt32, jankType);
- SAFE_PARCEL(output->writeInt64, frameIntervalNs);
- return NO_ERROR;
-}
-
-status_t JankData::readFromParcel(const Parcel* input) {
- SAFE_PARCEL(input->readInt64, &frameVsyncId);
- SAFE_PARCEL(input->readInt32, &jankType);
- SAFE_PARCEL(input->readInt64, &frameIntervalNs);
- return NO_ERROR;
-}
-
status_t SurfaceStats::writeToParcel(Parcel* output) const {
SAFE_PARCEL(output->writeStrongBinder, surfaceControl);
if (const auto* acquireFence = std::get_if<sp<Fence>>(&acquireTimeOrFence)) {
@@ -160,10 +137,6 @@
SAFE_PARCEL(output->writeUint32, currentMaxAcquiredBufferCount);
SAFE_PARCEL(output->writeParcelable, eventStats);
- SAFE_PARCEL(output->writeInt32, static_cast<int32_t>(jankData.size()));
- for (const auto& data : jankData) {
- SAFE_PARCEL(output->writeParcelable, data);
- }
SAFE_PARCEL(output->writeParcelable, previousReleaseCallbackId);
return NO_ERROR;
}
@@ -200,13 +173,6 @@
SAFE_PARCEL(input->readUint32, ¤tMaxAcquiredBufferCount);
SAFE_PARCEL(input->readParcelable, &eventStats);
- int32_t jankData_size = 0;
- SAFE_PARCEL_READ_SIZE(input->readInt32, &jankData_size, input->dataSize());
- for (int i = 0; i < jankData_size; i++) {
- JankData data;
- SAFE_PARCEL(input->readParcelable, &data);
- jankData.push_back(data);
- }
SAFE_PARCEL(input->readParcelable, &previousReleaseCallbackId);
return NO_ERROR;
}
@@ -371,11 +337,7 @@
status_t CallbackId::writeToParcel(Parcel* output) const {
SAFE_PARCEL(output->writeInt64, id);
- if (type == Type::ON_COMPLETE && includeJankData) {
- SAFE_PARCEL(output->writeInt32, kSerializedCallbackTypeOnCompelteWithJankData);
- } else {
- SAFE_PARCEL(output->writeInt32, static_cast<int32_t>(type));
- }
+ SAFE_PARCEL(output->writeInt32, static_cast<int32_t>(type));
return NO_ERROR;
}
@@ -383,13 +345,7 @@
SAFE_PARCEL(input->readInt64, &id);
int32_t typeAsInt;
SAFE_PARCEL(input->readInt32, &typeAsInt);
- if (typeAsInt == kSerializedCallbackTypeOnCompelteWithJankData) {
- type = Type::ON_COMPLETE;
- includeJankData = true;
- } else {
- type = static_cast<CallbackId::Type>(typeAsInt);
- includeJankData = false;
- }
+ type = static_cast<CallbackId::Type>(typeAsInt);
return NO_ERROR;
}
diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp
index af91bb3..e2ac1d5 100644
--- a/libs/gui/SurfaceComposerClient.cpp
+++ b/libs/gui/SurfaceComposerClient.cpp
@@ -86,7 +86,8 @@
return (((int64_t)getpid()) << 32) | ++idCounter;
}
-void emptyCallback(nsecs_t, const sp<Fence>&, const std::vector<SurfaceControlStats>&) {}
+constexpr int64_t INVALID_VSYNC = -1;
+
} // namespace
const std::string SurfaceComposerClient::kEmpty{};
@@ -207,9 +208,168 @@
return DefaultComposerClient::getComposerClient();
}
+// ---------------------------------------------------------------------------
+
JankDataListener::~JankDataListener() {
}
+status_t JankDataListener::flushJankData() {
+ if (mLayerId == -1) {
+ return INVALID_OPERATION;
+ }
+
+ binder::Status status = ComposerServiceAIDL::getComposerService()->flushJankData(mLayerId);
+ return statusTFromBinderStatus(status);
+}
+
+std::mutex JankDataListenerFanOut::sFanoutInstanceMutex;
+std::unordered_map<int32_t, sp<JankDataListenerFanOut>> JankDataListenerFanOut::sFanoutInstances;
+
+binder::Status JankDataListenerFanOut::onJankData(const std::vector<gui::JankData>& jankData) {
+ // Find the highest VSync ID.
+ int64_t lastVsync = jankData.empty()
+ ? 0
+ : std::max_element(jankData.begin(), jankData.end(),
+ [](const gui::JankData& jd1, const gui::JankData& jd2) {
+ return jd1.frameVsyncId < jd2.frameVsyncId;
+ })
+ ->frameVsyncId;
+
+ // Fan out the jank data callback.
+ std::vector<wp<JankDataListener>> listenersToRemove;
+ for (auto listener : getActiveListeners()) {
+ if (!listener->onJankDataAvailable(jankData) ||
+ (listener->mRemoveAfter >= 0 && listener->mRemoveAfter <= lastVsync)) {
+ listenersToRemove.push_back(listener);
+ }
+ }
+
+ return removeListeners(listenersToRemove)
+ ? binder::Status::ok()
+ : binder::Status::fromExceptionCode(binder::Status::EX_NULL_POINTER);
+}
+
+status_t JankDataListenerFanOut::addListener(sp<SurfaceControl> sc, sp<JankDataListener> listener) {
+ sp<IBinder> layer = sc->getHandle();
+ if (layer == nullptr) {
+ return UNEXPECTED_NULL;
+ }
+ int32_t layerId = sc->getLayerId();
+
+ sFanoutInstanceMutex.lock();
+ auto it = sFanoutInstances.find(layerId);
+ bool registerNeeded = it == sFanoutInstances.end();
+ sp<JankDataListenerFanOut> fanout;
+ if (registerNeeded) {
+ fanout = sp<JankDataListenerFanOut>::make(layerId);
+ sFanoutInstances.insert({layerId, fanout});
+ } else {
+ fanout = it->second;
+ }
+
+ fanout->mMutex.lock();
+ fanout->mListeners.insert(listener);
+ fanout->mMutex.unlock();
+
+ sFanoutInstanceMutex.unlock();
+
+ if (registerNeeded) {
+ binder::Status status =
+ ComposerServiceAIDL::getComposerService()->addJankListener(layer, fanout);
+ return statusTFromBinderStatus(status);
+ }
+ return OK;
+}
+
+status_t JankDataListenerFanOut::removeListener(sp<JankDataListener> listener) {
+ int32_t layerId = listener->mLayerId;
+ if (layerId == -1) {
+ return INVALID_OPERATION;
+ }
+
+ int64_t removeAfter = INVALID_VSYNC;
+ sp<JankDataListenerFanOut> fanout;
+
+ sFanoutInstanceMutex.lock();
+ auto it = sFanoutInstances.find(layerId);
+ if (it != sFanoutInstances.end()) {
+ fanout = it->second;
+ removeAfter = fanout->updateAndGetRemovalVSync();
+ }
+
+ if (removeAfter != INVALID_VSYNC) {
+ // Remove this instance from the map, so that no new listeners are added
+ // while we're scheduled to be removed.
+ sFanoutInstances.erase(layerId);
+ }
+ sFanoutInstanceMutex.unlock();
+
+ if (removeAfter < 0) {
+ return OK;
+ }
+
+ binder::Status status =
+ ComposerServiceAIDL::getComposerService()->removeJankListener(layerId, fanout,
+ removeAfter);
+ return statusTFromBinderStatus(status);
+}
+
+std::vector<sp<JankDataListener>> JankDataListenerFanOut::getActiveListeners() {
+ std::scoped_lock<std::mutex> lock(mMutex);
+
+ std::vector<sp<JankDataListener>> listeners;
+ for (auto it = mListeners.begin(); it != mListeners.end();) {
+ auto listener = it->promote();
+ if (!listener) {
+ it = mListeners.erase(it);
+ } else {
+ listeners.push_back(std::move(listener));
+ it++;
+ }
+ }
+ return listeners;
+}
+
+bool JankDataListenerFanOut::removeListeners(const std::vector<wp<JankDataListener>>& listeners) {
+ std::scoped_lock<std::mutex> fanoutLock(sFanoutInstanceMutex);
+ std::scoped_lock<std::mutex> listenersLock(mMutex);
+
+ for (auto listener : listeners) {
+ mListeners.erase(listener);
+ }
+
+ if (mListeners.empty()) {
+ sFanoutInstances.erase(mLayerId);
+ return false;
+ }
+ return true;
+}
+
+int64_t JankDataListenerFanOut::updateAndGetRemovalVSync() {
+ std::scoped_lock<std::mutex> lock(mMutex);
+ if (mRemoveAfter >= 0) {
+ // We've already been scheduled to be removed. Don't schedule again.
+ return INVALID_VSYNC;
+ }
+
+ int64_t removeAfter = 0;
+ for (auto it = mListeners.begin(); it != mListeners.end();) {
+ auto listener = it->promote();
+ if (!listener) {
+ it = mListeners.erase(it);
+ } else if (listener->mRemoveAfter < 0) {
+ // We have at least one listener that's still interested. Don't remove.
+ return INVALID_VSYNC;
+ } else {
+ removeAfter = std::max(removeAfter, listener->mRemoveAfter);
+ it++;
+ }
+ }
+
+ mRemoveAfter = removeAfter;
+ return removeAfter;
+}
+
// ---------------------------------------------------------------------------
// TransactionCompletedListener does not use ANDROID_SINGLETON_STATIC_INSTANCE because it needs
@@ -256,14 +416,6 @@
surfaceControls,
CallbackId::Type callbackType) {
std::lock_guard<std::mutex> lock(mMutex);
- return addCallbackFunctionLocked(callbackFunction, surfaceControls, callbackType);
-}
-
-CallbackId TransactionCompletedListener::addCallbackFunctionLocked(
- const TransactionCompletedCallback& callbackFunction,
- const std::unordered_set<sp<SurfaceControl>, SurfaceComposerClient::SCHash>&
- surfaceControls,
- CallbackId::Type callbackType) {
startListeningLocked();
CallbackId callbackId(getNextIdLocked(), callbackType);
@@ -272,33 +424,11 @@
for (const auto& surfaceControl : surfaceControls) {
callbackSurfaceControls[surfaceControl->getHandle()] = surfaceControl;
-
- if (callbackType == CallbackId::Type::ON_COMPLETE &&
- mJankListeners.count(surfaceControl->getLayerId()) != 0) {
- callbackId.includeJankData = true;
- }
}
return callbackId;
}
-void TransactionCompletedListener::addJankListener(const sp<JankDataListener>& listener,
- sp<SurfaceControl> surfaceControl) {
- std::lock_guard<std::mutex> lock(mMutex);
- mJankListeners.insert({surfaceControl->getLayerId(), listener});
-}
-
-void TransactionCompletedListener::removeJankListener(const sp<JankDataListener>& listener) {
- std::lock_guard<std::mutex> lock(mMutex);
- for (auto it = mJankListeners.begin(); it != mJankListeners.end();) {
- if (it->second == listener) {
- it = mJankListeners.erase(it);
- } else {
- it++;
- }
- }
-}
-
void TransactionCompletedListener::setReleaseBufferCallback(const ReleaseCallbackId& callbackId,
ReleaseBufferCallback listener) {
std::scoped_lock<std::mutex> lock(mMutex);
@@ -325,32 +455,20 @@
}
void TransactionCompletedListener::addSurfaceControlToCallbacks(
- SurfaceComposerClient::CallbackInfo& callbackInfo,
- const sp<SurfaceControl>& surfaceControl) {
+ const sp<SurfaceControl>& surfaceControl,
+ const std::unordered_set<CallbackId, CallbackIdHash>& callbackIds) {
std::lock_guard<std::mutex> lock(mMutex);
- bool includingJankData = false;
- for (auto callbackId : callbackInfo.callbackIds) {
+ for (auto callbackId : callbackIds) {
mCallbacks[callbackId].surfaceControls.emplace(std::piecewise_construct,
std::forward_as_tuple(
surfaceControl->getHandle()),
std::forward_as_tuple(surfaceControl));
- includingJankData = includingJankData || callbackId.includeJankData;
- }
-
- // If no registered callback is requesting jank data, but there is a jank listener registered
- // on the new surface control, add a synthetic callback that requests the jank data.
- if (!includingJankData && mJankListeners.count(surfaceControl->getLayerId()) != 0) {
- CallbackId callbackId =
- addCallbackFunctionLocked(&emptyCallback, callbackInfo.surfaceControls,
- CallbackId::Type::ON_COMPLETE);
- callbackInfo.callbackIds.emplace(callbackId);
}
}
void TransactionCompletedListener::onTransactionCompleted(ListenerStats listenerStats) {
std::unordered_map<CallbackId, CallbackTranslation, CallbackIdHash> callbacksMap;
- std::multimap<int32_t, sp<JankDataListener>> jankListenersMap;
{
std::lock_guard<std::mutex> lock(mMutex);
@@ -366,7 +484,6 @@
* sp<SurfaceControl> that could possibly exist for the callbacks.
*/
callbacksMap = mCallbacks;
- jankListenersMap = mJankListeners;
for (const auto& transactionStats : listenerStats.transactionStats) {
for (auto& callbackId : transactionStats.callbackIds) {
mCallbacks.erase(callbackId);
@@ -486,12 +603,6 @@
transactionStats.presentFence, surfaceStats);
}
}
-
- if (surfaceStats.jankData.empty()) continue;
- auto jankRange = jankListenersMap.equal_range(layerId);
- for (auto it = jankRange.first; it != jankRange.second; it++) {
- it->second->onJankDataAvailable(surfaceStats.jankData);
- }
}
}
}
@@ -1004,8 +1115,9 @@
// register all surface controls for all callbackIds for this listener that is merging
for (const auto& surfaceControl : currentProcessCallbackInfo.surfaceControls) {
- mTransactionCompletedListener->addSurfaceControlToCallbacks(currentProcessCallbackInfo,
- surfaceControl);
+ mTransactionCompletedListener
+ ->addSurfaceControlToCallbacks(surfaceControl,
+ currentProcessCallbackInfo.callbackIds);
}
}
@@ -1361,7 +1473,7 @@
auto& callbackInfo = mListenerCallbacks[TransactionCompletedListener::getIInstance()];
callbackInfo.surfaceControls.insert(sc);
- mTransactionCompletedListener->addSurfaceControlToCallbacks(callbackInfo, sc);
+ mTransactionCompletedListener->addSurfaceControlToCallbacks(sc, callbackInfo.callbackIds);
}
SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setPosition(
diff --git a/libs/gui/aidl/android/gui/IJankListener.aidl b/libs/gui/aidl/android/gui/IJankListener.aidl
new file mode 100644
index 0000000..2bfd1af
--- /dev/null
+++ b/libs/gui/aidl/android/gui/IJankListener.aidl
@@ -0,0 +1,29 @@
+/*
+ * 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.
+ */
+
+package android.gui;
+
+import android.gui.JankData;
+
+/** @hide */
+interface IJankListener {
+
+ /**
+ * Callback reporting jank data of the most recent frames.
+ * @See {@link ISurfaceComposer#addJankListener(IBinder, IJankListener)}
+ */
+ void onJankData(in JankData[] data);
+}
diff --git a/libs/gui/aidl/android/gui/ISurfaceComposer.aidl b/libs/gui/aidl/android/gui/ISurfaceComposer.aidl
index 6d018ea..ac14138 100644
--- a/libs/gui/aidl/android/gui/ISurfaceComposer.aidl
+++ b/libs/gui/aidl/android/gui/ISurfaceComposer.aidl
@@ -42,6 +42,7 @@
import android.gui.ITunnelModeEnabledListener;
import android.gui.IWindowInfosListener;
import android.gui.IWindowInfosPublisher;
+import android.gui.IJankListener;
import android.gui.LayerCaptureArgs;
import android.gui.OverlayProperties;
import android.gui.PullAtomData;
@@ -580,4 +581,22 @@
* This method should not block the ShutdownThread therefore it's handled asynchronously.
*/
oneway void notifyShutdown();
+
+ /**
+ * Registers the jank listener on the given layer to receive jank data of future frames.
+ */
+ void addJankListener(IBinder layer, IJankListener listener);
+
+ /**
+ * Flushes any pending jank data on the given layer to any registered listeners on that layer.
+ */
+ oneway void flushJankData(int layerId);
+
+ /**
+ * Schedules the removal of the jank listener from the given layer after the VSync with the
+ * specified ID. Use a value <= 0 for afterVsync to remove the listener immediately. The given
+ * listener will not be removed before the given VSync, but may still receive data for frames
+ * past the provided VSync.
+ */
+ oneway void removeJankListener(int layerId, IJankListener listener, long afterVsync);
}
diff --git a/libs/gui/aidl/android/gui/JankData.aidl b/libs/gui/aidl/android/gui/JankData.aidl
new file mode 100644
index 0000000..7ea9d22
--- /dev/null
+++ b/libs/gui/aidl/android/gui/JankData.aidl
@@ -0,0 +1,35 @@
+/*
+ * 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.
+ */
+
+ package android.gui;
+
+ /** @hide */
+parcelable JankData {
+ /**
+ * Identifier for the frame submitted with Transaction.setFrameTimelineVsyncId
+ */
+ long frameVsyncId;
+
+ /**
+ * Bitmask of jank types that occurred.
+ */
+ int jankType;
+
+ /**
+ * Expected duration in nanoseconds of this frame.
+ */
+ long frameIntervalNs;
+}
diff --git a/libs/gui/include/gui/ITransactionCompletedListener.h b/libs/gui/include/gui/ITransactionCompletedListener.h
index bc97cd0..014029b 100644
--- a/libs/gui/include/gui/ITransactionCompletedListener.h
+++ b/libs/gui/include/gui/ITransactionCompletedListener.h
@@ -16,8 +16,6 @@
#pragma once
-#include "JankInfo.h"
-
#include <binder/IInterface.h>
#include <binder/Parcel.h>
#include <binder/Parcelable.h>
@@ -40,15 +38,10 @@
class CallbackId : public Parcelable {
public:
int64_t id;
- enum class Type : int32_t {
- ON_COMPLETE = 0,
- ON_COMMIT = 1,
- /*reserved for serialization = 2*/
- } type;
- bool includeJankData; // Only respected for ON_COMPLETE callbacks.
+ enum class Type : int32_t { ON_COMPLETE = 0, ON_COMMIT = 1 } type;
CallbackId() {}
- CallbackId(int64_t id, Type type) : id(id), type(type), includeJankData(false) {}
+ CallbackId(int64_t id, Type type) : id(id), type(type) {}
status_t writeToParcel(Parcel* output) const override;
status_t readFromParcel(const Parcel* input) override;
@@ -113,29 +106,6 @@
nsecs_t dequeueReadyTime;
};
-/**
- * Jank information representing SurfaceFlinger's jank classification about frames for a specific
- * surface.
- */
-class JankData : public Parcelable {
-public:
- status_t writeToParcel(Parcel* output) const override;
- status_t readFromParcel(const Parcel* input) override;
-
- JankData();
- JankData(int64_t frameVsyncId, int32_t jankType, nsecs_t frameIntervalNs)
- : frameVsyncId(frameVsyncId), jankType(jankType), frameIntervalNs(frameIntervalNs) {}
-
- // Identifier for the frame submitted with Transaction.setFrameTimelineVsyncId
- int64_t frameVsyncId;
-
- // Bitmask of janks that occurred
- int32_t jankType;
-
- // Expected duration of the frame
- nsecs_t frameIntervalNs;
-};
-
class SurfaceStats : public Parcelable {
public:
status_t writeToParcel(Parcel* output) const override;
@@ -145,14 +115,13 @@
SurfaceStats(const sp<IBinder>& sc, std::variant<nsecs_t, sp<Fence>> acquireTimeOrFence,
const sp<Fence>& prevReleaseFence, std::optional<uint32_t> hint,
uint32_t currentMaxAcquiredBuffersCount, FrameEventHistoryStats frameEventStats,
- std::vector<JankData> jankData, ReleaseCallbackId previousReleaseCallbackId)
+ ReleaseCallbackId previousReleaseCallbackId)
: surfaceControl(sc),
acquireTimeOrFence(std::move(acquireTimeOrFence)),
previousReleaseFence(prevReleaseFence),
transformHint(hint),
currentMaxAcquiredBufferCount(currentMaxAcquiredBuffersCount),
eventStats(frameEventStats),
- jankData(std::move(jankData)),
previousReleaseCallbackId(previousReleaseCallbackId) {}
sp<IBinder> surfaceControl;
@@ -161,7 +130,6 @@
std::optional<uint32_t> transformHint = 0;
uint32_t currentMaxAcquiredBufferCount = 0;
FrameEventHistoryStats eventStats;
- std::vector<JankData> jankData;
ReleaseCallbackId previousReleaseCallbackId;
};
diff --git a/libs/gui/include/gui/SurfaceComposerClient.h b/libs/gui/include/gui/SurfaceComposerClient.h
index 0862e03..88c0c53 100644
--- a/libs/gui/include/gui/SurfaceComposerClient.h
+++ b/libs/gui/include/gui/SurfaceComposerClient.h
@@ -41,6 +41,7 @@
#include <ui/Rotation.h>
#include <ui/StaticDisplayInfo.h>
+#include <android/gui/BnJankListener.h>
#include <android/gui/ISurfaceComposerClient.h>
#include <gui/CpuConsumer.h>
@@ -864,12 +865,82 @@
// ---------------------------------------------------------------------------
-class JankDataListener : public VirtualLightRefBase {
+class JankDataListener;
+
+// Acts as a representative listener to the composer for a single layer and
+// forwards any received jank data to multiple listeners. Will remove itself
+// from the composer only once the last listener is removed.
+class JankDataListenerFanOut : public gui::BnJankListener {
public:
- virtual ~JankDataListener() = 0;
- virtual void onJankDataAvailable(const std::vector<JankData>& jankData) = 0;
+ JankDataListenerFanOut(int32_t layerId) : mLayerId(layerId) {}
+
+ binder::Status onJankData(const std::vector<gui::JankData>& jankData) override;
+
+ static status_t addListener(sp<SurfaceControl> sc, sp<JankDataListener> listener);
+ static status_t removeListener(sp<JankDataListener> listener);
+
+private:
+ std::vector<sp<JankDataListener>> getActiveListeners();
+ bool removeListeners(const std::vector<wp<JankDataListener>>& listeners);
+ int64_t updateAndGetRemovalVSync();
+
+ struct WpJDLHash {
+ std::size_t operator()(const wp<JankDataListener>& listener) const {
+ return std::hash<JankDataListener*>{}(listener.unsafe_get());
+ }
+ };
+
+ std::mutex mMutex;
+ std::unordered_set<wp<JankDataListener>, WpJDLHash> mListeners GUARDED_BY(mMutex);
+ int32_t mLayerId;
+ int64_t mRemoveAfter = -1;
+
+ static std::mutex sFanoutInstanceMutex;
+ static std::unordered_map<int32_t, sp<JankDataListenerFanOut>> sFanoutInstances;
};
+// Base class for client listeners interested in jank classification data from
+// the composer. Subclasses should override onJankDataAvailable and call the add
+// and removal methods to receive jank data.
+class JankDataListener : public virtual RefBase {
+public:
+ JankDataListener() {}
+ virtual ~JankDataListener();
+
+ virtual bool onJankDataAvailable(const std::vector<gui::JankData>& jankData) = 0;
+
+ status_t addListener(sp<SurfaceControl> sc) {
+ if (mLayerId != -1) {
+ removeListener(0);
+ mLayerId = -1;
+ }
+
+ int32_t layerId = sc->getLayerId();
+ status_t status =
+ JankDataListenerFanOut::addListener(std::move(sc),
+ sp<JankDataListener>::fromExisting(this));
+ if (status == OK) {
+ mLayerId = layerId;
+ }
+ return status;
+ }
+
+ status_t removeListener(int64_t afterVsync) {
+ mRemoveAfter = std::max(static_cast<int64_t>(0), afterVsync);
+ return JankDataListenerFanOut::removeListener(sp<JankDataListener>::fromExisting(this));
+ }
+
+ status_t flushJankData();
+
+ friend class JankDataListenerFanOut;
+
+private:
+ int32_t mLayerId = -1;
+ int64_t mRemoveAfter = -1;
+};
+
+// ---------------------------------------------------------------------------
+
class TransactionCompletedListener : public BnTransactionCompletedListener {
public:
TransactionCompletedListener();
@@ -904,7 +975,6 @@
std::unordered_map<CallbackId, CallbackTranslation, CallbackIdHash> mCallbacks
GUARDED_BY(mMutex);
- std::multimap<int32_t, sp<JankDataListener>> mJankListeners GUARDED_BY(mMutex);
std::unordered_map<ReleaseCallbackId, ReleaseBufferCallback, ReleaseBufferCallbackIdHash>
mReleaseBufferCallbacks GUARDED_BY(mMutex);
@@ -927,14 +997,10 @@
const std::unordered_set<sp<SurfaceControl>, SurfaceComposerClient::SCHash>&
surfaceControls,
CallbackId::Type callbackType);
- CallbackId addCallbackFunctionLocked(
- const TransactionCompletedCallback& callbackFunction,
- const std::unordered_set<sp<SurfaceControl>, SurfaceComposerClient::SCHash>&
- surfaceControls,
- CallbackId::Type callbackType) REQUIRES(mMutex);
- void addSurfaceControlToCallbacks(SurfaceComposerClient::CallbackInfo& callbackInfo,
- const sp<SurfaceControl>& surfaceControl);
+ void addSurfaceControlToCallbacks(
+ const sp<SurfaceControl>& surfaceControl,
+ const std::unordered_set<CallbackId, CallbackIdHash>& callbackIds);
void addQueueStallListener(std::function<void(const std::string&)> stallListener, void* id);
void removeQueueStallListener(void *id);
@@ -943,18 +1009,6 @@
TrustedPresentationCallback tpc, int id, void* context);
void clearTrustedPresentationCallback(int id);
- /*
- * Adds a jank listener to be informed about SurfaceFlinger's jank classification for a specific
- * surface. Jank classifications arrive as part of the transaction callbacks about previous
- * frames submitted to this Surface.
- */
- void addJankListener(const sp<JankDataListener>& listener, sp<SurfaceControl> surfaceControl);
-
- /**
- * Removes a jank listener previously added to addJankCallback.
- */
- void removeJankListener(const sp<JankDataListener>& listener);
-
void addSurfaceStatsListener(void* context, void* cookie, sp<SurfaceControl> surfaceControl,
SurfaceStatsCallback listener);
void removeSurfaceStatsListener(void* context, void* cookie);
diff --git a/libs/gui/tests/Surface_test.cpp b/libs/gui/tests/Surface_test.cpp
index 43cd0f8..ace4423 100644
--- a/libs/gui/tests/Surface_test.cpp
+++ b/libs/gui/tests/Surface_test.cpp
@@ -987,6 +987,19 @@
binder::Status notifyShutdown() override { return binder::Status::ok(); }
+ binder::Status addJankListener(const sp<IBinder>& /*layer*/,
+ const sp<gui::IJankListener>& /*listener*/) override {
+ return binder::Status::ok();
+ }
+
+ binder::Status flushJankData(int32_t /*layerId*/) override { return binder::Status::ok(); }
+
+ binder::Status removeJankListener(int32_t /*layerId*/,
+ const sp<gui::IJankListener>& /*listener*/,
+ int64_t /*afterVsync*/) override {
+ return binder::Status::ok();
+ }
+
protected:
IBinder* onAsBinder() override { return nullptr; }
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();