cc: Remove tile ref counting in tile manager.

cc/resources/tile_manager.cc

Index: cc/resources/tile_manager.cc
diff --git a/cc/resources/tile_manager.cc b/cc/resources/tile_manager.cc
index 54e5ed9243db97963d72d2881b90ee2f036ed5b8..a9a6217b4bb82b90547631987fac262258e3b9b7 100644
--- a/cc/resources/tile_manager.cc
+++ b/cc/resources/tile_manager.cc
@@ -123,6 +123,8 @@ TileManager::TileManager(
     : client_(client),
       resource_pool_(ResourcePool::Create(resource_provider)),
       raster_worker_pool_(raster_worker_pool.Pass()),
+      sorted_tiles_need_update_(false),
+      all_tiles_required_for_activation_have_memory_(true),
       ever_exceeded_memory_budget_(false),
       rendering_stats_instrumentation_(rendering_stats_instrumentation),
       did_initialize_visible_tile_(false),
@@ -134,13 +136,21 @@ TileManager::~TileManager() {
   // Reset global state and manage. This should cause
   // our memory usage to drop to zero.
   global_state_ = GlobalStateThatImpactsTilePriority();
-  AssignGpuMemoryToTiles();
+  DCHECK(tiles_.empty());

[reveman, 2013/07/04 00:46:11]

DCHECK_EQ(0u, tiles_.size()) is preferred

[vmpstr, 2013/07/08 17:28:30]

Done.

+
+  sorted_tiles_need_update_ = true;
+  UpdateSortedTilesIfNeeded();
+  TileVector tiles_that_need_to_be_rasterized;
+  TileSet oom_tiles_required_for_activation;
+  AssignGpuMemoryToTiles(&tiles_that_need_to_be_rasterized,
+                         &oom_tiles_required_for_activation);
+  ScheduleTasks(tiles_that_need_to_be_rasterized);

[reveman, 2013/07/04 00:46:11]

I don't think we need to exercise all this code here. how about just doing:
sorted_tiles_.clear();
TileVector empty;
ScheduleTasks(empty);

[vmpstr, 2013/07/08 17:28:30]

Done.

+
   CleanUpUnusedImageDecodeTasks();
   // This should finish all pending tasks and release any uninitialized
   // resources.
   raster_worker_pool_->Shutdown();
   raster_worker_pool_->CheckForCompletedTasks();
-  DCHECK_EQ(0u, tiles_.size());
 }
 
 void TileManager::SetGlobalState(
@@ -152,32 +162,42 @@ void TileManager::SetGlobalState(
 }
 
 void TileManager::RegisterTile(Tile* tile) {
-  DCHECK(std::find(tiles_.begin(), tiles_.end(), tile) == tiles_.end());
   DCHECK(!tile->required_for_activation());
-  tiles_.push_back(tile);
+  DCHECK(tiles_.find(tile->id()) == tiles_.end());
+
+  tiles_[tile->id()] = tile;
+  sorted_tiles_need_update_ = true;
 }
 
 void TileManager::UnregisterTile(Tile* tile) {
-  TileVector::iterator raster_iter =
-      std::find(tiles_that_need_to_be_rasterized_.begin(),
-                tiles_that_need_to_be_rasterized_.end(),
-                tile);
-  if (raster_iter != tiles_that_need_to_be_rasterized_.end())
-    tiles_that_need_to_be_rasterized_.erase(raster_iter);
-
   tiles_that_need_to_be_initialized_for_activation_.erase(tile);
-  oom_tiles_that_need_to_be_initialized_for_activation_.erase(tile);
 
-  DCHECK(std::find(tiles_.begin(), tiles_.end(), tile) != tiles_.end());
   FreeResourcesForTile(tile);
-  tiles_.erase(std::remove(tiles_.begin(), tiles_.end(), tile));
+
+  DCHECK(tiles_.find(tile->id()) != tiles_.end());
+  tiles_.erase(tile->id());
+
+  sorted_tiles_need_update_ = true;
 }
 
 bool TileManager::ShouldForceTasksRequiredForActivationToComplete() const {
   return GlobalState().tree_priority != SMOOTHNESS_TAKES_PRIORITY;
 }
 
+void TileManager::UpdateSortedTilesIfNeeded() {
+  TRACE_EVENT0("cc", "TileManager::UpdateSortedTilesIfNeeded");
+
+  if (!sorted_tiles_need_update_)
+    return;
+
+  sorted_tiles_.clear();
+  AssignBinsToTiles(&sorted_tiles_);
+  SortTiles(&sorted_tiles_);

[reveman, 2013/07/04 00:46:11]

I would prefer building the vector here and passing it to AssignBinsToTiles
instead of having that function build it.

It might also be a bit cleaner to use a new TileVector and
sorted_tiles_.swap(new_tiles) after calling SortTiles().

[vmpstr, 2013/07/08 17:28:30]

Done.

+}
+
 void TileManager::DidFinishedRunningTasks() {
+  TRACE_EVENT0("cc", "TileManager::DidFinishedRunningTasks");
+
   // When OOM, keep re-assigning memory until we reach a steady state
   // where top-priority tiles are initialized.
   if (!memory_stats_from_last_assign_.bytes_over)
@@ -185,29 +205,37 @@ void TileManager::DidFinishedRunningTasks() {
 
   raster_worker_pool_->CheckForCompletedTasks();
 
-  AssignGpuMemoryToTiles();
+  UpdateSortedTilesIfNeeded();
 
-  if (!oom_tiles_that_need_to_be_initialized_for_activation_.empty())
-    ReassignGpuMemoryToOOMTilesRequiredForActivation();
+  TileVector tiles_that_need_to_be_rasterized;
+  TileSet oom_tiles_required_for_activation;
+  AssignGpuMemoryToTiles(&tiles_that_need_to_be_rasterized,
+                         &oom_tiles_required_for_activation);
 
-  // |tiles_that_need_to_be_rasterized_| will be empty when we reach a
+  if (!oom_tiles_required_for_activation.empty()) {
+    ReassignGpuMemoryToOOMTilesRequiredForActivation(
+        &tiles_that_need_to_be_rasterized,
+        &oom_tiles_required_for_activation);
+  }
+  all_tiles_required_for_activation_have_memory_ =
+      oom_tiles_required_for_activation.empty();
+
+  // |tiles_that_need_to_be_rasterized| will be empty when we reach a
   // steady memory state. Keep scheduling tasks until we reach this state.
-  if (!tiles_that_need_to_be_rasterized_.empty()) {
-    ScheduleTasks();
+  if (!tiles_that_need_to_be_rasterized.empty()) {
+    ScheduleTasks(tiles_that_need_to_be_rasterized);
     return;
   }
 
   // Use on-demand raster for any tiles that have not been been assigned
   // memory after reaching a steady memory state.
-  for (TileSet::iterator it =
-           oom_tiles_that_need_to_be_initialized_for_activation_.begin();
-       it != oom_tiles_that_need_to_be_initialized_for_activation_.end();
+  for (TileSet::iterator it = oom_tiles_required_for_activation.begin();
+       it != oom_tiles_required_for_activation.end();
        ++it) {
     Tile* tile = *it;
     ManagedTileState& mts = tile->managed_state();
     mts.tile_versions[mts.raster_mode].set_rasterize_on_demand();
   }
-  oom_tiles_that_need_to_be_initialized_for_activation_.clear();
 
   DCHECK_EQ(0u, tiles_that_need_to_be_initialized_for_activation_.size());
   client_->NotifyReadyToActivate();
@@ -219,7 +247,7 @@ void TileManager::DidFinishedRunningTasksRequiredForActivation() {
   // wait for DidFinishRunningTasks() to be called, try to re-assign
   // memory and in worst case use on-demand raster when tiles
   // required for activation are OOM.
-  if (!oom_tiles_that_need_to_be_initialized_for_activation_.empty())
+  if (!all_tiles_required_for_activation_have_memory_)
     return;
 
   client_->NotifyReadyToActivate();
@@ -259,7 +287,7 @@ class BinComparator {
   }
 };
 
-void TileManager::AssignBinsToTiles() {
+void TileManager::AssignBinsToTiles(TileVector* tiles) {
   const TreePriority tree_priority = global_state_.tree_priority;
 
   // Memory limit policy works by mapping some bin states to the NEVER bin.
@@ -286,11 +314,14 @@ void TileManager::AssignBinsToTiles() {
     bin_map[NEVER_BIN] = NEVER_BIN;
   }
 
+  DCHECK(tiles->empty());
+  tiles->reserve(tiles_.size());
+
   // For each tree, bin into different categories of tiles.
-  for (TileVector::iterator it = tiles_.begin();
+  for (TileMap::iterator it = tiles_.begin();
        it != tiles_.end();
        ++it) {
-    Tile* tile = *it;
+    Tile* tile = it->second;
     ManagedTileState& mts = tile->managed_state();
 
     TilePriority prio[NUM_BIN_PRIORITIES];
@@ -334,21 +365,33 @@ void TileManager::AssignBinsToTiles() {
 
     for (int i = 0; i < NUM_BIN_PRIORITIES; ++i)
       mts.bin[i] = bin_map[mts.bin[i]];
+
+    tiles->push_back(tile);
   }
 }
 
-void TileManager::SortTiles() {
+void TileManager::SortTiles(TileVector* tiles) {
   TRACE_EVENT0("cc", "TileManager::SortTiles");
 
   // Sort by bin, resolution and time until needed.
-  std::sort(tiles_.begin(), tiles_.end(), BinComparator());
+  std::sort(tiles->begin(), tiles->end(), BinComparator());
 }
 
 void TileManager::ManageTiles() {
   TRACE_EVENT0("cc", "TileManager::ManageTiles");
-  AssignBinsToTiles();
-  SortTiles();
-  AssignGpuMemoryToTiles();
+
+  // Since priorities might have changed, we need to force
+  // the sorted list to be updated.
+  sorted_tiles_need_update_ = true;
+
+  UpdateSortedTilesIfNeeded();
+
+  TileVector tiles_that_need_to_be_rasterized;
+  TileSet oom_tiles_required_for_activation;
+  AssignGpuMemoryToTiles(&tiles_that_need_to_be_rasterized,
+                         &oom_tiles_required_for_activation);
+  all_tiles_required_for_activation_have_memory_ =
+      oom_tiles_required_for_activation.empty();
   CleanUpUnusedImageDecodeTasks();
 
   TRACE_EVENT_INSTANT1(
@@ -356,7 +399,7 @@ void TileManager::ManageTiles() {
       "state", TracedValue::FromValue(BasicStateAsValue().release()));
 
   // Finally, schedule rasterizer tasks.
-  ScheduleTasks();
+  ScheduleTasks(tiles_that_need_to_be_rasterized);
 }
 
 void TileManager::CheckForCompletedTileUploads() {
@@ -375,10 +418,10 @@ void TileManager::GetMemoryStats(
   *memory_required_bytes = 0;
   *memory_nice_to_have_bytes = 0;
   *memory_used_bytes = resource_pool_->acquired_memory_usage_bytes();
-  for (TileVector::const_iterator it = tiles_.begin();
+  for (TileMap::const_iterator it = tiles_.begin();
        it != tiles_.end();
        ++it) {
-    const Tile* tile = *it;
+    const Tile* tile = it->second;
     const ManagedTileState& mts = tile->managed_state();
 
     RasterMode mode = HIGH_QUALITY_NO_LCD_RASTER_MODE;
@@ -404,10 +447,10 @@ scoped_ptr<base::Value> TileManager::BasicStateAsValue() const {
 
 scoped_ptr<base::Value> TileManager::AllTilesAsValue() const {
   scoped_ptr<base::ListValue> state(new base::ListValue());
-  for (TileVector::const_iterator it = tiles_.begin();
+  for (TileMap::const_iterator it = tiles_.begin();
        it != tiles_.end();
        it++) {
-    state->Append((*it)->AsValue().release());
+    state->Append(it->second->AsValue().release());
   }
   return state.PassAs<base::Value>();
 }
@@ -456,18 +499,18 @@ RasterMode TileManager::DetermineRasterMode(const Tile* tile) const {
   return std::min(raster_mode, current_mode);
 }
 
-void TileManager::AssignGpuMemoryToTiles() {
+void TileManager::AssignGpuMemoryToTiles(

[reveman, 2013/07/04 00:46:11]

Maybe pass sorted tiles as a parameter to this function instead of having this
function assume it's valid.

[vmpstr, 2013/07/08 17:28:30]

Done.

+    TileVector* tiles_that_need_to_be_rasterized,
+    TileSet* oom_tiles_required_for_activation) {
   TRACE_EVENT0("cc", "TileManager::AssignGpuMemoryToTiles");
 
   // Now give memory out to the tiles until we're out, and build
   // the needs-to-be-rasterized queue.
-  tiles_that_need_to_be_rasterized_.clear();
   tiles_that_need_to_be_initialized_for_activation_.clear();
-  oom_tiles_that_need_to_be_initialized_for_activation_.clear();
 
   size_t bytes_releasable = 0;
-  for (TileVector::const_iterator it = tiles_.begin();
-       it != tiles_.end();
+  for (TileVector::const_iterator it = sorted_tiles_.begin();
+       it != sorted_tiles_.end();
        ++it) {
     const Tile* tile = *it;
     const ManagedTileState& mts = tile->managed_state();
@@ -490,8 +533,8 @@ void TileManager::AssignGpuMemoryToTiles() {
   size_t bytes_left = bytes_allocatable;
   size_t bytes_oom_tiles_that_need_to_be_initialized_for_activation = 0;
   bool higher_priority_tile_oomed = false;
-  for (TileVector::iterator it = tiles_.begin();
-       it != tiles_.end();
+  for (TileVector::iterator it = sorted_tiles_.begin();
+       it != sorted_tiles_.end();
        ++it) {
     Tile* tile = *it;
     ManagedTileState& mts = tile->managed_state();
@@ -532,7 +575,7 @@ void TileManager::AssignGpuMemoryToTiles() {
         // memory limit.
         if (bytes_oom_tiles_that_need_to_be_initialized_for_activation <
             global_state_.memory_limit_in_bytes) {
-          oom_tiles_that_need_to_be_initialized_for_activation_.insert(tile);
+          oom_tiles_required_for_activation->insert(tile);
           bytes_oom_tiles_that_need_to_be_initialized_for_activation +=
               tile_bytes;
         } else {
@@ -558,7 +601,7 @@ void TileManager::AssignGpuMemoryToTiles() {
       continue;
 
     if (!tile_version.resource_)
-      tiles_that_need_to_be_rasterized_.push_back(tile);
+      tiles_that_need_to_be_rasterized->push_back(tile);
 
     if (!tile->IsReadyToDraw(NULL) &&
         tile->required_for_activation()) {
@@ -582,16 +625,20 @@ void TileManager::AssignGpuMemoryToTiles() {
       bytes_that_exceeded_memory_budget;
 }
 
-void TileManager::ReassignGpuMemoryToOOMTilesRequiredForActivation() {
+void TileManager::ReassignGpuMemoryToOOMTilesRequiredForActivation(

[reveman, 2013/07/04 00:46:11]

Same thing here. Maybe pass sorted tiles as a parameter.

[vmpstr, 2013/07/08 17:28:30]

Done.

+    TileVector* tiles_that_need_to_be_rasterized,
+    TileSet* oom_tiles_required_for_activation) {
   TRACE_EVENT0(
       "cc", "TileManager::ReassignGpuMemoryToOOMTilesRequiredForActivation");
 
   size_t bytes_oom_for_required_tiles = 0;
   TileVector tiles_requiring_memory_but_oomed;
-  for (TileVector::iterator it = tiles_.begin(); it != tiles_.end(); ++it) {
+  for (TileVector::iterator it = sorted_tiles_.begin();
+       it != sorted_tiles_.end();
+       ++it) {
     Tile* tile = *it;
-    if (oom_tiles_that_need_to_be_initialized_for_activation_.find(tile) ==
-        oom_tiles_that_need_to_be_initialized_for_activation_.end())
+    if (oom_tiles_required_for_activation->find(tile) ==
+        oom_tiles_required_for_activation->end())
       continue;
 
     tiles_requiring_memory_but_oomed.push_back(tile);
@@ -601,11 +648,12 @@ void TileManager::ReassignGpuMemoryToOOMTilesRequiredForActivation() {
   if (tiles_requiring_memory_but_oomed.empty())
     return;
 
-  // In OOM situation, we iterate tiles_, remove the memory for active tree
-  // and not the now bin. And give them to bytes_oom_for_required_tiles
+  // In OOM situation, we iterate sorted_tiles_, remove the memory for active
+  // tree and not the now bin. And give them to bytes_oom_for_required_tiles
   size_t bytes_freed = 0;
-  for (TileVector::reverse_iterator it = tiles_.rbegin();
-       it != tiles_.rend(); ++it) {
+  for (TileVector::reverse_iterator it = sorted_tiles_.rbegin();
+       it != sorted_tiles_.rend();
+       ++it) {
     Tile* tile = *it;
     ManagedTileState& mts = tile->managed_state();
     if (mts.tree_bin[PENDING_TREE] == NEVER_BIN &&
@@ -616,13 +664,13 @@ void TileManager::ReassignGpuMemoryToOOMTilesRequiredForActivation() {
       // If the tile is in the to-rasterize list, but it has no task,
       // then it means that we have assigned memory for it.
       TileVector::iterator raster_it =
-          std::find(tiles_that_need_to_be_rasterized_.begin(),
-                    tiles_that_need_to_be_rasterized_.end(),
+          std::find(tiles_that_need_to_be_rasterized->begin(),
+                    tiles_that_need_to_be_rasterized->end(),
                     tile);
-      if (raster_it != tiles_that_need_to_be_rasterized_.end() &&
+      if (raster_it != tiles_that_need_to_be_rasterized->end() &&
           tile_version.raster_task_.is_null()) {
         bytes_freed += tile->bytes_consumed_if_allocated();
-        tiles_that_need_to_be_rasterized_.erase(raster_it);
+        tiles_that_need_to_be_rasterized->erase(raster_it);
       }
 
       // Also consider all of the completed resources for freeing.
@@ -649,18 +697,18 @@ void TileManager::ReassignGpuMemoryToOOMTilesRequiredForActivation() {
       continue;
     mts.tile_versions[mts.raster_mode].set_use_resource();
     bytes_freed -= bytes_needed;
-    tiles_that_need_to_be_rasterized_.push_back(tile);
+    tiles_that_need_to_be_rasterized->push_back(tile);
     DCHECK(tile->required_for_activation());
     AddRequiredTileForActivation(tile);
-    oom_tiles_that_need_to_be_initialized_for_activation_.erase(tile);
+    oom_tiles_required_for_activation->erase(tile);
   }
 }
 
 void TileManager::CleanUpUnusedImageDecodeTasks() {
   // Calculate a set of layers that are used by at least one tile.
   base::hash_set<int> used_layers;
-  for (TileVector::iterator it = tiles_.begin(); it != tiles_.end(); ++it)
-    used_layers.insert((*it)->layer_id());
+  for (TileMap::iterator it = tiles_.begin(); it != tiles_.end(); ++it)

[reveman, 2013/07/04 00:46:11]

maybe use sorted tiles here as that will likely be more efficient to iterate
over.

[vmpstr, 2013/07/08 17:28:30]

I prefer to keep it as is, since tiles_ is the authoritative source of what
tiles exist. If we use sorted tiles here, then the call site has to ensure that
it's up to date (after possible removal/addition of new tiles)

+    used_layers.insert(it->second->layer_id());
 
   // Now calculate the set of layers in |image_decode_tasks_| that are not used
   // by any tile.
@@ -703,15 +751,16 @@ void TileManager::FreeUnusedResourcesForTile(Tile* tile) {
   }
 }
 
-void TileManager::ScheduleTasks() {
+void TileManager::ScheduleTasks(
+    const TileVector& tiles_that_need_to_be_rasterized) {
   TRACE_EVENT1("cc", "TileManager::ScheduleTasks",
-               "count", tiles_that_need_to_be_rasterized_.size());
+               "count", tiles_that_need_to_be_rasterized.size());
   RasterWorkerPool::RasterTask::Queue tasks;
 
   // Build a new task queue containing all task currently needed. Tasks
   // are added in order of priority, highest priority task first.
-  for (TileVector::iterator it = tiles_that_need_to_be_rasterized_.begin();
-       it != tiles_that_need_to_be_rasterized_.end();
+  for (TileVector::const_iterator it = tiles_that_need_to_be_rasterized.begin();
+       it != tiles_that_need_to_be_rasterized.end();
        ++it) {
     Tile* tile = *it;
     ManagedTileState& mts = tile->managed_state();
@@ -801,7 +850,7 @@ RasterWorkerPool::RasterTask TileManager::CreateRasterTask(Tile* tile) {
       rendering_stats_instrumentation_,
       base::Bind(&TileManager::OnRasterTaskCompleted,
                  base::Unretained(this),
-                 make_scoped_refptr(tile),
+                 tile->id(),
                  base::Passed(&resource),
                  mts.raster_mode),
       &decode_tasks);
@@ -831,7 +880,7 @@ void TileManager::OnImageDecodeTaskCompleted(
 }
 
 void TileManager::OnRasterTaskCompleted(
-    scoped_refptr<Tile> tile,
+    ManagedTileId managed_id,
     scoped_ptr<ResourcePool::Resource> resource,
     RasterMode raster_mode,
     const PicturePileImpl::Analysis& analysis,
@@ -839,6 +888,13 @@ void TileManager::OnRasterTaskCompleted(
   TRACE_EVENT1("cc", "TileManager::OnRasterTaskCompleted",
                "was_canceled", was_canceled);
 
+  TileMap::iterator tile_iter = tiles_.find(managed_id);

[reveman, 2013/07/04 00:46:11]

TileMap::iterator it = tiles_.fi.. ?

[vmpstr, 2013/07/08 17:28:30]

Done.

+  if (tile_iter == tiles_.end()) {
+    resource_pool_->ReleaseResource(resource.Pass());
+    return;
+  }
+
+  Tile* tile = tile_iter->second;
   ManagedTileState& mts = tile->managed_state();
   ManagedTileState::TileVersion& tile_version =
       mts.tile_versions[raster_mode];
@@ -858,9 +914,9 @@ void TileManager::OnRasterTaskCompleted(
     tile_version.resource_ = resource.Pass();
   }
 
-  FreeUnusedResourcesForTile(tile.get());
+  FreeUnusedResourcesForTile(tile);
 
-  DidFinishTileInitialization(tile.get());
+  DidFinishTileInitialization(tile);
 }
 
 void TileManager::DidFinishTileInitialization(Tile* tile) {