cc: Rasterize cheap tiles immediately

cc/tile_manager.cc

Index: cc/tile_manager.cc
diff --git a/cc/tile_manager.cc b/cc/tile_manager.cc
index 83c0ab2b235d2f557631804d14967b174ae1e1bb..629d0e75e9acd1ff28f8b38655613a0910f574b3 100644
--- a/cc/tile_manager.cc
+++ b/cc/tile_manager.cc
@@ -34,6 +34,15 @@ const int kMaxPendingUploadBytes = 20 * 1024 * 1024;
 const int kMaxPendingUploadBytes = 100 * 1024 * 1024;
 #endif
 
+// Limit the total number of cheap tile rasterizations we are allowed to perform
+// during a single frame as well as the time spent rasterizing.
+// TODO(skyostil): Determine these limits more dynamically.
+const int kMaxCheapRasterCount = 6;
+const int kMaxCheapRasterMilliseconds = 6;
+
+// Expected time for a cheap tile raster. Only used for tracing.
+const int kExpectedCheapRasterMilliseconds = 2;

[nduca, 2013/02/06 08:34:57]

I've been working off of 1. Why 2?

[Sami, 2013/02/06 15:25:14]

Tom was telling me his heuristic gave 1 ms rasters on average, so I thought it'd
be better to flag the outliers. Tracking the raster time seems redundant now
that Vlad landed his histogram patch, so I'll just remove this.

+
 // Determine bin based on three categories of tiles: things we need now,
 // things we need soon, and eventually.
 inline TileManagerBin BinFromTilePriority(const TilePriority& prio) {
@@ -116,7 +125,8 @@ TileManager::TileManager(
     TileManagerClient* client,
     ResourceProvider* resource_provider,
     size_t num_raster_threads,
-    bool record_rendering_stats)
+    bool record_rendering_stats,
+    bool use_cheapness_estimator)
     : client_(client),
       resource_pool_(ResourcePool::Create(resource_provider)),
       raster_worker_pool_(RasterWorkerPool::Create(num_raster_threads, record_rendering_stats)),
