Commit 8dd06bbc authored by brettw's avatar brettw Committed by Commit bot

Use flat_set in SurfaceAggregator.

The sets and maps are typically small or empty and created once. flat_set and
flat_map will have better performance for these workloads.

See the bug for histograms of sizes.

Reland of https://codereview.chromium.org/2821353002 with fix.
Reland of https://codereview.chromium.org/2823043002 with fix.

BUG=712295
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
TBR=jbauman@chromium.org

Review-Url: https://codereview.chromium.org/2831663002
Cr-Commit-Position: refs/heads/master@{#465681}
parent 4d1da988
...@@ -144,8 +144,7 @@ int SurfaceAggregator::RemapPassId(int surface_local_pass_id, ...@@ -144,8 +144,7 @@ int SurfaceAggregator::RemapPassId(int surface_local_pass_id,
} }
int SurfaceAggregator::ChildIdForSurface(Surface* surface) { int SurfaceAggregator::ChildIdForSurface(Surface* surface) {
SurfaceToResourceChildIdMap::iterator it = auto it = surface_id_to_resource_child_id_.find(surface->surface_id());
surface_id_to_resource_child_id_.find(surface->surface_id());
if (it == surface_id_to_resource_child_id_.end()) { if (it == surface_id_to_resource_child_id_.end()) {
int child_id = int child_id =
provider_->CreateChild(base::Bind(&UnrefHelper, surface->factory())); provider_->CreateChild(base::Bind(&UnrefHelper, surface->factory()));
...@@ -239,7 +238,7 @@ void SurfaceAggregator::HandleSurfaceQuad( ...@@ -239,7 +238,7 @@ void SurfaceAggregator::HandleSurfaceQuad(
return; return;
} }
SurfaceSet::iterator it = referenced_surfaces_.insert(surface_id).first; referenced_surfaces_.insert(surface_id);
// TODO(vmpstr): provider check is a hack for unittests that don't set up a // TODO(vmpstr): provider check is a hack for unittests that don't set up a
// resource provider. // resource provider.
ResourceProvider::ResourceIdMap empty_map; ResourceProvider::ResourceIdMap empty_map;
...@@ -337,7 +336,8 @@ void SurfaceAggregator::HandleSurfaceQuad( ...@@ -337,7 +336,8 @@ void SurfaceAggregator::HandleSurfaceQuad(
gfx::RectF(surface_quad->rect)); gfx::RectF(surface_quad->rect));
} }
referenced_surfaces_.erase(it); // Need to re-query since referenced_surfaces_ iterators are not stable.
referenced_surfaces_.erase(referenced_surfaces_.find(surface_id));
} }
void SurfaceAggregator::AddColorConversionPass() { void SurfaceAggregator::AddColorConversionPass() {
...@@ -568,8 +568,7 @@ void SurfaceAggregator::ProcessAddedAndRemovedSurfaces() { ...@@ -568,8 +568,7 @@ void SurfaceAggregator::ProcessAddedAndRemovedSurfaces() {
for (const auto& surface : previous_contained_surfaces_) { for (const auto& surface : previous_contained_surfaces_) {
if (!contained_surfaces_.count(surface.first)) { if (!contained_surfaces_.count(surface.first)) {
// Release resources of removed surface. // Release resources of removed surface.
SurfaceToResourceChildIdMap::iterator it = auto it = surface_id_to_resource_child_id_.find(surface.first);
surface_id_to_resource_child_id_.find(surface.first);
if (it != surface_id_to_resource_child_id_.end()) { if (it != surface_id_to_resource_child_id_.end()) {
provider_->DestroyChild(it->second); provider_->DestroyChild(it->second);
surface_id_to_resource_child_id_.erase(it); surface_id_to_resource_child_id_.erase(it);
...@@ -653,13 +652,18 @@ gfx::Rect SurfaceAggregator::PrewalkTree(const SurfaceId& surface_id, ...@@ -653,13 +652,18 @@ gfx::Rect SurfaceAggregator::PrewalkTree(const SurfaceId& surface_id,
}; };
std::vector<SurfaceInfo> child_surfaces; std::vector<SurfaceInfo> child_surfaces;
std::unordered_set<int> pixel_moving_background_filter_passes; // This data is created once and typically small or empty. Collect all items
// and pass to a flat_vector to sort once.
std::vector<int> pixel_moving_background_filter_passes_data;
for (const auto& render_pass : frame.render_pass_list) { for (const auto& render_pass : frame.render_pass_list) {
if (render_pass->background_filters.HasFilterThatMovesPixels()) { if (render_pass->background_filters.HasFilterThatMovesPixels()) {
pixel_moving_background_filter_passes.insert( pixel_moving_background_filter_passes_data.push_back(
RemapPassId(render_pass->id, surface_id)); RemapPassId(render_pass->id, surface_id));
} }
} }
base::flat_set<int> pixel_moving_background_filter_passes(
std::move(pixel_moving_background_filter_passes_data),
base::KEEP_FIRST_OF_DUPES);
for (const auto& render_pass : base::Reversed(frame.render_pass_list)) { for (const auto& render_pass : base::Reversed(frame.render_pass_list)) {
int remapped_pass_id = RemapPassId(render_pass->id, surface_id); int remapped_pass_id = RemapPassId(render_pass->id, surface_id);
...@@ -728,8 +732,7 @@ gfx::Rect SurfaceAggregator::PrewalkTree(const SurfaceId& surface_id, ...@@ -728,8 +732,7 @@ gfx::Rect SurfaceAggregator::PrewalkTree(const SurfaceId& surface_id,
// Avoid infinite recursion by adding current surface to // Avoid infinite recursion by adding current surface to
// referenced_surfaces_. // referenced_surfaces_.
SurfaceSet::iterator it = referenced_surfaces_.insert(surface->surface_id());
referenced_surfaces_.insert(surface->surface_id()).first;
for (const auto& surface_info : child_surfaces) { for (const auto& surface_info : child_surfaces) {
gfx::Rect surface_damage = gfx::Rect surface_damage =
PrewalkTree(surface_info.id, surface_info.has_moved_pixels, PrewalkTree(surface_info.id, surface_info.has_moved_pixels,
...@@ -769,7 +772,7 @@ gfx::Rect SurfaceAggregator::PrewalkTree(const SurfaceId& surface_id, ...@@ -769,7 +772,7 @@ gfx::Rect SurfaceAggregator::PrewalkTree(const SurfaceId& surface_id,
} }
} }
referenced_surfaces_.erase(it); referenced_surfaces_.erase(referenced_surfaces_.find(surface->surface_id()));
if (!damage_rect.IsEmpty() && frame.metadata.may_contain_video) if (!damage_rect.IsEmpty() && frame.metadata.may_contain_video)
result->may_contain_video = true; result->may_contain_video = true;
return damage_rect; return damage_rect;
...@@ -810,7 +813,7 @@ void SurfaceAggregator::CopyUndrawnSurfaces(PrewalkResult* prewalk_result) { ...@@ -810,7 +813,7 @@ void SurfaceAggregator::CopyUndrawnSurfaces(PrewalkResult* prewalk_result) {
} }
} }
} else { } else {
SurfaceSet::iterator it = referenced_surfaces_.insert(surface_id).first; auto it = referenced_surfaces_.insert(surface_id).first;
CopyPasses(frame, surface); CopyPasses(frame, surface);
referenced_surfaces_.erase(it); referenced_surfaces_.erase(it);
} }
...@@ -860,9 +863,10 @@ CompositorFrame SurfaceAggregator::Aggregate(const SurfaceId& surface_id) { ...@@ -860,9 +863,10 @@ CompositorFrame SurfaceAggregator::Aggregate(const SurfaceId& surface_id) {
frame.metadata.may_contain_video = prewalk_result.may_contain_video; frame.metadata.may_contain_video = prewalk_result.may_contain_video;
CopyUndrawnSurfaces(&prewalk_result); CopyUndrawnSurfaces(&prewalk_result);
SurfaceSet::iterator it = referenced_surfaces_.insert(surface_id).first; referenced_surfaces_.insert(surface_id);
CopyPasses(root_surface_frame, surface); CopyPasses(root_surface_frame, surface);
referenced_surfaces_.erase(it); // CopyPasses may have mutated container, need to re-query to erase.
referenced_surfaces_.erase(referenced_surfaces_.find(surface_id));
AddColorConversionPass(); AddColorConversionPass();
moved_pixel_passes_.clear(); moved_pixel_passes_.clear();
...@@ -890,10 +894,8 @@ CompositorFrame SurfaceAggregator::Aggregate(const SurfaceId& surface_id) { ...@@ -890,10 +894,8 @@ CompositorFrame SurfaceAggregator::Aggregate(const SurfaceId& surface_id) {
contained_surfaces_.swap(previous_contained_surfaces_); contained_surfaces_.swap(previous_contained_surfaces_);
contained_surfaces_.clear(); contained_surfaces_.clear();
for (SurfaceIndexMap::iterator it = previous_contained_surfaces_.begin(); for (auto it : previous_contained_surfaces_) {
it != previous_contained_surfaces_.end(); Surface* surface = manager_->GetSurfaceForId(it.first);
++it) {
Surface* surface = manager_->GetSurfaceForId(it->first);
if (surface) if (surface)
surface->TakeLatencyInfo(&frame.metadata.latency_info); surface->TakeLatencyInfo(&frame.metadata.latency_info);
} }
...@@ -914,8 +916,7 @@ CompositorFrame SurfaceAggregator::Aggregate(const SurfaceId& surface_id) { ...@@ -914,8 +916,7 @@ CompositorFrame SurfaceAggregator::Aggregate(const SurfaceId& surface_id) {
} }
void SurfaceAggregator::ReleaseResources(const SurfaceId& surface_id) { void SurfaceAggregator::ReleaseResources(const SurfaceId& surface_id) {
SurfaceToResourceChildIdMap::iterator it = auto it = surface_id_to_resource_child_id_.find(surface_id);
surface_id_to_resource_child_id_.find(surface_id);
if (it != surface_id_to_resource_child_id_.end()) { if (it != surface_id_to_resource_child_id_.end()) {
provider_->DestroyChild(it->second); provider_->DestroyChild(it->second);
surface_id_to_resource_child_id_.erase(it); surface_id_to_resource_child_id_.erase(it);
......
...@@ -5,12 +5,10 @@ ...@@ -5,12 +5,10 @@
#ifndef CC_SURFACES_SURFACE_AGGREGATOR_H_ #ifndef CC_SURFACES_SURFACE_AGGREGATOR_H_
#define CC_SURFACES_SURFACE_AGGREGATOR_H_ #define CC_SURFACES_SURFACE_AGGREGATOR_H_
#include <map>
#include <memory> #include <memory>
#include <set>
#include <unordered_map>
#include <unordered_set>
#include "base/containers/flat_map.h"
#include "base/containers/flat_set.h"
#include "base/macros.h" #include "base/macros.h"
#include "base/memory/weak_ptr.h" #include "base/memory/weak_ptr.h"
#include "cc/quads/draw_quad.h" #include "cc/quads/draw_quad.h"
...@@ -30,7 +28,7 @@ class SurfaceManager; ...@@ -30,7 +28,7 @@ class SurfaceManager;
class CC_SURFACES_EXPORT SurfaceAggregator { class CC_SURFACES_EXPORT SurfaceAggregator {
public: public:
using SurfaceIndexMap = std::unordered_map<SurfaceId, int, SurfaceIdHash>; using SurfaceIndexMap = base::flat_map<SurfaceId, int>;
SurfaceAggregator(SurfaceManager* manager, SurfaceAggregator(SurfaceManager* manager,
ResourceProvider* provider, ResourceProvider* provider,
...@@ -65,7 +63,7 @@ class CC_SURFACES_EXPORT SurfaceAggregator { ...@@ -65,7 +63,7 @@ class CC_SURFACES_EXPORT SurfaceAggregator {
~PrewalkResult(); ~PrewalkResult();
// This is the set of Surfaces that were referenced by another Surface, but // This is the set of Surfaces that were referenced by another Surface, but
// not included in a SurfaceDrawQuad. // not included in a SurfaceDrawQuad.
std::set<SurfaceId> undrawn_surfaces; base::flat_set<SurfaceId> undrawn_surfaces;
bool may_contain_video = false; bool may_contain_video = false;
}; };
...@@ -145,9 +143,8 @@ class CC_SURFACES_EXPORT SurfaceAggregator { ...@@ -145,9 +143,8 @@ class CC_SURFACES_EXPORT SurfaceAggregator {
// each source (SurfaceId, RenderPass id) to a unified ID namespace that's // each source (SurfaceId, RenderPass id) to a unified ID namespace that's
// used in the aggregated frame. An entry is removed from the map if it's not // used in the aggregated frame. An entry is removed from the map if it's not
// used for one output frame. // used for one output frame.
using RenderPassIdAllocatorMap = base::flat_map<std::pair<SurfaceId, int>, RenderPassInfo>
std::map<std::pair<SurfaceId, int>, RenderPassInfo>; render_pass_allocator_map_;
RenderPassIdAllocatorMap render_pass_allocator_map_;
int next_render_pass_id_; int next_render_pass_id_;
const bool aggregate_only_damaged_; const bool aggregate_only_damaged_;
bool output_is_secure_; bool output_is_secure_;
...@@ -162,9 +159,7 @@ class CC_SURFACES_EXPORT SurfaceAggregator { ...@@ -162,9 +159,7 @@ class CC_SURFACES_EXPORT SurfaceAggregator {
// The id for the final color conversion render pass. // The id for the final color conversion render pass.
int color_conversion_render_pass_id_ = 0; int color_conversion_render_pass_id_ = 0;
using SurfaceToResourceChildIdMap = base::flat_map<SurfaceId, int> surface_id_to_resource_child_id_;
std::unordered_map<SurfaceId, int, SurfaceIdHash>;
SurfaceToResourceChildIdMap surface_id_to_resource_child_id_;
// The following state is only valid for the duration of one Aggregate call // The following state is only valid for the duration of one Aggregate call
// and is only stored on the class to avoid having to pass through every // and is only stored on the class to avoid having to pass through every
...@@ -172,8 +167,7 @@ class CC_SURFACES_EXPORT SurfaceAggregator { ...@@ -172,8 +167,7 @@ class CC_SURFACES_EXPORT SurfaceAggregator {
// This is the set of surfaces referenced in the aggregation so far, used to // This is the set of surfaces referenced in the aggregation so far, used to
// detect cycles. // detect cycles.
using SurfaceSet = std::set<SurfaceId>; base::flat_set<SurfaceId> referenced_surfaces_;
SurfaceSet referenced_surfaces_;
// For each Surface used in the last aggregation, gives the frame_index at // For each Surface used in the last aggregation, gives the frame_index at
// that time. // that time.
...@@ -181,22 +175,22 @@ class CC_SURFACES_EXPORT SurfaceAggregator { ...@@ -181,22 +175,22 @@ class CC_SURFACES_EXPORT SurfaceAggregator {
SurfaceIndexMap contained_surfaces_; SurfaceIndexMap contained_surfaces_;
// After surface validation, every Surface in this set is valid. // After surface validation, every Surface in this set is valid.
std::unordered_set<SurfaceId, SurfaceIdHash> valid_surfaces_; base::flat_set<SurfaceId> valid_surfaces_;
// This is the pass list for the aggregated frame. // This is the pass list for the aggregated frame.
RenderPassList* dest_pass_list_; RenderPassList* dest_pass_list_;
// This is the set of aggregated pass ids that are affected by filters that // This is the set of aggregated pass ids that are affected by filters that
// move pixels. // move pixels.
std::unordered_set<int> moved_pixel_passes_; base::flat_set<int> moved_pixel_passes_;
// This is the set of aggregated pass ids that are drawn by copy requests, so // This is the set of aggregated pass ids that are drawn by copy requests, so
// should not have their damage rects clipped to the root damage rect. // should not have their damage rects clipped to the root damage rect.
std::unordered_set<int> copy_request_passes_; base::flat_set<int> copy_request_passes_;
// This maps each aggregated pass id to the set of (aggregated) pass ids // This maps each aggregated pass id to the set of (aggregated) pass ids
// that its RenderPassDrawQuads depend on // that its RenderPassDrawQuads depend on
std::unordered_map<int, std::unordered_set<int>> render_pass_dependencies_; base::flat_map<int, base::flat_set<int>> render_pass_dependencies_;
// The root damage rect of the currently-aggregating frame. // The root damage rect of the currently-aggregating frame.
gfx::Rect root_damage_rect_; gfx::Rect root_damage_rect_;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment