diff options
22 files changed, 999 insertions, 334 deletions
diff --git a/libs/gui/ITransactionCompletedListener.cpp b/libs/gui/ITransactionCompletedListener.cpp index f5d19aac78..83fc827c5f 100644 --- a/libs/gui/ITransactionCompletedListener.cpp +++ b/libs/gui/ITransactionCompletedListener.cpp @@ -43,12 +43,6 @@ enum class Tag : uint32_t { } // 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 @@ status_t FrameEventHistoryStats::readFromParcel(const Parcel* input) { 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 @@ status_t SurfaceStats::writeToParcel(Parcel* output) const { 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 @@ status_t SurfaceStats::readFromParcel(const Parcel* input) { 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 @@ ListenerCallbacks ListenerCallbacks::filter(CallbackId::Type type) const { 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 @@ status_t CallbackId::readFromParcel(const Parcel* input) { 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 af91bb3ae2..e2ac1d5de5 100644 --- a/libs/gui/SurfaceComposerClient.cpp +++ b/libs/gui/SurfaceComposerClient.cpp @@ -86,7 +86,8 @@ int64_t generateId() { 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 @@ sp<SurfaceComposerClient> SurfaceComposerClient::getDefault() { 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 @@ CallbackId TransactionCompletedListener::addCallbackFunction( 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 @@ CallbackId TransactionCompletedListener::addCallbackFunctionLocked( 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::removeSurfaceStatsListener(void* context, voi } 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 @@ void TransactionCompletedListener::onTransactionCompleted(ListenerStats listener * 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 @@ void TransactionCompletedListener::onTransactionCompleted(ListenerStats listener 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 @@ SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::merge(Tr // 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 @@ void SurfaceComposerClient::Transaction::registerSurfaceControlForCallback( 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 0000000000..2bfd1af69d --- /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 6d018ea7ef..ac14138e8c 100644 --- a/libs/gui/aidl/android/gui/ISurfaceComposer.aidl +++ b/libs/gui/aidl/android/gui/ISurfaceComposer.aidl @@ -42,6 +42,7 @@ import android.gui.ISurfaceComposerClient; 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 @@ interface ISurfaceComposer { * 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 0000000000..7ea9d22ecf --- /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 bc97cd0828..014029b257 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 ListenerCallbacks; 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 @@ public: 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 @@ public: 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 @@ public: 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 0862e03c44..88c0c53e12 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 @@ public: // --------------------------------------------------------------------------- -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 @@ protected: 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 @@ public: 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 @@ public: 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 43cd0f8a7f..ace442339d 100644 --- a/libs/gui/tests/Surface_test.cpp +++ b/libs/gui/tests/Surface_test.cpp @@ -987,6 +987,19 @@ public: 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 1b6c598372..a37433cead 100644 --- a/services/surfaceflinger/Android.bp +++ b/services/surfaceflinger/Android.bp @@ -85,7 +85,7 @@ cc_defaults { "libui", "libutils", "libSurfaceFlingerProp", - "libaconfig_storage_read_api_cc" + "libaconfig_storage_read_api_cc", ], static_libs: [ "iinputflinger_aidl_lib_static", @@ -187,6 +187,7 @@ filegroup { "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 2596a25d15..f671006149 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 @@ void SurfaceFrame::onPresent(nsecs_t presentTime, int32_t displayFrameJankType, 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 0000000000..8e0e084cd2 --- /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 0000000000..5917358797 --- /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 d27bfd29ef..e758741afb 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -767,41 +767,6 @@ bool Layer::isSecure() const { 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 @@ std::shared_ptr<frametimeline::SurfaceFrame> Layer::createSurfaceFrameForTransac if (fps) { surfaceFrame->setRenderRate(*fps); } - onSurfaceFrameCreated(surfaceFrame); return surfaceFrame; } @@ -1453,7 +1417,6 @@ std::shared_ptr<frametimeline::SurfaceFrame> Layer::createSurfaceFrameForBuffer( if (fps) { surfaceFrame->setRenderRate(*fps); } - onSurfaceFrameCreated(surfaceFrame); return surfaceFrame; } @@ -1479,7 +1442,6 @@ void Layer::setFrameTimelineVsyncForSkippedFrames(const FrameTimelineInfo& info, if (fps) { surfaceFrame->setRenderRate(*fps); } - onSurfaceFrameCreated(surfaceFrame); addSurfaceFrameDroppedForBuffer(surfaceFrame, postTime); } @@ -2942,25 +2904,6 @@ void Layer::onLayerDisplayed(ftl::SharedFuture<FenceResult> futureFenceResult, } } -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 @@ void Layer::releasePendingBuffer(nsecs_t dequeueReadyTime) { } } - 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 @@ bool Layer::setTransactionCompletedListeners(const std::vector<sp<CallbackHandle 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 b9fcd5c333..d3b56f89d1 100644 --- a/services/surfaceflinger/Layer.h +++ b/services/surfaceflinger/Layer.h @@ -980,7 +980,6 @@ protected: void preparePerFrameBufferCompositionState(); void preparePerFrameEffectsCompositionState(); void gatherBufferInfo(); - void onSurfaceFrameCreated(const std::shared_ptr<frametimeline::SurfaceFrame>&); bool isClonedFromAlive() { return getClonedFrom() != nullptr; } @@ -1195,11 +1194,6 @@ private: 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 @@ private: // 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 5b40acf030..989020daeb 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 @@ uint32_t SurfaceFlinger::setClientStateLocked(const FrameTimelineInfo& frameTime } 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 @@ uint32_t SurfaceFlinger::updateLayerCallbacksAndStats(const FrameTimelineInfo& f } 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 @@ void SurfaceFlinger::onHandleDestroyed(BBinder* handle, sp<Layer>& layer, uint32 mTransactionHandler.onLayerDestroyed(layerId); } + JankTracker::flushJankData(layerId); + Mutex::Autolock lock(mStateLock); markLayerPendingRemovalLocked(layer); layer->onHandleDestroyed(); @@ -10463,6 +10462,28 @@ binder::Status SurfaceComposerAIDL::notifyShutdown() { 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 ee541c4364..6838994867 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 @@ public: 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 222ae30acb..b9555c04a8 100644 --- a/services/surfaceflinger/TransactionCallbackInvoker.cpp +++ b/services/surfaceflinger/TransactionCallbackInvoker.cpp @@ -64,13 +64,12 @@ status_t TransactionCallbackInvoker::addOnCommitCallbackHandles( 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::addOnCommitCallbackHandles( } 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 @@ status_t TransactionCallbackInvoker::findOrCreateTransactionStats( 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 @@ status_t TransactionCallbackInvoker::addCallbackHandle(const sp<CallbackHandle>& 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 cb7150b943..7853a9f359 100644 --- a/services/surfaceflinger/TransactionCallbackInvoker.h +++ b/services/surfaceflinger/TransactionCallbackInvoker.h @@ -63,8 +63,7 @@ public: 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 @@ public: 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 98d5754271..bc35639a00 100644 --- a/services/surfaceflinger/tests/unittests/Android.bp +++ b/services/surfaceflinger/tests/unittests/Android.bp @@ -84,6 +84,7 @@ cc_test { "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 dac9265b71..25437ca856 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 @@ public: 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 @@ public: [&](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 @@ TEST_F(FrameTimelineTest, presentFenceSignaled_presentedFramesUpdated) { 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 @@ TEST_F(FrameTimelineTest, displayFramesSlidingWindowMovesAfterLimit) { // 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 @@ TEST_F(FrameTimelineTest, presentFenceSignaled_doesNotReportForInvalidTokens) { presentFence1->signalForTest(70); mFrameTimeline->setSfPresent(59, presentFence1); + + EXPECT_EQ(getLayerOneJankData().size(), 0u); } TEST_F(FrameTimelineTest, presentFenceSignaled_reportsLongSfCpu) { @@ -603,6 +626,10 @@ TEST_F(FrameTimelineTest, presentFenceSignaled_reportsLongSfCpu) { 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 @@ TEST_F(FrameTimelineTest, presentFenceSignaled_reportsLongSfGpu) { 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 @@ TEST_F(FrameTimelineTest, presentFenceSignaled_reportsDisplayMiss) { 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 @@ TEST_F(FrameTimelineTest, presentFenceSignaled_reportsAppMiss) { 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 @@ TEST_F(FrameTimelineTest, presentFenceSignaled_reportsSfScheduling) { 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 @@ TEST_F(FrameTimelineTest, presentFenceSignaled_reportsSfPredictionError) { 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 @@ TEST_F(FrameTimelineTest, presentFenceSignaled_reportsAppBufferStuffing) { 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 @@ TEST_F(FrameTimelineTest, presentFenceSignaled_reportsAppMissWithRenderRate) { 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 @@ TEST_F(FrameTimelineTest, presentFenceSignaled_displayFramePredictionExpiredPres 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 @@ TEST_F(FrameTimelineTest, jankClassification_presentOnTimeDoesNotClassify) { 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 0000000000..2941a14ef9 --- /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 46733b9a83..0745f870e4 100644 --- a/services/surfaceflinger/tests/unittests/TransactionSurfaceFrameTest.cpp +++ b/services/surfaceflinger/tests/unittests/TransactionSurfaceFrameTest.cpp @@ -302,58 +302,6 @@ public: 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 @@ public: 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 @@ public: 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 @@ TEST_F(TransactionSurfaceFrameTest, MultipleSurfaceFramesPresentedTogether) { MultipleSurfaceFramesPresentedTogether(); } -TEST_F(TransactionSurfaceFrameTest, PendingSurfaceFramesRemovedAfterClassification) { - PendingSurfaceFramesRemovedAfterClassification(); -} - TEST_F(TransactionSurfaceFrameTest, BufferSurfaceFrame_ReplaceValidTokenBufferWithInvalidTokenBuffer) { BufferSurfaceFrame_ReplaceValidTokenBufferWithInvalidTokenBuffer(); |