commit 552e2a72c96e113f76bb5e536400f9c28f4f6ca5
author Adithya Srinivasan <adsrini@google.com> Fri Jan 15 03:21:53 2021 +0000
committer Adithya Srinivasan <adsrini@google.com> Sat Jan 16 00:40:59 2021 +0000
tree 38e97f0289b68a31e5b1c256e58098fa82c7a65d
parent d26cd28bce5a34a364d55a4ada25ba798dc868aa

Cleanup and send janktype as a bitmask to perfetto

This change sends jankType as a bitmask rather than typed enum, to
perfetto. Also cleans up the TODOs related to this bug.

Bug: 175843808
Test: libsurfaceflinger_unittest
Change-Id: Ib1f7f4a6fdb4c8cd563c26a8d1c3d2ae7b4840db

services/surfaceflinger/FrameTimeline/FrameTimeline.cpp

diff --git a/services/surfaceflinger/FrameTimeline/FrameTimeline.cpp b/services/surfaceflinger/FrameTimeline/FrameTimeline.cpp
index c994434..7b9c012 100644
--- a/services/surfaceflinger/FrameTimeline/FrameTimeline.cpp
+++ b/services/surfaceflinger/FrameTimeline/FrameTimeline.cpp
@@ -95,35 +95,46 @@
 }
 
 std::string jankTypeBitmaskToString(int32_t jankType) {
-    // TODO(b/175843808): Make this a switch case if jankType becomes an enum class
-    std::vector<std::string> janks;
     if (jankType == JankType::None) {
         return "None";
     }
+
+    std::vector<std::string> janks;
     if (jankType & JankType::DisplayHAL) {
         janks.emplace_back("Display HAL");
+        jankType &= ~JankType::DisplayHAL;
     }
     if (jankType & JankType::SurfaceFlingerCpuDeadlineMissed) {
         janks.emplace_back("SurfaceFlinger CPU Deadline Missed");
+        jankType &= ~JankType::SurfaceFlingerCpuDeadlineMissed;
     }
     if (jankType & JankType::SurfaceFlingerGpuDeadlineMissed) {
         janks.emplace_back("SurfaceFlinger GPU Deadline Missed");
+        jankType &= ~JankType::SurfaceFlingerGpuDeadlineMissed;
     }
     if (jankType & JankType::AppDeadlineMissed) {
         janks.emplace_back("App Deadline Missed");
+        jankType &= ~JankType::AppDeadlineMissed;
     }
     if (jankType & JankType::PredictionError) {
         janks.emplace_back("Prediction Error");
+        jankType &= ~JankType::PredictionError;
     }
     if (jankType & JankType::SurfaceFlingerScheduling) {
         janks.emplace_back("SurfaceFlinger Scheduling");
+        jankType &= ~JankType::SurfaceFlingerScheduling;
     }
     if (jankType & JankType::BufferStuffing) {
         janks.emplace_back("Buffer Stuffing");
+        jankType &= ~JankType::BufferStuffing;
     }
     if (jankType & JankType::Unknown) {
         janks.emplace_back("Unknown jank");
+        jankType &= ~JankType::Unknown;
     }
+
+    // jankType should be 0 if all types of jank were checked for.
+    LOG_ALWAYS_FATAL_IF(jankType != 0, "Unrecognized jank type value 0x%x", jankType);
     return std::accumulate(janks.begin(), janks.end(), std::string(),
                            [](const std::string& l, const std::string& r) {
                                return l.empty() ? r : l + ", " + r;
@@ -192,27 +203,48 @@
     }
 }
 
-FrameTimelineEvent::JankType jankTypeBitmaskToProto(int32_t jankType) {
-    // TODO(b/175843808): Either make the proto a bitmask or jankType an enum class
-    switch (jankType) {
-        case JankType::None:
-            return FrameTimelineEvent::JANK_NONE;
-        case JankType::DisplayHAL:
-            return FrameTimelineEvent::JANK_DISPLAY_HAL;
-        case JankType::SurfaceFlingerCpuDeadlineMissed:
-        case JankType::SurfaceFlingerGpuDeadlineMissed:
-            return FrameTimelineEvent::JANK_SF_DEADLINE_MISSED;
-        case JankType::AppDeadlineMissed:
-        case JankType::PredictionError:
-            return FrameTimelineEvent::JANK_APP_DEADLINE_MISSED;
-        case JankType::SurfaceFlingerScheduling:
-            return FrameTimelineEvent::JANK_SF_SCHEDULING;
-        case JankType::BufferStuffing:
-            return FrameTimelineEvent::JANK_BUFFER_STUFFING;
-        default:
-            // TODO(b/175843808): Remove default if JankType becomes an enum class
-            return FrameTimelineEvent::JANK_UNKNOWN;
+int32_t jankTypeBitmaskToProto(int32_t jankType) {
+    if (jankType == JankType::None) {
+        return FrameTimelineEvent::JANK_NONE;
     }
+
+    int32_t protoJank = 0;
+    if (jankType & JankType::DisplayHAL) {
+        protoJank |= FrameTimelineEvent::JANK_DISPLAY_HAL;
+        jankType &= ~JankType::DisplayHAL;
+    }
+    if (jankType & JankType::SurfaceFlingerCpuDeadlineMissed) {
+        protoJank |= FrameTimelineEvent::JANK_SF_CPU_DEADLINE_MISSED;
+        jankType &= ~JankType::SurfaceFlingerCpuDeadlineMissed;
+    }
+    if (jankType & JankType::SurfaceFlingerGpuDeadlineMissed) {
+        protoJank |= FrameTimelineEvent::JANK_SF_GPU_DEADLINE_MISSED;
+        jankType &= ~JankType::SurfaceFlingerGpuDeadlineMissed;
+    }
+    if (jankType & JankType::AppDeadlineMissed) {
+        protoJank |= FrameTimelineEvent::JANK_APP_DEADLINE_MISSED;
+        jankType &= ~JankType::AppDeadlineMissed;
+    }
+    if (jankType & JankType::PredictionError) {
+        protoJank |= FrameTimelineEvent::JANK_PREDICTION_ERROR;
+        jankType &= ~JankType::PredictionError;
+    }
+    if (jankType & JankType::SurfaceFlingerScheduling) {
+        protoJank |= FrameTimelineEvent::JANK_SF_SCHEDULING;
+        jankType &= ~JankType::SurfaceFlingerScheduling;
+    }
+    if (jankType & JankType::BufferStuffing) {
+        protoJank |= FrameTimelineEvent::JANK_BUFFER_STUFFING;
+        jankType &= ~JankType::BufferStuffing;
+    }
+    if (jankType & JankType::Unknown) {
+        protoJank |= FrameTimelineEvent::JANK_UNKNOWN;
+        jankType &= ~JankType::Unknown;
+    }
+
+    // jankType should be 0 if all types of jank were checked for.
+    LOG_ALWAYS_FATAL_IF(jankType != 0, "Unrecognized jank type value 0x%x", jankType);
+    return protoJank;
 }
 
 // Returns the smallest timestamp from the set of predictions and actuals.