Added a vector to TileManager to explicitly track live or allocated tiles.

cc/tile_manager.cc

Index: cc/tile_manager.cc
diff --git a/cc/tile_manager.cc b/cc/tile_manager.cc
index 5c804bb67c04d37aa688c2121a6e351e96c311ed..b77cc59565e19d0ff201161601ab696db0fa7c0d 100644
--- a/cc/tile_manager.cc
+++ b/cc/tile_manager.cc
@@ -49,7 +49,7 @@ inline TileManagerBin BinFromTilePriority(const TilePriority& prio) {
       TilePriority::kMaxDistanceInContentSpace)
     return NEVER_BIN;
 
-  if (prio.time_to_visible_in_seconds == std::numeric_limits<float>::max())
+  if (prio.time_to_visible_in_seconds >= std::numeric_limits<float>::max())

[reveman, 2013/02/11 22:42:28]

why did you change this?

[whunt, 2013/02/11 22:58:41]

The code that calculates distance actually returns
std::numeric_limits<float>::infinity() when things don't intersect.  That case
should be caught early.  I'm not actually sure why
std::numeric_limits<float>::max() was ever used, I don't think it's correct. 
However in order to preserve the behavior in case max *is* ever used I just made
it a >= rather than == to catch both cases.

[reveman, 2013/02/11 23:53:57]