@@ -124,7 +134,9 @@ TileManager::TileManager(
       manage_tiles_call_count_(0),
       bytes_pending_set_pixels_(0),
       ever_exceeded_memory_budget_(false),
-      record_rendering_stats_(record_rendering_stats) {
+      record_rendering_stats_(record_rendering_stats),
+      use_cheapness_estimator_(use_cheapness_estimator),
+      cheap_raster_count_(0) {
   for (int i = 0; i < NUM_STATES; ++i) {
     for (int j = 0; j < NUM_TREES; ++j) {
       for (int k = 0; k < NUM_BINS; ++k)
@@ -340,6 +352,11 @@ void TileManager::CheckForCompletedTileUploads() {
   DispatchMoreTasks();
 }
 
+void TileManager::ResetCheapRasterBudget() {
+  cheap_raster_count_ = 0;
+  cheap_raster_time_ = base::TimeDelta();
+}
+
 void TileManager::GetMemoryStats(
     size_t* memoryRequiredBytes,
     size_t* memoryNiceToHaveBytes,
@@ -509,7 +526,7 @@ void TileManager::FreeResourcesForTile(Tile* tile) {
     resource_pool_->ReleaseResource(managed_tile_state.resource.Pass());
 }
 
-bool TileManager::CanDispatchRasterTask(Tile* tile) {
+bool TileManager::CanDispatchRasterTask(Tile* tile) const {
   if (raster_worker_pool_->IsBusy())
     return false;
   size_t new_bytes_pending = bytes_pending_set_pixels_;
@@ -517,6 +534,39 @@ bool TileManager::CanDispatchRasterTask(Tile* tile) {
   return new_bytes_pending <= kMaxPendingUploadBytes;
 }
 
+bool TileManager::CanPerformCheapRaster(Tile* tile) const {
+  if (!use_cheapness_estimator_)
+    return false;
+  if (global_state_.tree_priority == SMOOTHNESS_TAKES_PRIORITY) {

[nduca, 2013/02/06 08:34:57]

I'm not convinced of this part. Please remove and we can add it back in
afterward. 

[Sami, 2013/02/06 15:25:14]

Okay, let's keep things simple.

+    TRACE_EVENT_INSTANT0(
+        "cc", "TileManager::CanPerformCheapRaster smoothness");
+    return false;
+  }
+  // TODO(skyostil): Use synchronous uploads for inline rasters.

[nduca, 2013/02/06 08:34:57]

dont see why this todo is relevant here... sync uploads are in another part of
the code

[Sami, 2013/02/06 15:25:14]

It's referring to the check against the total allowed async uploads below. Once
cheap tiles are sync uploaded, that check should go away.

I'll remove the TODO since it's already done in
https://codereview.chromium.org/12185024/.

+  size_t new_bytes_pending = bytes_pending_set_pixels_;
+  new_bytes_pending += tile->bytes_consumed_if_allocated();
+  if (new_bytes_pending > kMaxPendingUploadBytes)
+    return false;
+  if (cheap_raster_count_ >= kMaxCheapRasterCount) {
+    TRACE_EVENT_INSTANT0(
+        "cc", "TileManager::CanPerformCheapRaster too many rasters");
+    return false;
+  }
+  base::TimeDelta maxCheapRasterTime =

[reveman, 2013/02/05 20:59:13]

nit: max_cheap_raster_time please

[Sami, 2013/02/06 15:25:14]

Done (moved to lthi).

+      base::TimeDelta::FromMilliseconds(kMaxCheapRasterMilliseconds);
+  if (cheap_raster_time_ >= maxCheapRasterTime) {

[reveman, 2013/02/05 20:59:13]

would we get better accuracy if we recorded a "cheap raster end time" in
ResetCheapRasterBudget() and checked that we haven't gone past it here? I'm
thinking of cases where the impl thread is unscheduled for 11ms and 6ms of cheap
raster will make use miss the vsync.

[Sami, 2013/02/06 15:25:14]

Good point, we could easily use more real time if we're descheduled. I'll change
this to allow up to 6 ms of cheap rasters from when we last swapped. However,
that's not ideal in the case of jsgamebench since it's intentionally running at
30 fps. I'm not sure what to do about that -- maybe allow cheap rasters during
the first halves of frame intervals?

+    TRACE_EVENT_INSTANT0(
+        "cc", "TileManager::CanPerformCheapRaster out of time");
+    return false;
+  }
+  if (!tile->picture_pile()->IsCheapInRect(tile->content_rect_,
+                                           tile->contents_scale())) {
+    TRACE_EVENT_INSTANT0("cc", "TileManager::CanPerformCheapRaster not cheap");

[nduca, 2013/02/06 08:34:57]

please remove and integrate with what vlad did in
https://codereview.chromium.org/12192025/

[Sami, 2013/02/06 15:25:14]

Done.

+    return false;
+  }
+  return true;
+}
+
 void TileManager::DispatchMoreTasks() {
   // Because tiles in the image decoding list have higher priorities, we
   // need to process those tiles first before we start to handle the tiles
@@ -543,6 +593,20 @@ void TileManager::DispatchMoreTasks() {
     ManagedTileState& managed_state = tile->managed_state();
     if (!managed_state.pending_pixel_refs.empty()) {
       tiles_with_image_decoding_tasks_.push_back(tile);
+    } else if (CanPerformCheapRaster(tile)) {
+      cheap_raster_count_++;
+      DCHECK(cheap_raster_count_ <= kMaxCheapRasterCount);
+      base::TimeTicks begin_time = base::TimeTicks::Now();

[reveman, 2013/02/05 20:59:13]

we recently made sure not to call base::TimeTicks::Now() during raster unless
benchmarking is enabled. should we be worried about the calls to
base::TimeTicks::Now() here?

[Sami, 2013/02/06 15:25:14]

I'm not sure how else to keep track of time spent rasterizing. A cursory look
though base didn't reveal a cheaper low res time source.

+      PerformOneRaster(tile);
+      base::TimeTicks end_time = base::TimeTicks::Now();
+      base::TimeDelta duration = end_time - begin_time;
+      base::TimeDelta expectedCheapRasterTime =

[reveman, 2013/02/05 20:59:13]

nit: expected_cheap_raster_time

[Sami, 2013/02/06 15:25:14]

Done (moved to lthi).

+          base::TimeDelta::FromMilliseconds(kExpectedCheapRasterMilliseconds);

[nduca, 2013/02/06 08:34:57]

is this duplicative? you've got the same boookeeping inside the performOneRaster
call already, no?

[Sami, 2013/02/06 15:25:14]

Yeah, no need for this now that Vlad's patch landed. Removed.

+      if (duration > expectedCheapRasterTime) {
+        TRACE_EVENT_INSTANT0(
+            "cc", "TileManager::DispatchMoreTasks raster was not cheap");
+      }
+      cheap_raster_time_ += duration;
     } else {
       if (!CanDispatchRasterTask(tile))
         return;
@@ -633,8 +697,8 @@ void TileManager::OnImageDecodeTaskCompleted(
   DispatchMoreTasks();
 }
 
-void TileManager::DispatchOneRasterTask(scoped_refptr<Tile> tile) {
-  TRACE_EVENT0("cc", "TileManager::DispatchOneRasterTask");
+scoped_ptr<ResourcePool::Resource> TileManager::PrepareTileForRaster(
+    Tile* tile) {
   ManagedTileState& managed_tile_state = tile->managed_state();
   DCHECK(managed_tile_state.can_use_gpu_memory);
   scoped_ptr<ResourcePool::Resource> resource =
@@ -645,12 +709,17 @@ void TileManager::DispatchOneRasterTask(scoped_refptr<Tile> tile) {
   managed_tile_state.can_be_freed = false;
 
   DidTileRasterStateChange(tile, RASTER_STATE);
+  return resource.Pass();
+}
 
+void TileManager::DispatchOneRasterTask(scoped_refptr<Tile> tile) {
+  TRACE_EVENT0("cc", "TileManager::DispatchOneRasterTask");
+  scoped_ptr<ResourcePool::Resource> resource = PrepareTileForRaster(tile);
   ResourceProvider::ResourceId resource_id = resource->id();
 
   raster_worker_pool_->PostRasterTaskAndReply(
       tile->picture_pile(),
-      base::Bind(&TileManager::RunRasterTask,
+      base::Bind(&TileManager::PerformRaster,
                  resource_pool_->resource_provider()->mapPixelBuffer(
                      resource_id),
                  tile->content_rect_,
@@ -662,11 +731,25 @@ void TileManager::DispatchOneRasterTask(scoped_refptr<Tile> tile) {
                  manage_tiles_call_count_));
 }
 
-void TileManager::OnRasterTaskCompleted(
+void TileManager::PerformOneRaster(Tile* tile) {
+  scoped_ptr<ResourcePool::Resource> resource = PrepareTileForRaster(tile);
+  ResourceProvider::ResourceId resource_id = resource->id();
+
+  PerformRaster(resource_pool_->resource_provider()->mapPixelBuffer(
+                    resource_id),
+                tile->content_rect_,
+                tile->contents_scale(),
+                tile->picture_pile(),
+                &rendering_stats_);
+
+  OnRasterCompleted(tile, resource.Pass(), manage_tiles_call_count_);
+}
+
+void TileManager::OnRasterCompleted(
     scoped_refptr<Tile> tile,
     scoped_ptr<ResourcePool::Resource> resource,
     int manage_tiles_call_count_when_dispatched) {
-  TRACE_EVENT0("cc", "TileManager::OnRasterTaskCompleted");
+  TRACE_EVENT0("cc", "TileManager::OnRasterCompleted");
 
   // Release raster resources.
   resource_pool_->resource_provider()->unmapPixelBuffer(resource->id());
@@ -707,7 +790,14 @@ void TileManager::OnRasterTaskCompleted(
     managed_tile_state.resource_is_being_initialized = false;
     DidTileRasterStateChange(tile, IDLE_STATE);
   }
+}
 
+void TileManager::OnRasterTaskCompleted(
+    scoped_refptr<Tile> tile,
+    scoped_ptr<ResourcePool::Resource> resource,
+    int manage_tiles_call_count_when_dispatched) {
+  OnRasterCompleted(tile, resource.Pass(),
+                    manage_tiles_call_count_when_dispatched);
   DispatchMoreTasks();
 }
 
@@ -750,12 +840,12 @@ void TileManager::DidTileBinChange(Tile* tile,
 }
 
 // static
-void TileManager::RunRasterTask(uint8* buffer,
+void TileManager::PerformRaster(uint8* buffer,
                                 const gfx::Rect& rect,
                                 float contents_scale,
                                 PicturePileImpl* picture_pile,
                                 RenderingStats* stats) {
-  TRACE_EVENT0("cc", "TileManager::RunRasterTask");
+  TRACE_EVENT0("cc", "TileManager::PerformRaster");
   DCHECK(picture_pile);
   DCHECK(buffer);
   SkBitmap bitmap;