Merge "Use unique_ptr for proto parser to handle destruction." into pi-dev

5 files changed, 105 insertions, 39 deletions

diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 7e02922405..a6e0a9ec98 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -3980,7 +3980,7 @@ void SurfaceFlinger::dumpAllLocked(const Vector<String16>& args, size_t& index,
 
     LayersProto layersProto = dumpProtoInfo(LayerVector::StateSet::Current);
     auto layerTree = LayerProtoParser::generateLayerTree(layersProto);
-    result.append(LayerProtoParser::layersToString(layerTree).c_str());
+    result.append(LayerProtoParser::layersToString(std::move(layerTree)).c_str());
 
     /*
      * Dump Display state
diff --git a/services/surfaceflinger/layerproto/LayerProtoParser.cpp b/services/surfaceflinger/layerproto/LayerProtoParser.cpp
index bf37e1e156..c9b7fe55b8 100644
--- a/services/surfaceflinger/layerproto/LayerProtoParser.cpp
+++ b/services/surfaceflinger/layerproto/LayerProtoParser.cpp
@@ -13,7 +13,6 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-
 #include <android-base/stringprintf.h>
 #include <layerproto/LayerProtoParser.h>
 #include <ui/DebugUtils.h>
@@ -24,7 +23,7 @@ using android::base::StringPrintf;
 namespace android {
 namespace surfaceflinger {
 
-bool sortLayers(const LayerProtoParser::Layer* lhs, const LayerProtoParser::Layer* rhs) {
+bool sortLayers(LayerProtoParser::Layer* lhs, const LayerProtoParser::Layer* rhs) {
     uint32_t ls = lhs->layerStack;
     uint32_t rs = rhs->layerStack;
     if (ls != rs) return ls < rs;
@@ -38,20 +37,25 @@ bool sortLayers(const LayerProtoParser::Layer* lhs, const LayerProtoParser::Laye
     return lhs->id < rhs->id;
 }
 
-std::vector<const LayerProtoParser::Layer*> LayerProtoParser::generateLayerTree(
+bool sortLayerUniquePtrs(const std::unique_ptr<LayerProtoParser::Layer>& lhs,
+                   const std::unique_ptr<LayerProtoParser::Layer>& rhs) {
+    return sortLayers(lhs.get(), rhs.get());
+}
+
+std::vector<std::unique_ptr<LayerProtoParser::Layer>> LayerProtoParser::generateLayerTree(
         const LayersProto& layersProto) {
-    auto layerMap = generateMap(layersProto);
-
-    std::vector<const Layer*> layers;
-    std::for_each(layerMap.begin(), layerMap.end(),
-                  [&](const std::pair<const int32_t, Layer*>& ref) {
-                      if (ref.second->parent == nullptr) {
-                          // only save top level layers
-                          layers.push_back(ref.second);
-                      }
-                  });
-
-    std::sort(layers.begin(), layers.end(), sortLayers);
+    std::unordered_map<int32_t, LayerProtoParser::Layer*> layerMap = generateMap(layersProto);
+    std::vector<std::unique_ptr<LayerProtoParser::Layer>> layers;
+
+    for (std::pair<int32_t, Layer*> kv : layerMap) {
+        if (kv.second->parent == nullptr) {
+            // Make unique_ptr for top level layers since they are not children. This ensures there
+            // will only be one unique_ptr made for each layer.
+            layers.push_back(std::unique_ptr<Layer>(kv.second));
+        }
+    }
+
+    std::sort(layers.begin(), layers.end(), sortLayerUniquePtrs);
     return layers;
 }
 
@@ -155,63 +159,61 @@ void LayerProtoParser::updateChildrenAndRelative(const LayerProto& layerProto,
 
     for (int i = 0; i < layerProto.children_size(); i++) {
         if (layerMap.count(layerProto.children(i)) > 0) {
-            auto childLayer = layerMap[layerProto.children(i)];
-            currLayer->children.push_back(childLayer);
+            // Only make unique_ptrs for children since they are guaranteed to be unique, only one
+            // parent per child. This ensures there will only be one unique_ptr made for each layer.
+            currLayer->children.push_back(std::unique_ptr<Layer>(layerMap[layerProto.children(i)]));
         }
     }
 
     for (int i = 0; i < layerProto.relatives_size(); i++) {
         if (layerMap.count(layerProto.relatives(i)) > 0) {
-            auto relativeLayer = layerMap[layerProto.relatives(i)];
-            currLayer->relatives.push_back(relativeLayer);
+            currLayer->relatives.push_back(layerMap[layerProto.relatives(i)]);
         }
     }
 
     if (layerProto.has_parent()) {
         if (layerMap.count(layerProto.parent()) > 0) {
-            auto parentLayer = layerMap[layerProto.parent()];
-            currLayer->parent = parentLayer;
+            currLayer->parent = layerMap[layerProto.parent()];
         }
     }
 
     if (layerProto.has_z_order_relative_of()) {
         if (layerMap.count(layerProto.z_order_relative_of()) > 0) {
-            auto relativeLayer = layerMap[layerProto.z_order_relative_of()];
-            currLayer->zOrderRelativeOf = relativeLayer;
+            currLayer->zOrderRelativeOf = layerMap[layerProto.z_order_relative_of()];
         }
     }
 }
 
 std::string LayerProtoParser::layersToString(
-        const std::vector<const LayerProtoParser::Layer*> layers) {
+        std::vector<std::unique_ptr<LayerProtoParser::Layer>> layers) {
     std::string result;
-    for (const LayerProtoParser::Layer* layer : layers) {
+    for (std::unique_ptr<LayerProtoParser::Layer>& layer : layers) {
         if (layer->zOrderRelativeOf != nullptr) {
             continue;
         }
-        result.append(layerToString(layer).c_str());
+        result.append(layerToString(layer.get()).c_str());
     }
 
     return result;
 }
 
-std::string LayerProtoParser::layerToString(const LayerProtoParser::Layer* layer) {
+std::string LayerProtoParser::layerToString(LayerProtoParser::Layer* layer) {
     std::string result;
 
-    std::vector<const Layer*> traverse(layer->relatives);
-    for (const LayerProtoParser::Layer* child : layer->children) {
+    std::vector<Layer*> traverse(layer->relatives);
+    for (std::unique_ptr<LayerProtoParser::Layer>& child : layer->children) {
         if (child->zOrderRelativeOf != nullptr) {
             continue;
         }
 
-        traverse.push_back(child);
+        traverse.push_back(child.get());
     }
 
     std::sort(traverse.begin(), traverse.end(), sortLayers);
 
     size_t i = 0;
     for (; i < traverse.size(); i++) {
-        const auto& relative = traverse[i];
+        auto& relative = traverse[i];
         if (relative->z >= 0) {
             break;
         }
@@ -220,7 +222,7 @@ std::string LayerProtoParser::layerToString(const LayerProtoParser::Layer* layer
     result.append(layer->to_string().c_str());
     result.append("\n");
     for (; i < traverse.size(); i++) {
-        const auto& relative = traverse[i];
+        auto& relative = traverse[i];
         result.append(layerToString(relative).c_str());
     }
 
diff --git a/services/surfaceflinger/layerproto/include/layerproto/LayerProtoParser.h b/services/surfaceflinger/layerproto/include/layerproto/LayerProtoParser.h
index 78c6cd1345..b81a9595d2 100644
--- a/services/surfaceflinger/layerproto/include/layerproto/LayerProtoParser.h
+++ b/services/surfaceflinger/layerproto/include/layerproto/LayerProtoParser.h
@@ -18,6 +18,7 @@
 
 #include <math/vec4.h>
 
+#include <memory>
 #include <unordered_map>
 #include <vector>
 
@@ -68,8 +69,8 @@ public:
     public:
         int32_t id;
         std::string name;
-        std::vector<const Layer*> children;
-        std::vector<const Layer*> relatives;
+        std::vector<std::unique_ptr<Layer>> children;
+        std::vector<Layer*> relatives;
         std::string type;
         LayerProtoParser::Region transparentRegion;
         LayerProtoParser::Region visibleRegion;
@@ -99,8 +100,8 @@ public:
         std::string to_string() const;
     };
 
-    static std::vector<const Layer*> generateLayerTree(const LayersProto& layersProto);
-    static std::string layersToString(const std::vector<const LayerProtoParser::Layer*> layers);
+    static std::vector<std::unique_ptr<Layer>> generateLayerTree(const LayersProto& layersProto);
+    static std::string layersToString(std::vector<std::unique_ptr<LayerProtoParser::Layer>> layers);
 
 private:
     static std::unordered_map<int32_t, Layer*> generateMap(const LayersProto& layersProto);
@@ -113,7 +114,7 @@ private:
     static void updateChildrenAndRelative(const LayerProto& layerProto,
                                           std::unordered_map<int32_t, Layer*>& layerMap);
 
-    static std::string layerToString(const LayerProtoParser::Layer* layer);
+    static std::string layerToString(LayerProtoParser::Layer* layer);
 };
 
 } // namespace surfaceflinger
diff --git a/services/surfaceflinger/tests/Android.bp b/services/surfaceflinger/tests/Android.bp
index 7d3da329ce..7523399dbf 100644
--- a/services/surfaceflinger/tests/Android.bp
+++ b/services/surfaceflinger/tests/Android.bp
@@ -33,6 +33,7 @@ cc_test {
         "libEGL",
         "libGLESv2",
         "libgui",
+        "liblayers_proto",
         "liblog",
         "libprotobuf-cpp-full",
         "libui",
diff --git a/services/surfaceflinger/tests/Stress_test.cpp b/services/surfaceflinger/tests/Stress_test.cpp
index 33dd2f55d3..4577153df2 100644
--- a/services/surfaceflinger/tests/Stress_test.cpp
+++ b/services/surfaceflinger/tests/Stress_test.cpp
@@ -22,7 +22,7 @@
 
 #include <thread>
 #include <functional>
-
+#include <layerproto/LayerProtoParser.h>
 
 namespace android {
 
@@ -47,4 +47,66 @@ TEST(SurfaceFlingerStress, create_and_destroy) {
     }
 }
 
+surfaceflinger::LayersProto generateLayerProto() {
+    surfaceflinger::LayersProto layersProto;
+    std::array<surfaceflinger::LayerProto*, 10> layers = {};
+    for (size_t i = 0; i < layers.size(); ++i) {
+        layers[i] = layersProto.add_layers();
+        layers[i]->set_id(i);
+    }
+
+    layers[0]->add_children(1);
+    layers[1]->set_parent(0);
+    layers[0]->add_children(2);
+    layers[2]->set_parent(0);
+    layers[0]->add_children(3);
+    layers[3]->set_parent(0);
+    layers[2]->add_children(4);
+    layers[4]->set_parent(2);
+    layers[3]->add_children(5);
+    layers[5]->set_parent(3);
+    layers[5]->add_children(6);
+    layers[6]->set_parent(5);
+    layers[5]->add_children(7);
+    layers[7]->set_parent(5);
+    layers[6]->add_children(8);
+    layers[8]->set_parent(6);
+
+    layers[4]->set_z_order_relative_of(3);
+    layers[3]->add_relatives(4);
+    layers[8]->set_z_order_relative_of(9);
+    layers[9]->add_relatives(8);
+    layers[3]->set_z_order_relative_of(1);
+    layers[1]->add_relatives(3);
+
+/* ----------------------------
+ *       - 0 -      - 9 -
+ *      /  |  \
+ *     1   2   3(1)
+ *         |    |
+ *         4(3) 5
+ *             / \
+ *            6   7
+ *            |
+ *            8(9)
+ * -------------------------- */
+
+    return layersProto;
+}
+
+TEST(LayerProtoStress, mem_info) {
+    std::string cmd = "dumpsys meminfo ";
+    cmd += std::to_string(getpid());
+    system(cmd.c_str());
+    for (int i = 0; i < 100000; i++) {
+        surfaceflinger::LayersProto layersProto = generateLayerProto();
+        auto layerTree = surfaceflinger::LayerProtoParser::generateLayerTree(layersProto);
+        // Allow some layerTrees to just fall out of scope (instead of std::move)
+        if (i % 2) {
+            surfaceflinger::LayerProtoParser::layersToString(std::move(layerTree));
+        }
+    }
+    system(cmd.c_str());
+}
+
 }