Merge changes Ie828d9b4,I5ddd6323 into pi-dev

* changes:
  SF TimeStats: move dumpStats into TimeStatsHelper
  SF TimeStats: update build file and some cleanups

7 files changed, 86 insertions, 117 deletions

diff --git a/services/surfaceflinger/TimeStats/TimeStats.cpp b/services/surfaceflinger/TimeStats/TimeStats.cpp
index b207d9993a..d4f1e29042 100644
--- a/services/surfaceflinger/TimeStats/TimeStats.cpp
+++ b/services/surfaceflinger/TimeStats/TimeStats.cpp
@@ -59,14 +59,15 @@ void TimeStats::parseArgs(bool asProto, const Vector<String16>& args, size_t& in
     }
 
     if (argsMap.count("-dump")) {
-        int64_t maxLayers = 0;
+        std::optional<uint32_t> maxLayers = std::nullopt;
         auto iter = argsMap.find("-maxlayers");
         if (iter != argsMap.end() && iter->second + 1 < static_cast<int32_t>(args.size())) {
-            maxLayers = strtol(String8(args[iter->second + 1]).c_str(), nullptr, 10);
-            maxLayers = std::clamp(maxLayers, int64_t(0), int64_t(UINT32_MAX));
+            int64_t value = strtol(String8(args[iter->second + 1]).c_str(), nullptr, 10);
+            value = std::clamp(value, int64_t(0), int64_t(UINT32_MAX));
+            maxLayers = static_cast<uint32_t>(value);
         }
 
-        dump(asProto, static_cast<uint32_t>(maxLayers), result);
+        dump(asProto, maxLayers, result);
     }
 
     if (argsMap.count("-clear")) {
@@ -147,6 +148,21 @@ static int32_t msBetween(nsecs_t start, nsecs_t end) {
     return static_cast<int32_t>(delta);
 }
 
+static std::string getPackageName(const std::string& layerName) {
+    // This regular expression captures the following for instance:
+    // StatusBar in StatusBar#0
+    // com.appname in com.appname/com.appname.activity#0
+    // com.appname in SurfaceView - com.appname/com.appname.activity#0
+    const std::regex re("(?:SurfaceView[-\\s\\t]+)?([^/]+).*#\\d+");
+    std::smatch match;
+    if (std::regex_match(layerName.begin(), layerName.end(), match, re)) {
+        // There must be a match for group 1 otherwise the whole string is not
+        // matched and the above will return false
+        return match[1];
+    }
+    return "";
+}
+
 void TimeStats::flushAvailableRecordsToStatsLocked(const std::string& layerName) {
     ATRACE_CALL();
 
@@ -161,6 +177,7 @@ void TimeStats::flushAvailableRecordsToStatsLocked(const std::string& layerName)
         if (prevTimeRecord.ready) {
             if (!timeStats.stats.count(layerName)) {
                 timeStats.stats[layerName].layerName = layerName;
+                timeStats.stats[layerName].packageName = getPackageName(layerName);
                 timeStats.stats[layerName].statsStart = static_cast<int64_t>(std::time(0));
             }
             TimeStatsHelper::TimeStatsLayer& timeStatsLayer = timeStats.stats[layerName];
@@ -433,7 +450,6 @@ void TimeStats::clear() {
 
     std::lock_guard<std::mutex> lock(mMutex);
     ALOGD("Cleared");
-    timeStats.dumpStats.clear();
     timeStats.stats.clear();
     timeStats.statsStart = (mEnabled.load() ? static_cast<int64_t>(std::time(0)) : 0);
     timeStats.statsEnd = 0;
@@ -446,7 +462,7 @@ bool TimeStats::isEnabled() {
     return mEnabled.load();
 }
 
-void TimeStats::dump(bool asProto, uint32_t maxLayers, String8& result) {
+void TimeStats::dump(bool asProto, std::optional<uint32_t> maxLayers, String8& result) {
     ATRACE_CALL();
 
     std::lock_guard<std::mutex> lock(mMutex);
@@ -456,39 +472,15 @@ void TimeStats::dump(bool asProto, uint32_t maxLayers, String8& result) {
 
     timeStats.statsEnd = static_cast<int64_t>(std::time(0));
 
-    // TODO(zzyiwei): refactor dumpStats into TimeStatsHelper
-    timeStats.dumpStats.clear();
-    for (auto& ele : timeStats.stats) {
-        timeStats.dumpStats.push_back(&ele.second);
-    }
-
-    std::sort(timeStats.dumpStats.begin(), timeStats.dumpStats.end(),
-              [](TimeStatsHelper::TimeStatsLayer* const& l,
-                 TimeStatsHelper::TimeStatsLayer* const& r) {
-                  return l->totalFrames > r->totalFrames;
-              });
-
-    if (maxLayers != 0 && maxLayers < timeStats.dumpStats.size()) {
-        timeStats.dumpStats.resize(maxLayers);
-    }
-
     if (asProto) {
-        dumpAsProtoLocked(result);
+        ALOGD("Dumping TimeStats as proto");
+        SFTimeStatsGlobalProto timeStatsProto = timeStats.toProto(maxLayers);
+        result.append(timeStatsProto.SerializeAsString().c_str(), timeStatsProto.ByteSize());
     } else {
-        dumpAsTextLocked(result);
+        ALOGD("Dumping TimeStats as text");
+        result.append(timeStats.toString(maxLayers).c_str());
+        result.append("\n");
     }
 }
 
-void TimeStats::dumpAsTextLocked(String8& result) {
-    ALOGD("Dumping TimeStats as text");
-    result.append(timeStats.toString().c_str());
-    result.append("\n");
-}
-
-void TimeStats::dumpAsProtoLocked(String8& result) {
-    ALOGD("Dumping TimeStats as proto");
-    SFTimeStatsGlobalProto timeStatsProto = timeStats.toProto();
-    result.append(timeStatsProto.SerializeAsString().c_str(), timeStatsProto.ByteSize());
-}
-
 } // namespace android
diff --git a/services/surfaceflinger/TimeStats/TimeStats.h b/services/surfaceflinger/TimeStats/TimeStats.h
index 8399c5dfca..831821000f 100644
--- a/services/surfaceflinger/TimeStats/TimeStats.h
+++ b/services/surfaceflinger/TimeStats/TimeStats.h
@@ -27,6 +27,7 @@
 
 #include <deque>
 #include <mutex>
+#include <optional>
 #include <unordered_map>
 
 using namespace android::surfaceflinger;
@@ -90,9 +91,7 @@ private:
     void disable();
     void clear();
     bool isEnabled();
-    void dump(bool asProto, uint32_t maxLayer, String8& result);
-    void dumpAsTextLocked(String8& result);
-    void dumpAsProtoLocked(String8& result);
+    void dump(bool asProto, std::optional<uint32_t> maxLayers, String8& result);
 
     std::atomic<bool> mEnabled = false;
     std::mutex mMutex;
diff --git a/services/surfaceflinger/TimeStats/timestatsproto/Android.bp b/services/surfaceflinger/TimeStats/timestatsproto/Android.bp
index 66aa719381..bef6b7c2eb 100644
--- a/services/surfaceflinger/TimeStats/timestatsproto/Android.bp
+++ b/services/surfaceflinger/TimeStats/timestatsproto/Android.bp
@@ -1,6 +1,5 @@
 cc_library_shared {
     name: "libtimestats_proto",
-    vendor_available: true,
     export_include_dirs: ["include"],
 
     srcs: [
@@ -9,11 +8,8 @@ cc_library_shared {
     ],
 
     shared_libs: [
-        "android.hardware.graphics.common@1.1",
-        "libui",
-        "libprotobuf-cpp-lite",
         "libbase",
-        "liblog",
+        "libprotobuf-cpp-lite",
     ],
 
     proto: {
@@ -21,35 +17,17 @@ cc_library_shared {
     },
 
     cppflags: [
+        "-std=c++1z",
         "-Werror",
-        "-Wno-unused-parameter",
-        "-Wno-format",
         "-Wno-c++98-compat-pedantic",
-        "-Wno-float-conversion",
         "-Wno-disabled-macro-expansion",
+        "-Wno-float-conversion",
         "-Wno-float-equal",
-        "-Wno-sign-conversion",
-        "-Wno-padded",
+        "-Wno-format",
         "-Wno-old-style-cast",
+        "-Wno-padded",
+        "-Wno-sign-conversion",
         "-Wno-undef",
+        "-Wno-unused-parameter",
     ],
-
-}
-
-java_library_static {
-    name: "timestatsprotosnano",
-    host_supported: true,
-    proto: {
-        type: "nano",
-    },
-    srcs: ["*.proto"],
-    no_framework_libs: true,
-    target: {
-        android: {
-            jarjar_rules: "jarjar-rules.txt",
-        },
-        host: {
-            static_libs: ["libprotobuf-java-nano"],
-        },
-    },
 }
diff --git a/services/surfaceflinger/TimeStats/timestatsproto/TimeStatsHelper.cpp b/services/surfaceflinger/TimeStats/timestatsproto/TimeStatsHelper.cpp
index 3e5007c0b8..21f3ef3d7e 100644
--- a/services/surfaceflinger/TimeStats/timestatsproto/TimeStatsHelper.cpp
+++ b/services/surfaceflinger/TimeStats/timestatsproto/TimeStatsHelper.cpp
@@ -17,7 +17,6 @@
 #include <timestatsproto/TimeStatsHelper.h>
 
 #include <array>
-#include <regex>
 
 #define HISTOGRAM_SIZE 85
 
@@ -47,55 +46,39 @@ void TimeStatsHelper::Histogram::insert(int32_t delta) {
     hist[*iter]++;
 }
 
-float TimeStatsHelper::Histogram::averageTime() {
+float TimeStatsHelper::Histogram::averageTime() const {
     int64_t ret = 0;
     int64_t count = 0;
-    for (auto ele : hist) {
+    for (auto& ele : hist) {
         count += ele.second;
         ret += ele.first * ele.second;
     }
     return static_cast<float>(ret) / count;
 }
 
-std::string TimeStatsHelper::Histogram::toString() {
+std::string TimeStatsHelper::Histogram::toString() const {
     std::string result;
     for (int32_t i = 0; i < HISTOGRAM_SIZE; ++i) {
         int32_t bucket = histogramConfig[i];
-        int32_t count = (hist.count(bucket) == 0) ? 0 : hist[bucket];
+        int32_t count = (hist.count(bucket) == 0) ? 0 : hist.at(bucket);
         StringAppendF(&result, "%dms=%d ", bucket, count);
     }
     result.back() = '\n';
     return result;
 }
 
-static std::string getPackageName(const std::string& layerName) {
-    // This regular expression captures the following for instance:
-    // StatusBar in StatusBar#0
-    // com.appname in com.appname/com.appname.activity#0
-    // com.appname in SurfaceView - com.appname/com.appname.activity#0
-    const std::regex re("(?:SurfaceView[-\\s\\t]+)?([^/]+).*#\\d+");
-    std::smatch match;
-    if (std::regex_match(layerName.begin(), layerName.end(), match, re)) {
-        // There must be a match for group 1 otherwise the whole string is not
-        // matched and the above will return false
-        return match[1];
-    }
-    return "";
-}
-
-std::string TimeStatsHelper::TimeStatsLayer::toString() {
+std::string TimeStatsHelper::TimeStatsLayer::toString() const {
     std::string result = "";
     StringAppendF(&result, "layerName = %s\n", layerName.c_str());
-    packageName = getPackageName(layerName);
     StringAppendF(&result, "packageName = %s\n", packageName.c_str());
     StringAppendF(&result, "statsStart = %lld\n", static_cast<long long int>(statsStart));
     StringAppendF(&result, "statsEnd = %lld\n", static_cast<long long int>(statsEnd));
     StringAppendF(&result, "totalFrames= %d\n", totalFrames);
-    if (deltas.find("present2present") != deltas.end()) {
-        StringAppendF(&result, "averageFPS = %.3f\n",
-                      1000.0 / deltas["present2present"].averageTime());
+    auto iter = deltas.find("present2present");
+    if (iter != deltas.end()) {
+        StringAppendF(&result, "averageFPS = %.3f\n", 1000.0 / iter->second.averageTime());
     }
-    for (auto ele : deltas) {
+    for (auto& ele : deltas) {
         StringAppendF(&result, "%s histogram is as below:\n", ele.first.c_str());
         StringAppendF(&result, "%s", ele.second.toString().c_str());
     }
@@ -103,7 +86,7 @@ std::string TimeStatsHelper::TimeStatsLayer::toString() {
     return result;
 }
 
-std::string TimeStatsHelper::TimeStatsGlobal::toString() {
+std::string TimeStatsHelper::TimeStatsGlobal::toString(std::optional<uint32_t> maxLayers) const {
     std::string result = "SurfaceFlinger TimeStats:\n";
     StringAppendF(&result, "statsStart = %lld\n", static_cast<long long int>(statsStart));
     StringAppendF(&result, "statsEnd = %lld\n", static_cast<long long int>(statsEnd));
@@ -111,25 +94,25 @@ std::string TimeStatsHelper::TimeStatsGlobal::toString() {
     StringAppendF(&result, "missedFrames= %d\n", missedFrames);
     StringAppendF(&result, "clientCompositionFrames= %d\n", clientCompositionFrames);
     StringAppendF(&result, "TimeStats for each layer is as below:\n");
-    for (auto ele : dumpStats) {
+    const auto dumpStats = generateDumpStats(maxLayers);
+    for (auto& ele : dumpStats) {
         StringAppendF(&result, "%s", ele->toString().c_str());
     }
 
     return result;
 }
 
-SFTimeStatsLayerProto TimeStatsHelper::TimeStatsLayer::toProto() {
+SFTimeStatsLayerProto TimeStatsHelper::TimeStatsLayer::toProto() const {
     SFTimeStatsLayerProto layerProto;
     layerProto.set_layer_name(layerName);
-    packageName = getPackageName(layerName);
     layerProto.set_package_name(packageName);
     layerProto.set_stats_start(statsStart);
     layerProto.set_stats_end(statsEnd);
     layerProto.set_total_frames(totalFrames);
-    for (auto ele : deltas) {
+    for (auto& ele : deltas) {
         SFTimeStatsDeltaProto* deltaProto = layerProto.add_deltas();
         deltaProto->set_delta_name(ele.first);
-        for (auto histEle : ele.second.hist) {
+        for (auto& histEle : ele.second.hist) {
             SFTimeStatsHistogramBucketProto* histProto = deltaProto->add_histograms();
             histProto->set_render_millis(histEle.first);
             histProto->set_frame_count(histEle.second);
@@ -138,19 +121,40 @@ SFTimeStatsLayerProto TimeStatsHelper::TimeStatsLayer::toProto() {
     return layerProto;
 }
 
-SFTimeStatsGlobalProto TimeStatsHelper::TimeStatsGlobal::toProto() {
+SFTimeStatsGlobalProto TimeStatsHelper::TimeStatsGlobal::toProto(
+        std::optional<uint32_t> maxLayers) const {
     SFTimeStatsGlobalProto globalProto;
     globalProto.set_stats_start(statsStart);
     globalProto.set_stats_end(statsEnd);
     globalProto.set_total_frames(totalFrames);
     globalProto.set_missed_frames(missedFrames);
     globalProto.set_client_composition_frames(clientCompositionFrames);
-    for (auto ele : dumpStats) {
+    const auto dumpStats = generateDumpStats(maxLayers);
+    for (auto& ele : dumpStats) {
         SFTimeStatsLayerProto* layerProto = globalProto.add_stats();
         layerProto->CopyFrom(ele->toProto());
     }
     return globalProto;
 }
 
+std::vector<TimeStatsHelper::TimeStatsLayer const*>
+TimeStatsHelper::TimeStatsGlobal::generateDumpStats(std::optional<uint32_t> maxLayers) const {
+    std::vector<TimeStatsLayer const*> dumpStats;
+    for (auto& ele : stats) {
+        dumpStats.push_back(&ele.second);
+    }
+
+    std::sort(dumpStats.begin(), dumpStats.end(),
+              [](TimeStatsHelper::TimeStatsLayer const* l,
+                 TimeStatsHelper::TimeStatsLayer const* r) {
+                  return l->totalFrames > r->totalFrames;
+              });
+
+    if (maxLayers && (*maxLayers < dumpStats.size())) {
+        dumpStats.resize(*maxLayers);
+    }
+    return dumpStats;
+}
+
 } // namespace surfaceflinger
 } // namespace android
diff --git a/services/surfaceflinger/TimeStats/timestatsproto/include/timestatsproto/TimeStatsHelper.h b/services/surfaceflinger/TimeStats/timestatsproto/include/timestatsproto/TimeStatsHelper.h
index c876f21404..1798555be5 100644
--- a/services/surfaceflinger/TimeStats/timestatsproto/include/timestatsproto/TimeStatsHelper.h
+++ b/services/surfaceflinger/TimeStats/timestatsproto/include/timestatsproto/TimeStatsHelper.h
@@ -17,9 +17,7 @@
 
 #include <timestatsproto/TimeStatsProtoHeader.h>
 
-#include <math/vec4.h>
-
-#include <memory>
+#include <optional>
 #include <string>
 #include <unordered_map>
 #include <vector>
@@ -36,8 +34,8 @@ public:
         std::unordered_map<int32_t, int32_t> hist;
 
         void insert(int32_t delta);
-        float averageTime();
-        std::string toString();
+        float averageTime() const;
+        std::string toString() const;
     };
 
     class TimeStatsLayer {
@@ -49,8 +47,8 @@ public:
         int32_t totalFrames = 0;
         std::unordered_map<std::string, Histogram> deltas;
 
-        std::string toString();
-        SFTimeStatsLayerProto toProto();
+        std::string toString() const;
+        SFTimeStatsLayerProto toProto() const;
     };
 
     class TimeStatsGlobal {
@@ -61,10 +59,13 @@ public:
         int32_t missedFrames = 0;
         int32_t clientCompositionFrames = 0;
         std::unordered_map<std::string, TimeStatsLayer> stats;
-        std::vector<TimeStatsLayer*> dumpStats;
 
-        std::string toString();
-        SFTimeStatsGlobalProto toProto();
+        std::string toString(std::optional<uint32_t> maxLayers) const;
+        SFTimeStatsGlobalProto toProto(std::optional<uint32_t> maxLayers) const;
+
+    private:
+        std::vector<TimeStatsLayer const*> generateDumpStats(
+                std::optional<uint32_t> maxLayers) const;
     };
 };
 
diff --git a/services/surfaceflinger/TimeStats/timestatsproto/jarjar-rules.txt b/services/surfaceflinger/TimeStats/timestatsproto/jarjar-rules.txt
deleted file mode 100644
index 40043a861c..0000000000
--- a/services/surfaceflinger/TimeStats/timestatsproto/jarjar-rules.txt
+++ /dev/null
@@ -1 +0,0 @@
-rule com.google.protobuf.nano.** com.android.framework.protobuf.nano.@1
diff --git a/services/surfaceflinger/TimeStats/timestatsproto/timestats.proto b/services/surfaceflinger/TimeStats/timestatsproto/timestats.proto
index a8f6fa8ab1..f29fbd187a 100644
--- a/services/surfaceflinger/TimeStats/timestatsproto/timestats.proto
+++ b/services/surfaceflinger/TimeStats/timestatsproto/timestats.proto
@@ -20,10 +20,6 @@ package android.surfaceflinger;
 
 option optimize_for = LITE_RUNTIME;
 
-// frameworks/base/core/proto/android/service/sftimestats.proto is based on
-// this proto. Please only make valid protobuf changes to these messages, and
-// keep the other file in sync with this one.
-
 message SFTimeStatsGlobalProto {
   // The start & end timestamps in UTC as
   // milliseconds since January 1, 1970