SF TimeStats: Remove some redundant functions
This change also tunes TimeStats for unittesting purpose.
Bug: 119290000
Test: atest libsurfaceflinger_unittest:TimeStatsTest
Change-Id: I9d9adb589bf4c41d362fa6eab71af094bd82fcb6
diff --git a/services/surfaceflinger/BufferQueueLayer.cpp b/services/surfaceflinger/BufferQueueLayer.cpp
index 0f2a15d..b784d11 100644
--- a/services/surfaceflinger/BufferQueueLayer.cpp
+++ b/services/surfaceflinger/BufferQueueLayer.cpp
@@ -262,7 +262,7 @@
Mutex::Autolock lock(mQueueItemLock);
mQueueItems.clear();
mQueuedFrames = 0;
- mFlinger->mTimeStats->clearLayerRecord(layerID);
+ mFlinger->mTimeStats->onDestroy(layerID);
}
// Once we have hit this state, the shadow queue may no longer
diff --git a/services/surfaceflinger/BufferStateLayer.cpp b/services/surfaceflinger/BufferStateLayer.cpp
index 7a55d00..1f71a44 100644
--- a/services/surfaceflinger/BufferStateLayer.cpp
+++ b/services/surfaceflinger/BufferStateLayer.cpp
@@ -471,7 +471,7 @@
if (SyncFeatures::getInstance().useNativeFenceSync() && releaseFence != Fence::NO_FENCE) {
// TODO(alecmouri): Fail somewhere upstream if the fence is invalid.
if (!releaseFence->isValid()) {
- mFlinger->mTimeStats->clearLayerRecord(layerID);
+ mFlinger->mTimeStats->onDestroy(layerID);
return UNKNOWN_ERROR;
}
@@ -481,7 +481,7 @@
auto currentStatus = s.acquireFence->getStatus();
if (currentStatus == Fence::Status::Invalid) {
ALOGE("Existing fence has invalid state");
- mFlinger->mTimeStats->clearLayerRecord(layerID);
+ mFlinger->mTimeStats->onDestroy(layerID);
return BAD_VALUE;
}
@@ -489,7 +489,7 @@
if (incomingStatus == Fence::Status::Invalid) {
ALOGE("New fence has invalid state");
mDrawingState.acquireFence = releaseFence;
- mFlinger->mTimeStats->clearLayerRecord(layerID);
+ mFlinger->mTimeStats->onDestroy(layerID);
return BAD_VALUE;
}
@@ -505,7 +505,7 @@
// synchronization is broken, the best we can do is hope fences
// signal in order so the new fence will act like a union
mDrawingState.acquireFence = releaseFence;
- mFlinger->mTimeStats->clearLayerRecord(layerID);
+ mFlinger->mTimeStats->onDestroy(layerID);
return BAD_VALUE;
}
mDrawingState.acquireFence = mergedFence;
@@ -528,7 +528,7 @@
// a GL-composited layer) not at all.
status_t err = bindTextureImage();
if (err != NO_ERROR) {
- mFlinger->mTimeStats->clearLayerRecord(layerID);
+ mFlinger->mTimeStats->onDestroy(layerID);
return BAD_VALUE;
}
}
diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index 7526ed8..2ac42e8 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -1523,7 +1523,7 @@
void Layer::onDisconnect() {
Mutex::Autolock lock(mFrameEventHistoryMutex);
mFrameEventHistory.onDisconnect();
- mFlinger->mTimeStats->onDisconnect(getSequence());
+ mFlinger->mTimeStats->onDestroy(getSequence());
}
void Layer::addAndGetFrameTimestamps(const NewFrameEventsEntry* newTimestamps,
diff --git a/services/surfaceflinger/TimeStats/TimeStats.cpp b/services/surfaceflinger/TimeStats/TimeStats.cpp
index a5bc401..2b9f5c8 100644
--- a/services/surfaceflinger/TimeStats/TimeStats.cpp
+++ b/services/surfaceflinger/TimeStats/TimeStats.cpp
@@ -263,11 +263,9 @@
if (!mTimeStatsTracker.count(layerID)) return;
LayerRecord& layerRecord = mTimeStatsTracker[layerID];
if (layerRecord.timeRecords.size() == MAX_NUM_TIME_RECORDS) {
- ALOGE("[%d]-[%s]-timeRecords is already at its maximum size[%zu]. Please file a bug.",
+ ALOGE("[%d]-[%s]-timeRecords is at its maximum size[%zu]. Ignore this when unittesting.",
layerID, layerRecord.layerName.c_str(), MAX_NUM_TIME_RECORDS);
- layerRecord.timeRecords.clear();
- layerRecord.prevTimeRecord.ready = false;
- layerRecord.waitData = -1;
+ mTimeStatsTracker.erase(layerID);
return;
}
// For most media content, the acquireFence is invalid because the buffer is
@@ -278,7 +276,9 @@
{
.frameNumber = frameNumber,
.postTime = postTime,
+ .latchTime = postTime,
.acquireTime = postTime,
+ .desiredTime = postTime,
},
};
layerRecord.timeRecords.push_back(timeRecord);
@@ -389,18 +389,6 @@
flushAvailableRecordsToStatsLocked(layerID);
}
-void TimeStats::onDisconnect(int32_t layerID) {
- if (!mEnabled.load()) return;
-
- ATRACE_CALL();
- ALOGV("[%d]-onDisconnect", layerID);
-
- std::lock_guard<std::mutex> lock(mMutex);
- if (!mTimeStatsTracker.count(layerID)) return;
- flushAvailableRecordsToStatsLocked(layerID);
- mTimeStatsTracker.erase(layerID);
-}
-
void TimeStats::onDestroy(int32_t layerID) {
if (!mEnabled.load()) return;
@@ -409,25 +397,9 @@
std::lock_guard<std::mutex> lock(mMutex);
if (!mTimeStatsTracker.count(layerID)) return;
- flushAvailableRecordsToStatsLocked(layerID);
mTimeStatsTracker.erase(layerID);
}
-void TimeStats::clearLayerRecord(int32_t layerID) {
- if (!mEnabled.load()) return;
-
- ATRACE_CALL();
- ALOGV("[%d]-clearLayerRecord", layerID);
-
- std::lock_guard<std::mutex> lock(mMutex);
- if (!mTimeStatsTracker.count(layerID)) return;
- LayerRecord& layerRecord = mTimeStatsTracker[layerID];
- layerRecord.timeRecords.clear();
- layerRecord.prevTimeRecord.ready = false;
- layerRecord.waitData = -1;
- layerRecord.droppedFrames = 0;
-}
-
void TimeStats::removeTimeRecord(int32_t layerID, uint64_t frameNumber) {
if (!mEnabled.load()) return;
diff --git a/services/surfaceflinger/TimeStats/TimeStats.h b/services/surfaceflinger/TimeStats/TimeStats.h
index 57754ac..0b24c46 100644
--- a/services/surfaceflinger/TimeStats/TimeStats.h
+++ b/services/surfaceflinger/TimeStats/TimeStats.h
@@ -38,10 +38,6 @@
class String8;
class TimeStats {
- // TODO(zzyiwei): Bound the timeStatsTracker with weighted LRU
- // static const size_t MAX_NUM_LAYER_RECORDS = 200;
- static const size_t MAX_NUM_TIME_RECORDS = 64;
-
struct FrameTime {
uint64_t frameNumber = 0;
nsecs_t postTime = 0;
@@ -84,6 +80,7 @@
~TimeStats() = default;
void parseArgs(bool asProto, const Vector<String16>& args, size_t& index, String8& result);
+ bool isEnabled();
void incrementTotalFrames();
void incrementMissedFrames();
@@ -99,18 +96,18 @@
void setPresentTime(int32_t layerID, uint64_t frameNumber, nsecs_t presentTime);
void setPresentFence(int32_t layerID, uint64_t frameNumber,
const std::shared_ptr<FenceTime>& presentFence);
- // On producer disconnect with BufferQueue.
- void onDisconnect(int32_t layerID);
- // On layer tear down.
+ // Clean up the layer record
void onDestroy(int32_t layerID);
- // When SF is cleaning up the queue, clear the LayerRecord as well.
- void clearLayerRecord(int32_t layerID);
// If SF skips or rejects a buffer, remove the corresponding TimeRecord.
void removeTimeRecord(int32_t layerID, uint64_t frameNumber);
void setPowerMode(int32_t powerMode);
void setPresentFenceGlobal(const std::shared_ptr<FenceTime>& presentFence);
+ // TODO(zzyiwei): Bound the timeStatsTracker with weighted LRU
+ // static const size_t MAX_NUM_LAYER_RECORDS = 200;
+ static const size_t MAX_NUM_TIME_RECORDS = 64;
+
private:
bool recordReadyLocked(int32_t layerID, TimeRecord* timeRecord);
void flushAvailableRecordsToStatsLocked(int32_t layerID);
@@ -120,7 +117,6 @@
void enable();
void disable();
void clear();
- bool isEnabled();
void dump(bool asProto, std::optional<uint32_t> maxLayers, String8& result);
std::atomic<bool> mEnabled = false;