Re-land: cc: Add raster finished signals to RasterWorkerPool.

cc/resources/tile_manager.cc

 // Copyright 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "cc/resources/tile_manager.h"
 
 #include <algorithm>
 #include <limits>
 #include <string>
 
@@ skipping 152 matching lines @@
 
 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));
 }
 
 bool TileManager::ShouldForceTasksRequiredForActivationToComplete() const {
   return client_->ShouldForceTileUploadsRequiredForActivationToComplete();
 }
 
+void 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)
+    return;
+
+  raster_worker_pool_->CheckForCompletedTasks();
+
+  AssignGpuMemoryToTiles();
+
+  if (!oom_tiles_that_need_to_be_initialized_for_activation_.empty())
+    ReassignGpuMemoryToOOMTilesRequiredForActivation();
+
+  // |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();
+    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();
+       ++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();
+
+  client_->NotifyReadyToActivate();
+}
+
+void TileManager::DidFinishedRunningTasksRequiredForActivation() {
+  // This is only a true indication that all tiles required for
+  // activation are initialized when no tiles are OOM. We need to
+  // 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())
+    return;
+
+  // TODO(reveman): Push the responsibility of calling CheckForCompletedTasks()
+  // to the LTHI where it can be delayed until it's time to draw.
+  raster_worker_pool_->CheckForCompletedTasks();
+}
+
 class BinComparator {
  public:
   bool operator() (const Tile* a, const Tile* b) const {
     const ManagedTileState& ams = a->managed_state();
     const ManagedTileState& bms = b->managed_state();
     if (ams.bin[HIGH_PRIORITY_BIN] != bms.bin[HIGH_PRIORITY_BIN])
       return ams.bin[HIGH_PRIORITY_BIN] < bms.bin[HIGH_PRIORITY_BIN];
 
     if (ams.bin[LOW_PRIORITY_BIN] != bms.bin[LOW_PRIORITY_BIN])
       return ams.bin[LOW_PRIORITY_BIN] < bms.bin[LOW_PRIORITY_BIN];
@@ skipping 106 matching lines @@
   std::sort(tiles_.begin(), tiles_.end(), BinComparator());
 }
 
 void TileManager::ManageTiles() {
   TRACE_EVENT0("cc", "TileManager::ManageTiles");
   AssignBinsToTiles();
   SortTiles();
   AssignGpuMemoryToTiles();
   CleanUpUnusedImageDecodeTasks();
 
-  // This could have changed after AssignGpuMemoryToTiles.
-  if (AreTilesRequiredForActivationReady())
-    client_->NotifyReadyToActivate();
-
   TRACE_EVENT_INSTANT1(
       "cc", "DidManage", TRACE_EVENT_SCOPE_THREAD,
       "state", TracedValue::FromValue(BasicStateAsValue().release()));
 
   // Finally, schedule rasterizer tasks.
   ScheduleTasks();
 }
 
 void TileManager::CheckForCompletedTileUploads() {
   raster_worker_pool_->CheckForCompletedTasks();
@@ skipping 84 matching lines @@
   return raster_mode;
 }
 
 void TileManager::AssignGpuMemoryToTiles() {
   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();
        ++it) {
     const Tile* tile = *it;
     const ManagedTileState& mts = tile->managed_state();
     for (int mode = 0; mode < NUM_RASTER_MODES; ++mode) {
       if (mts.tile_versions[mode].resource_)
         bytes_releasable += tile->bytes_consumed_if_allocated();
     }
   }
 
   // Cast to prevent overflow.
   int64 bytes_available =
       static_cast<int64>(bytes_releasable) +
       static_cast<int64>(global_state_.memory_limit_in_bytes) -
       static_cast<int64>(resource_pool_->acquired_memory_usage_bytes());
 
   size_t bytes_allocatable =
       std::max(static_cast<int64>(0), bytes_available);
 
-  size_t bytes_that_exceeded_memory_budget_in_now_bin = 0;
+  size_t bytes_that_exceeded_memory_budget = 0;
   size_t bytes_left = bytes_allocatable;
-  size_t bytes_oom_in_now_bin_on_pending_tree = 0;
-  TileVector tiles_requiring_memory_but_oomed;
   bool higher_priority_tile_oomed = false;
   for (TileVector::iterator it = tiles_.begin();
        it != tiles_.end();
        ++it) {
     Tile* tile = *it;
     ManagedTileState& mts = tile->managed_state();
 
     // Pick the better version out of the one we already set,
     // and the one that is required.
     mts.raster_mode = std::min(mts.raster_mode, DetermineRasterMode(tile));
@@ skipping 19 matching lines @@
         tile_bytes += tile->bytes_consumed_if_allocated();
     }
 
     // If we don't have the required version, and it's not in flight
     // then we'll have to pay to create a new task.
     if (!tile_version.resource_ && tile_version.raster_task_.is_null())
       tile_bytes += tile->bytes_consumed_if_allocated();
 
     // Tile is OOM.
     if (tile_bytes > bytes_left) {
-      mts.tile_versions[mts.raster_mode].set_rasterize_on_demand();
-      if (mts.tree_bin[PENDING_TREE] == NOW_BIN) {
-        tiles_requiring_memory_but_oomed.push_back(tile);
-        bytes_oom_in_now_bin_on_pending_tree += tile_bytes;
-      }
+      if (tile->required_for_activation())
+        oom_tiles_that_need_to_be_initialized_for_activation_.insert(tile);
       FreeResourcesForTile(tile);
       higher_priority_tile_oomed = true;
+      bytes_that_exceeded_memory_budget += tile_bytes;
       continue;
     }
 
     tile_version.set_use_resource();
     bytes_left -= tile_bytes;
 
     // Tile shouldn't be rasterized if we've failed to assign
     // gpu memory to a higher priority tile. This is important for
     // two reasons:
     // 1. Tile size should not impact raster priority.
     // 2. Tile with unreleasable memory could otherwise incorrectly
     //    be added as it's not affected by |bytes_allocatable|.
     if (higher_priority_tile_oomed)
       continue;
 
     if (!tile_version.resource_)
       tiles_that_need_to_be_rasterized_.push_back(tile);
 
     if (!tile->IsReadyToDraw(NULL) &&
         tile->required_for_activation()) {
       AddRequiredTileForActivation(tile);
     }
   }
 
