Merge "SF: Remove EventLog frame duration" into main

8 files changed, 2 insertions, 294 deletions

diff --git a/services/surfaceflinger/Android.bp b/services/surfaceflinger/Android.bp
index c2a9880d87..8161b4731e 100644
--- a/services/surfaceflinger/Android.bp
+++ b/services/surfaceflinger/Android.bp
@@ -173,7 +173,6 @@ filegroup {
         "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 3b609524e8..0000000000
--- 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 ee3587ef8a..0000000000
--- 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 6c851dd09c..e68d9f5ec0 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 ca8cdc3c0a..93d0313e53 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::setDisplayRefreshPeriod(nsecs_t displayPeriod) {
 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::getStats(FrameStats* outStats) const {
     }
 }
 
-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 @@ void FrameTracker::processFencesLocked() const {
             if (records[idx].frameReadyTime < INT64_MAX) {
                 records[idx].frameReadyFence = nullptr;
                 numFences--;
-                updated = true;
             }
         }
 
@@ -169,58 +152,7 @@ void FrameTracker::processFencesLocked() const {
             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 bc412aee2f..fd6fadc6dd 100644
--- a/services/surfaceflinger/FrameTracker.h
+++ b/services/surfaceflinger/FrameTracker.h
@@ -41,8 +41,6 @@ public:
     // 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 @@ private:
     // 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 9fdcbd0f1b..92894847fa 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -183,7 +183,6 @@ Layer::~Layer() {
     mFlinger->mTimeStats->onDestroy(layerId);
     mFlinger->mFrameTracer->onDestroy(layerId);
 
-    mFrameTracker.logAndResetStats(mName);
     mFlinger->onLayerDestroyed(this);
 
     if (mDrawingState.sidebandStream != nullptr) {
@@ -605,10 +604,6 @@ void Layer::clearFrameStats() {
     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 a821e3d494..5010ba15bf 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -3284,8 +3284,6 @@ void SurfaceFlinger::onCompositionPresented(PhysicalDisplayId pacesetterId,
         // 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() {