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

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 ab53cfd5f3984375ae05393a16b20a326ab66ea8..a8128f52a922b709930d4e50d292feaa6833058c 100644
--- a/cc/resources/tile_manager.cc
+++ b/cc/resources/tile_manager.cc
@@ -165,6 +165,7 @@ void TileManager::UnregisterTile(Tile* tile) {
     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);
@@ -175,6 +176,44 @@ bool TileManager::ShouldForceTasksRequiredForActivationToComplete() const {
   return client_->ShouldForceTileUploadsRequiredForActivationToComplete();
 }
 
+void TileManager::DidFinishedRunningTasks() {
+  if (oom_tiles_that_need_to_be_initialized_for_activation_.empty())
+    return;
+
+  raster_worker_pool_->CheckForCompletedTasks();
+
+  AssignGpuMemoryToTiles();

[vmpstr, 2013/06/21 18:38:33]

Can we make this return true if reassign oom is required?

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

I'd rather call reassign unconditionally to keep things as simple as possible
for now. This is a bit different in latest patch. PTAL.

+  ReassignGpuMemoryToOOMTiles();
+
+  if (!tiles_that_need_to_be_rasterized_.empty()) {
+    ScheduleTasks();
+    return;
+  }
+
+  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() {
+  if (!oom_tiles_that_need_to_be_initialized_for_activation_.empty())

[vmpstr, 2013/06/21 18:38:33]

Can you make a comment that explains why this is not a dcheck. That is, the fact
that we could have already scheduled new activation required tasks

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

Added a comment that explains why we might need to early out here.

+    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();
+
+  client_->NotifyReadyToActivate();
+}
+
 class BinComparator {
  public:
   bool operator() (const Tile* a, const Tile* b) const {
@@ -301,10 +340,6 @@ void TileManager::ManageTiles() {
   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()));
@@ -409,6 +444,7 @@ void TileManager::AssignGpuMemoryToTiles() {
   // 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();
@@ -433,8 +469,6 @@ void TileManager::AssignGpuMemoryToTiles() {
 
   size_t bytes_that_exceeded_memory_budget_in_now_bin = 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();
@@ -474,11 +508,8 @@ void TileManager::AssignGpuMemoryToTiles() {
 
     // 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;
       continue;
@@ -505,58 +536,6 @@ void TileManager::AssignGpuMemoryToTiles() {
     }
   }
 
-  // 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) {
-      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];
-          if (tile_version.resource_) {
-            DCHECK(!tile->required_for_activation());
-            bytes_that_can_be_freed += tile->bytes_consumed_if_allocated();
-          }
-        }
-
-        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();
-          TileVector::iterator it = std::find(
-                  tiles_that_need_to_be_rasterized_.begin(),
-                  tiles_that_need_to_be_rasterized_.end(),
-                  tile);
-          if (it != tiles_that_need_to_be_rasterized_.end())
-              tiles_that_need_to_be_rasterized_.erase(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;
   if (ever_exceeded_memory_budget_) {
@@ -574,6 +553,70 @@ void TileManager::AssignGpuMemoryToTiles() {
       bytes_that_exceeded_memory_budget_in_now_bin;
 }
 
+void TileManager::ReassignGpuMemoryToOOMTiles() {
+  TRACE_EVENT0("cc", "TileManager::ReassignGpuMemoryToOOMTiles");
+
+  size_t bytes_oom_for_required_tiles = 0;
+  TileVector tiles_requiring_memory_but_oomed;
+  for (TileVector::iterator it = tiles_.begin(); it != tiles_.end(); ++it) {

[vmpstr, 2013/06/21 18:38:33]

Isn't this just a loop over
oom_tiles_that_need_to_be_initialized_for_activation_?

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

Yes, but it adds tiles to |tiles_requiring_memory_but_oomed| in order of
priority. We could instead make
|oom_tiles_that_need_to_be_initialized_for_activation_| a vector ordered by
priority but that would instead make erase less efficient. I'm not too worried
about performance here as we're already in a relatively bad spot when trying to
re-assign memory from active tree.

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

Oh I didn't realize that it was a set, my bad.

+    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();
+  }
+
+  // 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) {

[vmpstr, 2013/06/21 18:38:33]

You should early out before this if nothing is OOM.

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

Done.

+    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];
+        if (tile_version.resource_) {
+          DCHECK(!tile->required_for_activation());
+          bytes_that_can_be_freed += tile->bytes_consumed_if_allocated();
+        }
+      }
+
+      if (bytes_that_can_be_freed > 0) {
+        FreeResourcesForTile(tile);
+        bytes_freed += bytes_that_can_be_freed;
+        TileVector::iterator it = std::find(
+            tiles_that_need_to_be_rasterized_.begin(),
+            tiles_that_need_to_be_rasterized_.end(),
+            tile);
+        if (it != tiles_that_need_to_be_rasterized_.end())
+          tiles_that_need_to_be_rasterized_.erase(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);
+    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;