-  // In OOM situation, we iterate tiles_, remove the memory for active tree
-  // and not the now bin. And give them to bytes_oom_in_now_bin_on_pending_tree
-  if (!tiles_requiring_memory_but_oomed.empty()) {
-    size_t bytes_freed = 0;
-    for (TileVector::iterator it = tiles_.begin(); it != tiles_.end(); ++it) {

[reveman, 2013/06/24 19:05:17]

I changed this to a reverse iterator so we try to steal memory from low priority
tiles in the active tree before higher priority tiles.

-      Tile* tile = *it;
-      ManagedTileState& mts = tile->managed_state();
-      ManagedTileState::TileVersion& tile_version =
-          mts.tile_versions[mts.raster_mode];
-      if (mts.tree_bin[PENDING_TREE] == NEVER_BIN &&
-          mts.tree_bin[ACTIVE_TREE] != NOW_BIN) {
-        size_t bytes_that_can_be_freed = 0;
-
-        // If the tile is in the to-rasterize list, but it has no task,
-        // then it means that we have assigned memory for it.

[reveman, 2013/06/24 16:04:04]

I think there's a bug here. A tile will still be in the to-rasterize list after
completing and raster_task will then be null. We might want to land a fix for
this separately.

[vmpstr, 2013/06/24 16:34:03]

I don't think that's possible with the current code. to-rasterize list is
cleared at the top of the function, and the only items put into this are tasks
that actually need a raster (ie not completed). Unless you mean that it was
completed but we were not notified of that yet? I'm not sure what we can do
about that, short of forcing completion check

I think with the refactor that you have done here, it might be more possible? :P
I'm not sure.

[reveman, 2013/06/24 19:05:17]

You're right. It's not possible as tiles_that_need_to_be_rasterized_ is a
temporary variable in practice. We should make that a real temporary variable to
avoid confusion.

I changed back to your original code but simplified it a bit. computing
bytes_that_can_be_freed before actually freeing things doesn't seem to serve a
purpose. Latest patch just frees and adjusts bytes_freed directly. PTAL.

-        TileVector::iterator raster_it =
-            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() &&
-            tile_version.raster_task_.is_null()) {
-          bytes_that_can_be_freed += tile->bytes_consumed_if_allocated();
-        }
-
-        // Also consider all of the completed resources for freeing.
-        for (int mode = 0; mode < NUM_RASTER_MODES; ++mode) {
-          ManagedTileState::TileVersion& tile_version =
-              mts.tile_versions[mode];
-          if (tile_version.resource_) {
-            DCHECK(!tile->required_for_activation());
-            bytes_that_can_be_freed += tile->bytes_consumed_if_allocated();
-          }
-        }
-
-        // If we can free anything, then do so.
-        if (bytes_that_can_be_freed > 0) {
-          FreeResourcesForTile(tile);
-          bytes_freed += bytes_that_can_be_freed;
-          mts.tile_versions[mts.raster_mode].set_rasterize_on_demand();
-          if (raster_it != tiles_that_need_to_be_rasterized_.end())
-              tiles_that_need_to_be_rasterized_.erase(raster_it);
-        }
-      }
-
-      if (bytes_oom_in_now_bin_on_pending_tree <= bytes_freed)
-        break;
-    }
-
-    for (TileVector::iterator it = tiles_requiring_memory_but_oomed.begin();
-         it != tiles_requiring_memory_but_oomed.end() && bytes_freed > 0;
-         ++it) {
-      Tile* tile = *it;
-      ManagedTileState& mts = tile->managed_state();
-      size_t bytes_needed = tile->bytes_consumed_if_allocated();
-      if (bytes_needed > bytes_freed)
-        continue;
-      mts.tile_versions[mts.raster_mode].set_use_resource();
-      bytes_freed -= bytes_needed;
-      tiles_that_need_to_be_rasterized_.push_back(tile);
-      if (tile->required_for_activation())
-        AddRequiredTileForActivation(tile);
-    }
-  }
-
-  ever_exceeded_memory_budget_ |=
-      bytes_that_exceeded_memory_budget_in_now_bin > 0;
+  ever_exceeded_memory_budget_ |= bytes_that_exceeded_memory_budget > 0;
   if (ever_exceeded_memory_budget_) {
       TRACE_COUNTER_ID2("cc", "over_memory_budget", this,
                         "budget", global_state_.memory_limit_in_bytes,
-                        "over", bytes_that_exceeded_memory_budget_in_now_bin);
+                        "over", bytes_that_exceeded_memory_budget);
   }
   memory_stats_from_last_assign_.total_budget_in_bytes =
       global_state_.memory_limit_in_bytes;
   memory_stats_from_last_assign_.bytes_allocated =
       bytes_allocatable - bytes_left;
   memory_stats_from_last_assign_.bytes_unreleasable =
       bytes_allocatable - bytes_releasable;
   memory_stats_from_last_assign_.bytes_over =
