cc: Do not limit number of raster tasks for upload limit.

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 354 matching lines @@
   state->SetInteger("canceled_count", stats.canceled_count);
   return state.PassAs<base::Value>();
 }
 
 // static
 scoped_ptr<TileManager> TileManager::Create(
     TileManagerClient* client,
     ResourcePool* resource_pool,
     Rasterizer* rasterizer,
     Rasterizer* gpu_rasterizer,
-    size_t max_raster_usage_bytes,
     bool use_rasterize_on_demand,
     RenderingStatsInstrumentation* rendering_stats_instrumentation) {
   return make_scoped_ptr(new TileManager(client,
                                          resource_pool,
                                          rasterizer,
                                          gpu_rasterizer,
-                                         max_raster_usage_bytes,
                                          use_rasterize_on_demand,
                                          rendering_stats_instrumentation));
 }
 
 TileManager::TileManager(
     TileManagerClient* client,
     ResourcePool* resource_pool,
     Rasterizer* rasterizer,
     Rasterizer* gpu_rasterizer,
-    size_t max_raster_usage_bytes,
     bool use_rasterize_on_demand,
     RenderingStatsInstrumentation* rendering_stats_instrumentation)
     : client_(client),
       resource_pool_(resource_pool),
       prioritized_tiles_dirty_(false),
       all_tiles_that_need_to_be_rasterized_have_memory_(true),
       all_tiles_required_for_activation_have_memory_(true),
       memory_required_bytes_(0),
       memory_nice_to_have_bytes_(0),
       bytes_releasable_(0),
       resources_releasable_(0),
