SF: Remove EventLog frame duration
In the process of replacing FrameTracker it became apparent that there
was a good deal of supporting logic for frame duration. This logic was
flawed (all high numbers go to the last bucket) and is not ideal to
replicate in the new FrameTimeline based future.
By removing this, FrameTracker replacement is significantly easier.
Bug: 241394120
Flag: EXEMPT EventLog removal
Test: presubmit
Change-Id: I526561b11c21cd2872e9f128849b33883ae9c948
diff --git a/services/surfaceflinger/Android.bp b/services/surfaceflinger/Android.bp
index c2a9880..8161b47 100644
--- a/services/surfaceflinger/Android.bp
+++ b/services/surfaceflinger/Android.bp
@@ -173,7 +173,6 @@
"DisplayHardware/VirtualDisplaySurface.cpp",
"DisplayRenderArea.cpp",
"Effects/Daltonizer.cpp",
- "EventLog/EventLog.cpp",
"FrontEnd/LayerCreationArgs.cpp",
"FrontEnd/LayerHandle.cpp",
"FrontEnd/LayerSnapshot.cpp",
diff --git a/services/surfaceflinger/EventLog/EventLog.cpp b/services/surfaceflinger/EventLog/EventLog.cpp
deleted file mode 100644
index 3b60952..0000000
--- a/services/surfaceflinger/EventLog/EventLog.cpp
+++ /dev/null
@@ -1,133 +0,0 @@
-/*
- * Copyright 2013 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.
- */
-
-// TODO(b/129481165): remove the #pragma below and fix conversion issues
-#pragma clang diagnostic push
-#pragma clang diagnostic ignored "-Wconversion"
-
-#include <stdio.h>
-#include <stdlib.h>
-#include <log/log.h>
-
-#include "EventLog.h"
-
-namespace android {
-
-ANDROID_SINGLETON_STATIC_INSTANCE(EventLog)
-
-
-EventLog::EventLog() {
-}
-
-void EventLog::doLogFrameDurations(const std::string_view& name, const int32_t* durations,
- size_t numDurations) {
- EventLog::TagBuffer buffer(LOGTAG_SF_FRAME_DUR);
- buffer.startList(1 + numDurations);
- buffer.writeString(name);
- for (size_t i = 0; i < numDurations; i++) {
- buffer.writeInt32(durations[i]);
- }
- buffer.endList();
- buffer.log();
-}
-
-void EventLog::logFrameDurations(const std::string_view& name, const int32_t* durations,
- size_t numDurations) {
- EventLog::getInstance().doLogFrameDurations(name, durations, numDurations);
-}
-
-// ---------------------------------------------------------------------------
-
-EventLog::TagBuffer::TagBuffer(int32_t tag)
- : mPos(0), mTag(tag), mOverflow(false) {
-}
-
-void EventLog::TagBuffer::log() {
- if (mOverflow) {
- ALOGW("couldn't log to binary event log: overflow.");
- } else if (android_bWriteLog(mTag, mStorage, mPos) < 0) {
- ALOGE("couldn't log to EventLog: %s", strerror(errno));
- }
- // purge the buffer
- mPos = 0;
- mOverflow = false;
-}
-
-void EventLog::TagBuffer::startList(int8_t count) {
- if (mOverflow) return;
- const size_t needed = 1 + sizeof(count);
- if (mPos + needed > STORAGE_MAX_SIZE) {
- mOverflow = true;
- return;
- }
- mStorage[mPos + 0] = EVENT_TYPE_LIST;
- mStorage[mPos + 1] = count;
- mPos += needed;
-}
-
-void EventLog::TagBuffer::endList() {
- if (mOverflow) return;
- const size_t needed = 1;
- if (mPos + needed > STORAGE_MAX_SIZE) {
- mOverflow = true;
- return;
- }
- mStorage[mPos + 0] = '\n';
- mPos += needed;
-}
-
-void EventLog::TagBuffer::writeInt32(int32_t value) {
- if (mOverflow) return;
- const size_t needed = 1 + sizeof(value);
- if (mPos + needed > STORAGE_MAX_SIZE) {
- mOverflow = true;
- return;
- }
- mStorage[mPos + 0] = EVENT_TYPE_INT;
- memcpy(&mStorage[mPos + 1], &value, sizeof(value));
- mPos += needed;
-}
-
-void EventLog::TagBuffer::writeInt64(int64_t value) {
- if (mOverflow) return;
- const size_t needed = 1 + sizeof(value);
- if (mPos + needed > STORAGE_MAX_SIZE) {
- mOverflow = true;
- return;
- }
- mStorage[mPos + 0] = EVENT_TYPE_LONG;
- memcpy(&mStorage[mPos + 1], &value, sizeof(value));
- mPos += needed;
-}
-
-void EventLog::TagBuffer::writeString(const std::string_view& value) {
- if (mOverflow) return;
- const size_t stringLen = value.length();
- const size_t needed = 1 + sizeof(int32_t) + stringLen;
- if (mPos + needed > STORAGE_MAX_SIZE) {
- mOverflow = true;
- return;
- }
- mStorage[mPos + 0] = EVENT_TYPE_STRING;
- memcpy(&mStorage[mPos + 1], &stringLen, sizeof(int32_t));
- memcpy(&mStorage[mPos + 5], value.data(), stringLen);
- mPos += needed;
-}
-
-} // namespace android
-
-// TODO(b/129481165): remove the #pragma below and fix conversion issues
-#pragma clang diagnostic pop // ignored "-Wconversion"
diff --git a/services/surfaceflinger/EventLog/EventLog.h b/services/surfaceflinger/EventLog/EventLog.h
deleted file mode 100644
index ee3587e..0000000
--- a/services/surfaceflinger/EventLog/EventLog.h
+++ /dev/null
@@ -1,73 +0,0 @@
-/*
- * Copyright 2013 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 <utils/Errors.h>
-#include <utils/Singleton.h>
-
-#include <cstdint>
-#include <string_view>
-
-namespace android {
-
-class EventLog : public Singleton<EventLog> {
-
-public:
- static void logFrameDurations(const std::string_view& name, const int32_t* durations,
- size_t numDurations);
-
-protected:
- EventLog();
-
-private:
- /*
- * EventLogBuffer is a helper class to construct an in-memory event log
- * tag. In this version the buffer is not dynamic, so write operation can
- * fail if there is not enough space in the temporary buffer.
- * Once constructed, the buffer can be logger by calling the log()
- * method.
- */
-
- class TagBuffer {
- enum { STORAGE_MAX_SIZE = 128 };
- int32_t mPos;
- int32_t mTag;
- bool mOverflow;
- char mStorage[STORAGE_MAX_SIZE];
- public:
- explicit TagBuffer(int32_t tag);
-
- void startList(int8_t count);
- void endList();
-
- void writeInt32(int32_t);
- void writeInt64(int64_t);
- void writeString(const std::string_view&);
-
- void log();
- };
-
- friend class Singleton<EventLog>;
- EventLog(const EventLog&);
- EventLog& operator =(const EventLog&);
-
- enum { LOGTAG_SF_FRAME_DUR = 60100 };
- void doLogFrameDurations(const std::string_view& name, const int32_t* durations,
- size_t numDurations);
-};
-
-} // namespace android
diff --git a/services/surfaceflinger/EventLog/EventLogTags.logtags b/services/surfaceflinger/EventLog/EventLogTags.logtags
index 6c851dd..e68d9f5 100644
--- a/services/surfaceflinger/EventLog/EventLogTags.logtags
+++ b/services/surfaceflinger/EventLog/EventLogTags.logtags
@@ -35,7 +35,6 @@
# 60100 - 60199 reserved for surfaceflinger
-60100 sf_frame_dur (window|3),(dur0|1),(dur1|1),(dur2|1),(dur3|1),(dur4|1),(dur5|1),(dur6|1)
60110 sf_stop_bootanim (time|2|3)
# NOTE - the range 1000000-2000000 is reserved for partners and others who
diff --git a/services/surfaceflinger/FrameTracker.cpp b/services/surfaceflinger/FrameTracker.cpp
index ca8cdc3..93d0313 100644
--- a/services/surfaceflinger/FrameTracker.cpp
+++ b/services/surfaceflinger/FrameTracker.cpp
@@ -26,16 +26,10 @@
#include <ui/FrameStats.h>
#include "FrameTracker.h"
-#include "EventLog/EventLog.h"
namespace android {
-FrameTracker::FrameTracker() :
- mOffset(0),
- mNumFences(0),
- mDisplayPeriod(0) {
- resetFrameCountersLocked();
-}
+FrameTracker::FrameTracker() : mOffset(0), mNumFences(0), mDisplayPeriod(0) {}
void FrameTracker::setDesiredPresentTime(nsecs_t presentTime) {
Mutex::Autolock lock(mMutex);
@@ -73,9 +67,6 @@
void FrameTracker::advanceFrame() {
Mutex::Autolock lock(mMutex);
- // Update the statistic to include the frame we just finished.
- updateStatsLocked(mOffset);
-
// Advance to the next frame.
mOffset = (mOffset+1) % NUM_FRAME_RECORDS;
mFrameRecords[mOffset].desiredPresentTime = INT64_MAX;
@@ -138,19 +129,12 @@
}
}
-void FrameTracker::logAndResetStats(const std::string_view& name) {
- Mutex::Autolock lock(mMutex);
- logStatsLocked(name);
- resetFrameCountersLocked();
-}
-
void FrameTracker::processFencesLocked() const {
FrameRecord* records = const_cast<FrameRecord*>(mFrameRecords);
int& numFences = const_cast<int&>(mNumFences);
for (int i = 1; i < NUM_FRAME_RECORDS && numFences > 0; i++) {
- size_t idx = (mOffset+NUM_FRAME_RECORDS-i) % NUM_FRAME_RECORDS;
- bool updated = false;
+ size_t idx = (mOffset + NUM_FRAME_RECORDS - i) % NUM_FRAME_RECORDS;
const std::shared_ptr<FenceTime>& rfence = records[idx].frameReadyFence;
if (rfence != nullptr) {
@@ -158,7 +142,6 @@
if (records[idx].frameReadyTime < INT64_MAX) {
records[idx].frameReadyFence = nullptr;
numFences--;
- updated = true;
}
}
@@ -169,59 +152,8 @@
if (records[idx].actualPresentTime < INT64_MAX) {
records[idx].actualPresentFence = nullptr;
numFences--;
- updated = true;
}
}
-
- if (updated) {
- updateStatsLocked(idx);
- }
- }
-}
-
-void FrameTracker::updateStatsLocked(size_t newFrameIdx) const {
- int* numFrames = const_cast<int*>(mNumFrames);
-
- if (mDisplayPeriod > 0 && isFrameValidLocked(newFrameIdx)) {
- size_t prevFrameIdx = (newFrameIdx+NUM_FRAME_RECORDS-1) %
- NUM_FRAME_RECORDS;
-
- if (isFrameValidLocked(prevFrameIdx)) {
- nsecs_t newPresentTime =
- mFrameRecords[newFrameIdx].actualPresentTime;
- nsecs_t prevPresentTime =
- mFrameRecords[prevFrameIdx].actualPresentTime;
-
- nsecs_t duration = newPresentTime - prevPresentTime;
- int numPeriods = int((duration + mDisplayPeriod/2) /
- mDisplayPeriod);
-
- for (int i = 0; i < NUM_FRAME_BUCKETS-1; i++) {
- int nextBucket = 1 << (i+1);
- if (numPeriods < nextBucket) {
- numFrames[i]++;
- return;
- }
- }
-
- // The last duration bucket is a catch-all.
- numFrames[NUM_FRAME_BUCKETS-1]++;
- }
- }
-}
-
-void FrameTracker::resetFrameCountersLocked() {
- for (int i = 0; i < NUM_FRAME_BUCKETS; i++) {
- mNumFrames[i] = 0;
- }
-}
-
-void FrameTracker::logStatsLocked(const std::string_view& name) const {
- for (int i = 0; i < NUM_FRAME_BUCKETS; i++) {
- if (mNumFrames[i] > 0) {
- EventLog::logFrameDurations(name, mNumFrames, NUM_FRAME_BUCKETS);
- return;
- }
}
}
diff --git a/services/surfaceflinger/FrameTracker.h b/services/surfaceflinger/FrameTracker.h
index bc412ae..fd6fadc 100644
--- a/services/surfaceflinger/FrameTracker.h
+++ b/services/surfaceflinger/FrameTracker.h
@@ -41,8 +41,6 @@
// frame time history.
enum { NUM_FRAME_RECORDS = 128 };
- enum { NUM_FRAME_BUCKETS = 7 };
-
FrameTracker();
// setDesiredPresentTime sets the time at which the current frame
@@ -142,13 +140,6 @@
// doesn't grow with NUM_FRAME_RECORDS.
int mNumFences;
- // mNumFrames keeps a count of the number of frames with a duration in a
- // particular range of vsync periods. Element n of the array stores the
- // number of frames with duration in the half-inclusive range
- // [2^n, 2^(n+1)). The last element of the array contains the count for
- // all frames with duration greater than 2^(NUM_FRAME_BUCKETS-1).
- int32_t mNumFrames[NUM_FRAME_BUCKETS];
-
// mDisplayPeriod is the display refresh period of the display for which
// this FrameTracker is gathering information.
nsecs_t mDisplayPeriod;
diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index dcb0812..b323b98 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -183,7 +183,6 @@
mFlinger->mTimeStats->onDestroy(layerId);
mFlinger->mFrameTracer->onDestroy(layerId);
- mFrameTracker.logAndResetStats(mName);
mFlinger->onLayerDestroyed(this);
if (mDrawingState.sidebandStream != nullptr) {
@@ -605,10 +604,6 @@
mFrameTracker.clearStats();
}
-void Layer::logFrameStats() {
- mFrameTracker.logAndResetStats(mName);
-}
-
void Layer::getFrameStats(FrameStats* outStats) const {
mFrameTracker.getStats(outStats);
}
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 65a0ed3..170052d 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -3264,8 +3264,6 @@
// getTotalSize returns the total number of buffers that were allocated by SurfaceFlinger
SFTRACE_INT64("Total Buffer Size", GraphicBufferAllocator::get().getTotalSize());
}
-
- logFrameStats(presentTime);
}
void SurfaceFlinger::commitTransactions() {