-      bytes_that_exceeded_memory_budget_in_now_bin;
+      bytes_that_exceeded_memory_budget;
+}
+
+void TileManager::ReassignGpuMemoryToOOMTilesRequiredForActivation() {
+  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) {
+    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())
+      continue;
+
+    tiles_requiring_memory_but_oomed.push_back(tile);
+    bytes_oom_for_required_tiles += tile->bytes_consumed_if_allocated();
+  }
+
+  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
+  size_t bytes_freed = 0;
+  for (TileVector::iterator it = tiles_.begin(); it != tiles_.end(); ++it) {
+    Tile* tile = *it;
+    ManagedTileState& mts = tile->managed_state();
+    if (mts.tree_bin[PENDING_TREE] == NEVER_BIN &&
+        mts.tree_bin[ACTIVE_TREE] != NOW_BIN) {
+      size_t bytes_that_can_be_freed = 0;
+
+      for (int mode = 0; mode < NUM_RASTER_MODES; ++mode) {
+        ManagedTileState::TileVersion& tile_version = mts.tile_versions[mode];
+
+        // Consider all of the completed resources for freeing.
+        if (tile_version.resource_) {
+          DCHECK(!tile->required_for_activation());
+          bytes_that_can_be_freed += tile->bytes_consumed_if_allocated();
+          continue;
+        }
+
+        // If the tile is in the to-rasterize list, it means that we
+        // have assigned memory to initialize current raster mode.
+        if (mode == mts.raster_mode) {

[vmpstr, 2013/06/24 16:34:03]

It's a bit awkward to have this in the inner loop. Is it possible to move this
out? Maybe have something like "bool current_raster_mode_had_resource" temporary
variable or something? This is not a strong opinion though, so I'm fine if you
like it this way.

+          TileVector::iterator raster_it =
+              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())
+            bytes_that_can_be_freed += tile->bytes_consumed_if_allocated();
+        }
+      }
+
+      // If we can free anything, then do so.
+      if (bytes_that_can_be_freed > 0) {
+        FreeResourcesForTile(tile);
+        bytes_freed += bytes_that_can_be_freed;
+        TileVector::iterator raster_it = 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())
+          tiles_that_need_to_be_rasterized_.erase(raster_it);
+      }
+    }
+
+    if (bytes_oom_for_required_tiles <= bytes_freed)
+      break;
+  }
+
+  for (TileVector::iterator it = tiles_requiring_memory_but_oomed.begin();
+       it != tiles_requiring_memory_but_oomed.end() && bytes_freed > 0;
+       ++it) {
+    Tile* tile = *it;
+    ManagedTileState& mts = tile->managed_state();
+    size_t bytes_needed = tile->bytes_consumed_if_allocated();
+    if (bytes_needed > bytes_freed)
+      continue;
+    mts.tile_versions[mts.raster_mode].set_use_resource();
+    bytes_freed -= bytes_needed;
+    tiles_that_need_to_be_rasterized_.push_back(tile);
+    AddRequiredTileForActivation(tile);

[vmpstr, 2013/06/24 16:34:03]

DCHECK that it's required for activation, please.

[reveman, 2013/06/24 19:05:17]

Done.

+    oom_tiles_that_need_to_be_initialized_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());
 
   // Now calculate the set of layers in |image_decode_tasks_| that are not used
   // by any tile.