Ok, nice catch. I'm seeing use of both max() and infinity() in
tile_priority.h/picture_layer_tiling.cc. This can be causing some of
prioritizing issues we're seeing and I think it deserves it's own cl. Can you
quickly create a separate patch for this and instead of using >= here, create a
TilePriority::kMaxTimeToVisibleInSeconds constant and make sure it's used where
appropriate?

     return NEVER_BIN;
 
   if (prio.time_to_visible_in_seconds == 0 ||
@@ -144,8 +144,9 @@ TileManager::~TileManager() {
   // resources.
   raster_worker_pool_.reset();
   CheckForCompletedTileUploads();
-  DCHECK(tiles_with_pending_set_pixels_.size() == 0);
-  DCHECK(tiles_.size() == 0);
+  DCHECK_EQ(tiles_with_pending_set_pixels_.size(), 0);
+  DCHECK_EQ(all_tiles_.size(), 0);
+  DCHECK_EQ(live_or_allocated_tiles_.size(), 0);
 }
 
 void TileManager::SetGlobalState(
@@ -156,7 +157,7 @@ void TileManager::SetGlobalState(
 }
 
 void TileManager::RegisterTile(Tile* tile) {
-  tiles_.push_back(tile);
+  all_tiles_.push_back(tile);
 
   const ManagedTileState& mts = tile->managed_state();
   for (int i = 0; i < NUM_TREES; ++i)
@@ -180,13 +181,21 @@ void TileManager::UnregisterTile(Tile* tile) {
       break;
     }
   }
-  for (TileVector::iterator it = tiles_.begin(); it != tiles_.end(); it++) {
+  for (TileVector::iterator it = live_or_allocated_tiles_.begin();
+       it != live_or_allocated_tiles_.end(); it++) {
+    if (*it == tile) {
+      live_or_allocated_tiles_.erase(it);
+      break;
+    }
+  }
+  for (TileVector::iterator it = all_tiles_.begin();
+       it != all_tiles_.end(); it++) {
     if (*it == tile) {
       const ManagedTileState& mts = tile->managed_state();
       for (int i = 0; i < NUM_TREES; ++i)
         --raster_state_count_[mts.raster_state][i][mts.tree_bin[i]];
       FreeResourcesForTile(tile);
-      tiles_.erase(it);
+      all_tiles_.erase(it);
       return;
     }
   }
@@ -254,7 +263,8 @@ void TileManager::SortTiles() {
   TRACE_COUNTER_ID1("cc", "LiveTileCount", this, i + 1 - tiles_.begin());
 
   // Sort by bin, resolution and time until needed.
-  std::sort(tiles_.begin(), i + 1, BinComparator());
+  std::sort(live_or_allocated_tiles_.begin(),
+            live_or_allocated_tiles_.end(), BinComparator());
 }
 
 void TileManager::ManageTiles() {
@@ -263,10 +273,36 @@ void TileManager::ManageTiles() {
   ++manage_tiles_call_count_;
 
   const TreePriority tree_priority = global_state_.tree_priority;
-  TRACE_COUNTER_ID1("cc", "TileCount", this, tiles_.size());
+  TRACE_COUNTER_ID1("cc", "TileCount", this, all_tiles_.size());
+
+  // Memory limit policy works by mapping some bin states to the NEVER bin.
+  TileManagerBin bin_map[NUM_BINS];
+  if (global_state_.memory_limit_policy == ALLOW_NOTHING) {
+    bin_map[NOW_BIN] = NEVER_BIN;
+    bin_map[SOON_BIN] = NEVER_BIN;
+    bin_map[EVENTUALLY_BIN] = NEVER_BIN;
+    bin_map[NEVER_BIN] = NEVER_BIN;
+  } else if (global_state_.memory_limit_policy == ALLOW_ABSOLUTE_MINIMUM) {
+    bin_map[NOW_BIN] = NOW_BIN;
+    bin_map[SOON_BIN] = NEVER_BIN;
+    bin_map[EVENTUALLY_BIN] = NEVER_BIN;
+    bin_map[NEVER_BIN] = NEVER_BIN;
+  } else if (global_state_.memory_limit_policy == ALLOW_PREPAINT_ONLY) {
+    bin_map[NOW_BIN] = NOW_BIN;
+    bin_map[SOON_BIN] = SOON_BIN;
+    bin_map[EVENTUALLY_BIN] = NEVER_BIN;
+    bin_map[NEVER_BIN] = NEVER_BIN;
+  } else {
+    bin_map[NOW_BIN] = NOW_BIN;
+    bin_map[SOON_BIN] = SOON_BIN;
+    bin_map[EVENTUALLY_BIN] = EVENTUALLY_BIN;
+    bin_map[NEVER_BIN] = NEVER_BIN;
+  }
 
+  live_or_allocated_tiles_.clear();
   // For each tree, bin into different categories of tiles.
-  for (TileVector::iterator it = tiles_.begin(); it != tiles_.end(); ++it) {
+  for (TileVector::iterator it = all_tiles_.begin();
+       it != all_tiles_.end(); ++it) {
     Tile* tile = *it;
     ManagedTileState& mts = tile->managed_state();
 
@@ -293,46 +329,29 @@ void TileManager::ManageTiles() {
     mts.bin[LOW_PRIORITY_BIN] = BinFromTilePriority(prio[LOW_PRIORITY_BIN]);
     mts.gpu_memmgr_stats_bin = BinFromTilePriority(tile->combined_priority());
 
-    DidTileBinChange(tile,
-                     BinFromTilePriority(tile->priority(ACTIVE_TREE)),
-                     ACTIVE_TREE);
-    DidTileBinChange(tile,
-                     BinFromTilePriority(tile->priority(PENDING_TREE)),
-                     PENDING_TREE);
-  }
+    DidTileTreeBinChange(tile,
+                         BinFromTilePriority(tile->priority(ACTIVE_TREE)),
+                         ACTIVE_TREE);
+    DidTileTreeBinChange(tile,
+                         BinFromTilePriority(tile->priority(PENDING_TREE)),
+                         PENDING_TREE);
 
-  // Memory limit policy works by mapping some bin states to the NEVER bin.
-  TileManagerBin bin_map[NUM_BINS];
-  if (global_state_.memory_limit_policy == ALLOW_NOTHING) {
-    bin_map[NOW_BIN] = NEVER_BIN;
-    bin_map[SOON_BIN] = NEVER_BIN;
-    bin_map[EVENTUALLY_BIN] = NEVER_BIN;
-    bin_map[NEVER_BIN] = NEVER_BIN;
-  } else if (global_state_.memory_limit_policy == ALLOW_ABSOLUTE_MINIMUM) {
-    bin_map[NOW_BIN] = NOW_BIN;
-    bin_map[SOON_BIN] = NEVER_BIN;
-    bin_map[EVENTUALLY_BIN] = NEVER_BIN;
-    bin_map[NEVER_BIN] = NEVER_BIN;
-  } else if (global_state_.memory_limit_policy == ALLOW_PREPAINT_ONLY) {
-    bin_map[NOW_BIN] = NOW_BIN;
-    bin_map[SOON_BIN] = SOON_BIN;
-    bin_map[EVENTUALLY_BIN] = NEVER_BIN;
-    bin_map[NEVER_BIN] = NEVER_BIN;
-  } else {
-    bin_map[NOW_BIN] = NOW_BIN;
-    bin_map[SOON_BIN] = SOON_BIN;
-    bin_map[EVENTUALLY_BIN] = EVENTUALLY_BIN;
-    bin_map[NEVER_BIN] = NEVER_BIN;
-  }
-  for (TileVector::iterator it = tiles_.begin(); it != tiles_.end(); ++it) {
-    Tile* tile = *it;
-    ManagedTileState& mts = tile->managed_state();
     for (int i = 0; i < NUM_BIN_PRIORITIES; ++i)
       mts.bin[i] = bin_map[mts.bin[i]];
 
-    DidTileBinChange(tile, bin_map[mts.tree_bin[ACTIVE_TREE]], ACTIVE_TREE);
-    DidTileBinChange(tile, bin_map[mts.tree_bin[PENDING_TREE]], PENDING_TREE);
+    DidTileTreeBinChange(tile, bin_map[mts.tree_bin[ACTIVE_TREE]],
+                         ACTIVE_TREE);
+    DidTileTreeBinChange(tile, bin_map[mts.tree_bin[PENDING_TREE]],
+                         PENDING_TREE);
+
+    if (tile->priority(ACTIVE_TREE).is_live ||
+        tile->priority(PENDING_TREE).is_live ||
+        tile->GetResourceId() != 0) {

[reveman, 2013/02/11 22:42:28]

what if the tile is being initialized? GetResourceId() will only return non zero
when tile has finished initializing.

[whunt, 2013/02/11 22:58:41]

Then we can't free it this round, we'll have to wait until a frame on which it
has been initialized before it'll get freed (which makes sense).

[reveman, 2013/02/11 23:53:57]

but you're not adding it to the vector, which means it wont be included when
computing unreleasable_bytes in AssignGpuMemoryToTiles()? That doesn't seem
right.

[whunt, 2013/02/12 19:15:36]

I added || !tile->managed_state().can_be_freed

+      live_or_allocated_tiles_.push_back(tile);
+    }
   }
+  TRACE_COUNTER_ID1("cc", "LiveOrAllocatedTileCount", this,
+                    live_or_allocated_tiles_.size());
 
   SortTiles();
 
@@ -381,8 +400,8 @@ void TileManager::GetMemoryStats(
   *memoryRequiredBytes = 0;
   *memoryNiceToHaveBytes = 0;
   *memoryUsedBytes = 0;
-  for(size_t i = 0; i < tiles_.size(); i++) {
-    const Tile* tile = tiles_[i];
+  for (size_t i = 0; i < live_or_allocated_tiles_.size(); i++) {
+    const Tile* tile = live_or_allocated_tiles_[i];
     const ManagedTileState& mts = tile->managed_state();
     size_t tile_bytes = tile->bytes_consumed_if_allocated();
     if (mts.gpu_memmgr_stats_bin == NOW_BIN)
@@ -396,7 +415,7 @@ void TileManager::GetMemoryStats(
 
 scoped_ptr<base::Value> TileManager::AsValue() const {
     scoped_ptr<base::DictionaryValue> state(new base::DictionaryValue());
-    state->SetInteger("tile_count", tiles_.size());
+    state->SetInteger("tile_count", all_tiles_.size());
 
     state->Set("global_state", global_state_.AsValue().release());
 
@@ -457,19 +476,11 @@ bool TileManager::HasPendingWorkScheduled(WhichTree tree) const {
 
 void TileManager::AssignGpuMemoryToTiles() {
   TRACE_EVENT0("cc", "TileManager::AssignGpuMemoryToTiles");
-  // Some memory cannot be released. Figure out which.
   size_t unreleasable_bytes = 0;
-  for (TileVector::iterator it = tiles_.begin(); it != tiles_.end(); ++it) {
-    Tile* tile = *it;
-    if (!tile->managed_state().can_be_freed)
-      unreleasable_bytes += tile->bytes_consumed_if_allocated();
-  }
 
   // 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_.erase(
-      tiles_that_need_to_be_rasterized_.begin(),
-      tiles_that_need_to_be_rasterized_.end());
+  tiles_that_need_to_be_rasterized_.clear();
 
   // Reset the image decoding list so that we don't mess up with tile
   // priorities. Tiles will be added to the image decoding list again
@@ -481,8 +492,13 @@ void TileManager::AssignGpuMemoryToTiles() {
   // currently waiting for raster to idle state.
   // Call DidTileRasterStateChange() for each of these tiles to
   // have this state change take effect.
-  for (TileVector::iterator it = tiles_.begin(); it != tiles_.end(); ++it) {
+  // Some memory cannot be released. We figure out how much in this
+  // loop as well.
+  for (TileVector::iterator it = live_or_allocated_tiles_.begin();
+       it != live_or_allocated_tiles_.end(); ++it) {
     Tile* tile = *it;
+    if (!tile->managed_state().can_be_freed)
+      unreleasable_bytes += tile->bytes_consumed_if_allocated();
     if (tile->managed_state().raster_state == WAITING_FOR_RASTER_STATE)
       DidTileRasterStateChange(tile, IDLE_STATE);
   }
@@ -490,7 +506,8 @@ void TileManager::AssignGpuMemoryToTiles() {
   size_t bytes_allocatable = global_state_.memory_limit_in_bytes - unreleasable_bytes;
   size_t bytes_that_exceeded_memory_budget = 0;
   size_t bytes_left = bytes_allocatable;
-  for (TileVector::iterator it = tiles_.begin(); it != tiles_.end(); ++it) {
+  for (TileVector::iterator it = live_or_allocated_tiles_.begin();
+       it != live_or_allocated_tiles_.end(); ++it) {
     Tile* tile = *it;
     size_t tile_bytes = tile->bytes_consumed_if_allocated();
     ManagedTileState& managed_tile_state = tile->managed_state();
@@ -798,9 +815,9 @@ void TileManager::DidTileRasterStateChange(Tile* tile, TileRasterState state) {
   mts.raster_state = state;
 }
 
-void TileManager::DidTileBinChange(Tile* tile,
-                                   TileManagerBin bin,
-                                   WhichTree tree) {
+void TileManager::DidTileTreeBinChange(Tile* tile,
+                                       TileManagerBin new_tree_bin,
+                                       WhichTree tree) {
   ManagedTileState& mts = tile->managed_state();
 
   // Decrement count for current bin.
@@ -808,9 +825,9 @@ void TileManager::DidTileBinChange(Tile* tile,
   DCHECK_GE(raster_state_count_[mts.raster_state][tree][mts.tree_bin[tree]], 0);
 
   // Increment count for new bin.
-  ++raster_state_count_[mts.raster_state][tree][bin];
+  ++raster_state_count_[mts.raster_state][tree][new_tree_bin];
 
-  mts.tree_bin[tree] = bin;
+  mts.tree_bin[tree] = new_tree_bin;
 }
 
 // static