-      max_raster_usage_bytes_(max_raster_usage_bytes),
       ever_exceeded_memory_budget_(false),
       rendering_stats_instrumentation_(rendering_stats_instrumentation),
       did_initialize_visible_tile_(false),
       did_check_for_completed_tasks_since_last_schedule_tasks_(true),
       use_rasterize_on_demand_(use_rasterize_on_demand) {
   Rasterizer* rasterizers[NUM_RASTERIZER_TYPES] = {
       rasterizer,      // RASTERIZER_TYPE_DEFAULT
       gpu_rasterizer,  // RASTERIZER_TYPE_GPU
   };
   rasterizer_delegate_ =
@@ skipping 410 matching lines @@
 
   size_t bytes_that_exceeded_memory_budget = 0;
   size_t soft_bytes_left = soft_bytes_allocatable;
   size_t hard_bytes_left = hard_bytes_allocatable;
 
   size_t resources_left = resources_allocatable;
   bool oomed_soft = false;
   bool oomed_hard = false;
   bool have_hit_soft_memory = false;  // Soft memory comes after hard.
 
-  // Memory we assign to raster tasks now will be deducted from our memory
-  // in future iterations if priorities change. By assigning at most half
-  // the raster limit, we will always have another 50% left even if priorities
-  // change completely (assuming we check for completed/cancelled rasters
-  // between each call to this function).

[reveman, 2014/05/08 13:55:37]

FYI, I'm OK removing this as I don't think this "assign at most 1/2 available
budget" logic is worthwhile. The worst that should happen is that we end up
cancelling all currently scheduled task and reclaim the memory and use it
immediately after when DidFinishRunningTasks is called.

[vmpstr, 2014/05/08 16:49:04]

I agree with this as well. I think tile manager throttling should only apply to
amount of tasks scheduled, not the type of tasks or how long they would take to
upload.

-  size_t max_raster_bytes = max_raster_usage_bytes_ / 2;
-  size_t raster_bytes = 0;
-
   unsigned schedule_priority = 1u;
   for (PrioritizedTileSet::Iterator it(tiles, true); it; ++it) {
     Tile* tile = *it;
     ManagedTileState& mts = tile->managed_state();
 
     mts.scheduled_priority = schedule_priority++;
 
     mts.raster_mode = tile->DetermineOverallRasterMode();
 
     ManagedTileState::TileVersion& tile_version =
         mts.tile_versions[mts.raster_mode];
 
     // If this tile doesn't need a resource, then nothing to do.
     if (!tile_version.requires_resource())
       continue;
 
     // If the tile is not needed, free it up.
     if (mts.bin == NEVER_BIN) {
       FreeResourcesForTile(tile);
       continue;
     }
 
     const bool tile_uses_hard_limit = mts.bin <= NOW_BIN;
     const size_t bytes_if_allocated = BytesConsumedIfAllocated(tile);
-    const size_t raster_bytes_if_rastered = raster_bytes + bytes_if_allocated;
     const size_t tile_bytes_left =
         (tile_uses_hard_limit) ? hard_bytes_left : soft_bytes_left;
 
     // Hard-limit is reserved for tiles that would cause a calamity
     // if they were to go away, so by definition they are the highest
     // priority memory, and must be at the front of the list.
     DCHECK(!(have_hit_soft_memory && tile_uses_hard_limit));
     have_hit_soft_memory |= !tile_uses_hard_limit;
 
     size_t tile_bytes = 0;
     size_t tile_resources = 0;
 
     // It costs to maintain a resource.
     for (int mode = 0; mode < NUM_RASTER_MODES; ++mode) {
       if (mts.tile_versions[mode].resource_) {
         tile_bytes += bytes_if_allocated;
         tile_resources++;
       }
     }
 
     // Allow lower priority tiles with initialized resources to keep
     // their memory by only assigning memory to new raster tasks if
     // they can be scheduled.
-    if (raster_bytes_if_rastered <= max_raster_bytes) {
+    if (tiles_that_need_to_be_rasterized->size() < kScheduledRasterTasksLimit) {

[reveman, 2014/05/08 13:55:37]

why do we need a conditional here now? was this code wrong before?

[vmpstr, 2014/05/08 16:49:04]

I think it should've been <= raster_bytes || size <= limit before... 

[reveman, 2014/05/08 17:07:51]

Ok, please add a variable here instead of doing
"tiles_that_need_to_be_rasterized->size() < kScheduledRasterTasksLimit" both
here and below. Maybe:

bool have_reached_scheduled_raster_tasks_limit =
    tiles_that_need_to_be_rasterized->size() >=
    kScheduledRasterTasksLimit;

[alokp, 2014/05/08 18:04:47]

Done.

       // 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_) {
         tile_bytes += bytes_if_allocated;
         tile_resources++;
       }
     }
 
     // Tile is OOM.
     if (tile_bytes > tile_bytes_left || tile_resources > resources_left) {
@@ skipping 23 matching lines @@
 
     // Tile shouldn't be rasterized if |tiles_that_need_to_be_rasterized|
     // has reached it's limit or we've failed to assign gpu memory to this
     // or any higher priority tile. Preventing tiles that fit into memory
     // budget to be rasterized when higher priority tile is oom is
     // important for two reasons:
     // 1. Tile size should not impact raster priority.
     // 2. Tiles with existing raster task could otherwise incorrectly
     //    be added as they are not affected by |bytes_allocatable|.
     bool can_schedule_tile =
-        !oomed_soft && raster_bytes_if_rastered <= max_raster_bytes &&
+        !oomed_soft &&
         tiles_that_need_to_be_rasterized->size() < kScheduledRasterTasksLimit;
 
     if (!can_schedule_tile) {
       all_tiles_that_need_to_be_rasterized_have_memory_ = false;
       if (tile->required_for_activation())
         all_tiles_required_for_activation_have_memory_ = false;
       it.DisablePriorityOrdering();
       continue;
     }
 
-    raster_bytes = raster_bytes_if_rastered;
     tiles_that_need_to_be_rasterized->push_back(tile);
   }
 
   // OOM reporting uses hard-limit, soft-OOM is normal depending on limit.
   ever_exceeded_memory_budget_ |= oomed_hard;
   if (ever_exceeded_memory_budget_) {
     TRACE_COUNTER_ID2("cc",
                       "over_memory_budget",
                       this,
                       "budget",
@@ skipping 669 matching lines @@
 
   if (b_priority.resolution != a_priority.resolution) {
     return (prioritize_low_res && b_priority.resolution == LOW_RESOLUTION) ||
            (!prioritize_low_res && b_priority.resolution == HIGH_RESOLUTION) ||
            (a_priority.resolution == NON_IDEAL_RESOLUTION);
   }
   return a_priority.IsHigherPriorityThan(b_priority);
 }
 
 }  // namespace cc