@@ skipping 30 matching lines @@
 void TileManager::FreeUnusedResourcesForTile(Tile* tile) {
   RasterMode used_mode = HIGH_QUALITY_RASTER_MODE;
   bool version_is_used = tile->IsReadyToDraw(&used_mode);
   for (int mode = 0; mode < NUM_RASTER_MODES; ++mode) {
     if (!version_is_used || mode != used_mode)
       FreeResourceForTile(tile, static_cast<RasterMode>(mode));
   }
 }
 
 void TileManager::ScheduleTasks() {
-  TRACE_EVENT0("cc", "TileManager::ScheduleTasks");
+  TRACE_EVENT1("cc", "TileManager::ScheduleTasks",
+               "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();
        ++it) {
     Tile* tile = *it;
     ManagedTileState& mts = tile->managed_state();
     ManagedTileState::TileVersion& tile_version =
         mts.tile_versions[mts.raster_mode];
 
     DCHECK(tile_version.requires_resource());
     DCHECK(!tile_version.resource_);
 
     if (tile_version.raster_task_.is_null())
       tile_version.raster_task_ = CreateRasterTask(tile);
 
     tasks.Append(tile_version.raster_task_, tile->required_for_activation());
   }
 
   // Schedule running of |tasks|. This replaces any previously
   // scheduled tasks and effectively cancels all tasks not present
   // in |tasks|.
   raster_worker_pool_->ScheduleTasks(&tasks);
 }
 
 RasterWorkerPool::Task TileManager::CreateImageDecodeTask(
     Tile* tile, skia::LazyPixelRef* pixel_ref) {
-  TRACE_EVENT0("cc", "TileManager::CreateImageDecodeTask");

[vmpstr, 2013/06/24 16:34:03]

Can we replace these with INSTANT traces that just log the number of image
decodes and raster tasks that were used?

[reveman, 2013/06/24 19:05:17]

I added a count argument to the ScheduleTasks trace for this purpose. A trace
event for each task allocation seems a bit excessive but I can add that if you
insist?

[vmpstr, 2013/06/24 19:16:46]

No, I just meant to have the count somewhere. I'm happy with what you have now.

-
   return RasterWorkerPool::CreateImageDecodeTask(
       pixel_ref,
       tile->layer_id(),
       rendering_stats_instrumentation_,
       base::Bind(&TileManager::OnImageDecodeTaskCompleted,
                  base::Unretained(this),
                  tile->layer_id(),
                  base::Unretained(pixel_ref)));
 }
 
 RasterTaskMetadata TileManager::GetRasterTaskMetadata(
     const Tile& tile) const {
   RasterTaskMetadata metadata;
   const ManagedTileState& mts = tile.managed_state();
   metadata.is_tile_in_pending_tree_now_bin =
       mts.tree_bin[PENDING_TREE] == NOW_BIN;
   metadata.tile_resolution = mts.resolution;
   metadata.layer_id = tile.layer_id();
   metadata.tile_id = &tile;
   metadata.source_frame_number = tile.source_frame_number();
   return metadata;
 }
 
 RasterWorkerPool::RasterTask TileManager::CreateRasterTask(Tile* tile) {
-  TRACE_EVENT0("cc", "TileManager::CreateRasterTask");
-
   ManagedTileState& mts = tile->managed_state();
 
   scoped_ptr<ResourcePool::Resource> resource =
       resource_pool_->AcquireResource(tile->tile_size_.size(),
                                       texture_format_);
   const Resource* const_resource = resource.get();
 
   // Create and queue all image decode tasks that this tile depends on.
   RasterWorkerPool::Task::Set decode_tasks;
   PixelRefTaskMap& existing_pixel_refs = image_decode_tasks_[tile->layer_id()];
@@ skipping 107 matching lines @@
 }
 
 void TileManager::DidTileTreeBinChange(Tile* tile,
                                        TileManagerBin new_tree_bin,
                                        WhichTree tree) {
   ManagedTileState& mts = tile->managed_state();
   mts.tree_bin[tree] = new_tree_bin;
 }
 
 }  // namespace cc