surfaceflinger: simplify memory management for layerproto
Use std::vector<Layer> instead of a tree of std::unique_ptr<Layer>
to manage layers. This significantly reduces the number of
allocations, and more importantly, we can get rid of "new" in
generateLayer. Because there is no "new", it is easier to reason
that there is no leaked layer.
We still keep the pointers to top-level layers in
LayerTree::topLevelLayers. But AFAICT, we probably don't need it
after all.
Test: dumpsys, winscope trace
Change-Id: I302c9115ab025b69e9ea05f4bcd759ed3ee4a8eb
diff --git a/services/surfaceflinger/LayerStats.cpp b/services/surfaceflinger/LayerStats.cpp
index 04ab121..9c2d312 100644
--- a/services/surfaceflinger/LayerStats.cpp
+++ b/services/surfaceflinger/LayerStats.cpp
@@ -57,7 +57,7 @@
}
void LayerStats::traverseLayerTreeStatsLocked(
- const std::vector<std::unique_ptr<LayerProtoParser::Layer>>& layerTree,
+ const std::vector<LayerProtoParser::Layer*>& layerTree,
const LayerProtoParser::LayerGlobal& layerGlobal,
std::vector<std::string>* const outLayerShapeVec) {
for (const auto& layer : layerTree) {
@@ -82,7 +82,7 @@
base::StringAppendF(&key, ",%s",
destinationSize(layer->hwcFrame.bottom - layer->hwcFrame.top,
layerGlobal.resolution[1], false));
- base::StringAppendF(&key, ",%s", scaleRatioWH(layer.get()).c_str());
+ base::StringAppendF(&key, ",%s", scaleRatioWH(layer).c_str());
base::StringAppendF(&key, ",%s", alpha(static_cast<float>(layer->color.a)));
outLayerShapeVec->push_back(key);
@@ -98,7 +98,7 @@
std::vector<std::string> layerShapeVec;
std::lock_guard<std::mutex> lock(mMutex);
- traverseLayerTreeStatsLocked(layerTree, layerGlobal, &layerShapeVec);
+ traverseLayerTreeStatsLocked(layerTree.topLevelLayers, layerGlobal, &layerShapeVec);
std::string layerShapeKey =
base::StringPrintf("%d,%s,%s,%s", static_cast<int32_t>(layerShapeVec.size()),
diff --git a/services/surfaceflinger/LayerStats.h b/services/surfaceflinger/LayerStats.h
index 7a190fd..944073b 100644
--- a/services/surfaceflinger/LayerStats.h
+++ b/services/surfaceflinger/LayerStats.h
@@ -38,7 +38,7 @@
private:
// Traverse layer tree to get all visible layers' stats
void traverseLayerTreeStatsLocked(
- const std::vector<std::unique_ptr<LayerProtoParser::Layer>>& layerTree,
+ const std::vector<LayerProtoParser::Layer*>& layerTree,
const LayerProtoParser::LayerGlobal& layerGlobal,
std::vector<std::string>* const outLayerShapeVec);
// Convert layer's top-left position into 8x8 percentage of the display
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index fa9cc79..eb04563 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -4620,10 +4620,12 @@
mGraphicBufferProducerList.size(), mMaxGraphicBufferProducerListSize);
colorizer.reset(result);
- LayersProto layersProto = dumpProtoInfo(LayerVector::StateSet::Current);
- auto layerTree = LayerProtoParser::generateLayerTree(layersProto);
- result.append(LayerProtoParser::layersToString(std::move(layerTree)).c_str());
- result.append("\n");
+ {
+ LayersProto layersProto = dumpProtoInfo(LayerVector::StateSet::Current);
+ auto layerTree = LayerProtoParser::generateLayerTree(layersProto);
+ result.append(LayerProtoParser::layerTreeToString(layerTree).c_str());
+ result.append("\n");
+ }
result.append("\nFrame-Composition information:\n");
dumpFrameCompositionInfo(result);
diff --git a/services/surfaceflinger/layerproto/LayerProtoParser.cpp b/services/surfaceflinger/layerproto/LayerProtoParser.cpp
index bf12df6..1d7fb67 100644
--- a/services/surfaceflinger/layerproto/LayerProtoParser.cpp
+++ b/services/surfaceflinger/layerproto/LayerProtoParser.cpp
@@ -37,11 +37,6 @@
return lhs->id < rhs->id;
}
-bool sortLayerUniquePtrs(const std::unique_ptr<LayerProtoParser::Layer>& lhs,
- const std::unique_ptr<LayerProtoParser::Layer>& rhs) {
- return sortLayers(lhs.get(), rhs.get());
-}
-
const LayerProtoParser::LayerGlobal LayerProtoParser::generateLayerGlobalInfo(
const LayersProto& layersProto) {
LayerGlobal layerGlobal;
@@ -52,77 +47,80 @@
return layerGlobal;
}
-std::vector<std::unique_ptr<LayerProtoParser::Layer>> LayerProtoParser::generateLayerTree(
- const LayersProto& layersProto) {
- std::unordered_map<int32_t, LayerProtoParser::Layer*> layerMap = generateMap(layersProto);
- std::vector<std::unique_ptr<LayerProtoParser::Layer>> layers;
+LayerProtoParser::LayerTree LayerProtoParser::generateLayerTree(const LayersProto& layersProto) {
+ LayerTree layerTree;
+ layerTree.allLayers = generateLayerList(layersProto);
- 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));
+ // find and sort the top-level layers
+ for (Layer& layer : layerTree.allLayers) {
+ if (layer.parent == nullptr) {
+ layerTree.topLevelLayers.push_back(&layer);
}
}
+ std::sort(layerTree.topLevelLayers.begin(), layerTree.topLevelLayers.end(), sortLayers);
- std::sort(layers.begin(), layers.end(), sortLayerUniquePtrs);
- return layers;
+ return layerTree;
}
-std::unordered_map<int32_t, LayerProtoParser::Layer*> LayerProtoParser::generateMap(
+std::vector<LayerProtoParser::Layer> LayerProtoParser::generateLayerList(
const LayersProto& layersProto) {
+ std::vector<Layer> layerList;
std::unordered_map<int32_t, Layer*> layerMap;
+ // build the layer list and the layer map
+ layerList.reserve(layersProto.layers_size());
+ layerMap.reserve(layersProto.layers_size());
for (int i = 0; i < layersProto.layers_size(); i++) {
- const LayerProto& layerProto = layersProto.layers(i);
- layerMap[layerProto.id()] = generateLayer(layerProto);
+ layerList.emplace_back(generateLayer(layersProto.layers(i)));
+ // this works because layerList never changes capacity
+ layerMap[layerList.back().id] = &layerList.back();
}
+ // fix up children and relatives
for (int i = 0; i < layersProto.layers_size(); i++) {
- const LayerProto& layerProto = layersProto.layers(i);
- updateChildrenAndRelative(layerProto, layerMap);
+ updateChildrenAndRelative(layersProto.layers(i), layerMap);
}
- return layerMap;
+ return layerList;
}
-LayerProtoParser::Layer* LayerProtoParser::generateLayer(const LayerProto& layerProto) {
- Layer* layer = new Layer();
- layer->id = layerProto.id();
- layer->name = layerProto.name();
- layer->type = layerProto.type();
- layer->transparentRegion = generateRegion(layerProto.transparent_region());
- layer->visibleRegion = generateRegion(layerProto.visible_region());
- layer->damageRegion = generateRegion(layerProto.damage_region());
- layer->layerStack = layerProto.layer_stack();
- layer->z = layerProto.z();
- layer->position = {layerProto.position().x(), layerProto.position().y()};
- layer->requestedPosition = {layerProto.requested_position().x(),
+LayerProtoParser::Layer LayerProtoParser::generateLayer(const LayerProto& layerProto) {
+ Layer layer;
+ layer.id = layerProto.id();
+ layer.name = layerProto.name();
+ layer.type = layerProto.type();
+ layer.transparentRegion = generateRegion(layerProto.transparent_region());
+ layer.visibleRegion = generateRegion(layerProto.visible_region());
+ layer.damageRegion = generateRegion(layerProto.damage_region());
+ layer.layerStack = layerProto.layer_stack();
+ layer.z = layerProto.z();
+ layer.position = {layerProto.position().x(), layerProto.position().y()};
+ layer.requestedPosition = {layerProto.requested_position().x(),
layerProto.requested_position().y()};
- layer->size = {layerProto.size().w(), layerProto.size().h()};
- layer->crop = generateRect(layerProto.crop());
- layer->isOpaque = layerProto.is_opaque();
- layer->invalidate = layerProto.invalidate();
- layer->dataspace = layerProto.dataspace();
- layer->pixelFormat = layerProto.pixel_format();
- layer->color = {layerProto.color().r(), layerProto.color().g(), layerProto.color().b(),
+ layer.size = {layerProto.size().w(), layerProto.size().h()};
+ layer.crop = generateRect(layerProto.crop());
+ layer.isOpaque = layerProto.is_opaque();
+ layer.invalidate = layerProto.invalidate();
+ layer.dataspace = layerProto.dataspace();
+ layer.pixelFormat = layerProto.pixel_format();
+ layer.color = {layerProto.color().r(), layerProto.color().g(), layerProto.color().b(),
layerProto.color().a()};
- layer->requestedColor = {layerProto.requested_color().r(), layerProto.requested_color().g(),
+ layer.requestedColor = {layerProto.requested_color().r(), layerProto.requested_color().g(),
layerProto.requested_color().b(), layerProto.requested_color().a()};
- layer->flags = layerProto.flags();
- layer->transform = generateTransform(layerProto.transform());
- layer->requestedTransform = generateTransform(layerProto.requested_transform());
- layer->activeBuffer = generateActiveBuffer(layerProto.active_buffer());
- layer->bufferTransform = generateTransform(layerProto.buffer_transform());
- layer->queuedFrames = layerProto.queued_frames();
- layer->refreshPending = layerProto.refresh_pending();
- layer->hwcFrame = generateRect(layerProto.hwc_frame());
- layer->hwcCrop = generateFloatRect(layerProto.hwc_crop());
- layer->hwcTransform = layerProto.hwc_transform();
- layer->windowType = layerProto.window_type();
- layer->appId = layerProto.app_id();
- layer->hwcCompositionType = layerProto.hwc_composition_type();
- layer->isProtected = layerProto.is_protected();
+ layer.flags = layerProto.flags();
+ layer.transform = generateTransform(layerProto.transform());
+ layer.requestedTransform = generateTransform(layerProto.requested_transform());
+ layer.activeBuffer = generateActiveBuffer(layerProto.active_buffer());
+ layer.bufferTransform = generateTransform(layerProto.buffer_transform());
+ layer.queuedFrames = layerProto.queued_frames();
+ layer.refreshPending = layerProto.refresh_pending();
+ layer.hwcFrame = generateRect(layerProto.hwc_frame());
+ layer.hwcCrop = generateFloatRect(layerProto.hwc_crop());
+ layer.hwcTransform = layerProto.hwc_transform();
+ layer.windowType = layerProto.window_type();
+ layer.appId = layerProto.app_id();
+ layer.hwcCompositionType = layerProto.hwc_composition_type();
+ layer.isProtected = layerProto.is_protected();
return layer;
}
@@ -186,9 +184,7 @@
for (int i = 0; i < layerProto.children_size(); i++) {
if (layerMap.count(layerProto.children(i)) > 0) {
- // 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)]));
+ currLayer->children.push_back(layerMap[layerProto.children(i)]);
}
}
@@ -211,29 +207,28 @@
}
}
-std::string LayerProtoParser::layersToString(
- std::vector<std::unique_ptr<LayerProtoParser::Layer>> layers) {
+std::string LayerProtoParser::layerTreeToString(const LayerTree& layerTree) {
std::string result;
- for (std::unique_ptr<LayerProtoParser::Layer>& layer : layers) {
+ for (const LayerProtoParser::Layer* layer : layerTree.topLevelLayers) {
if (layer->zOrderRelativeOf != nullptr) {
continue;
}
- result.append(layerToString(layer.get()).c_str());
+ result.append(layerToString(layer));
}
return result;
}
-std::string LayerProtoParser::layerToString(LayerProtoParser::Layer* layer) {
+std::string LayerProtoParser::layerToString(const LayerProtoParser::Layer* layer) {
std::string result;
std::vector<Layer*> traverse(layer->relatives);
- for (std::unique_ptr<LayerProtoParser::Layer>& child : layer->children) {
+ for (LayerProtoParser::Layer* child : layer->children) {
if (child->zOrderRelativeOf != nullptr) {
continue;
}
- traverse.push_back(child.get());
+ traverse.push_back(child);
}
std::sort(traverse.begin(), traverse.end(), sortLayers);
@@ -244,13 +239,13 @@
if (relative->z >= 0) {
break;
}
- result.append(layerToString(relative).c_str());
+ result.append(layerToString(relative));
}
- result.append(layer->to_string().c_str());
+ result.append(layer->to_string());
result.append("\n");
for (; i < traverse.size(); i++) {
auto& relative = traverse[i];
- result.append(layerToString(relative).c_str());
+ result.append(layerToString(relative));
}
return result;
diff --git a/services/surfaceflinger/layerproto/include/layerproto/LayerProtoParser.h b/services/surfaceflinger/layerproto/include/layerproto/LayerProtoParser.h
index b1610cf..6b3b497 100644
--- a/services/surfaceflinger/layerproto/include/layerproto/LayerProtoParser.h
+++ b/services/surfaceflinger/layerproto/include/layerproto/LayerProtoParser.h
@@ -80,7 +80,7 @@
public:
int32_t id;
std::string name;
- std::vector<std::unique_ptr<Layer>> children;
+ std::vector<Layer*> children;
std::vector<Layer*> relatives;
std::string type;
LayerProtoParser::Region transparentRegion;
@@ -126,13 +126,22 @@
int32_t globalTransform;
};
+ class LayerTree {
+ public:
+ // all layers in LayersProto and in the original order
+ std::vector<Layer> allLayers;
+
+ // pointers to top-level layers in allLayers
+ std::vector<Layer*> topLevelLayers;
+ };
+
static const LayerGlobal generateLayerGlobalInfo(const LayersProto& layersProto);
- static std::vector<std::unique_ptr<Layer>> generateLayerTree(const LayersProto& layersProto);
- static std::string layersToString(std::vector<std::unique_ptr<LayerProtoParser::Layer>> layers);
+ static LayerTree generateLayerTree(const LayersProto& layersProto);
+ static std::string layerTreeToString(const LayerTree& layerTree);
private:
- static std::unordered_map<int32_t, Layer*> generateMap(const LayersProto& layersProto);
- static LayerProtoParser::Layer* generateLayer(const LayerProto& layerProto);
+ static std::vector<Layer> generateLayerList(const LayersProto& layersProto);
+ static LayerProtoParser::Layer generateLayer(const LayerProto& layerProto);
static LayerProtoParser::Region generateRegion(const RegionProto& regionProto);
static LayerProtoParser::Rect generateRect(const RectProto& rectProto);
static LayerProtoParser::FloatRect generateFloatRect(const FloatRectProto& rectProto);
@@ -142,7 +151,7 @@
static void updateChildrenAndRelative(const LayerProto& layerProto,
std::unordered_map<int32_t, Layer*>& layerMap);
- static std::string layerToString(LayerProtoParser::Layer* layer);
+ static std::string layerToString(const LayerProtoParser::Layer* layer);
};
} // namespace surfaceflinger
diff --git a/services/surfaceflinger/tests/Stress_test.cpp b/services/surfaceflinger/tests/Stress_test.cpp
index 4577153..3e1be8e 100644
--- a/services/surfaceflinger/tests/Stress_test.cpp
+++ b/services/surfaceflinger/tests/Stress_test.cpp
@@ -101,10 +101,7 @@
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));
- }
+ surfaceflinger::LayerProtoParser::layerTreeToString(layerTree);
}
system(cmd.c_str());
}