Delay activation/draw on GPU tile work completion

cc/tiles/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/tiles/tile_manager.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <algorithm>
@@ skipping 350 matching lines @@
           task_runner_,
           base::Bind(&TileManager::CheckIfMoreTilesNeedToBePrepared,
                      base::Unretained(this))),
       signals_check_notifier_(task_runner_,
                               base::Bind(&TileManager::CheckAndIssueSignals,
                                          base::Unretained(this))),
       has_scheduled_tile_tasks_(false),
       prepare_tiles_count_(0u),
       next_tile_id_(0u),
       check_tile_priority_inversion_(check_tile_priority_inversion),
-      task_set_finished_weak_ptr_factory_(this) {}
+      task_set_finished_weak_ptr_factory_(this),
+      ready_to_draw_callback_weak_ptr_factory_(this) {}
 
 TileManager::~TileManager() {
   FinishTasksAndCleanUp();
 }
 
 void TileManager::FinishTasksAndCleanUp() {
   if (!tile_task_manager_)
     return;
 
   global_state_ = GlobalStateThatImpactsTilePriority();
 
   // This cancels tasks if possible, finishes pending tasks, and release any
   // uninitialized resources.
   tile_task_manager_->Shutdown();
 
   raster_buffer_provider_->Shutdown();
 
   tile_task_manager_->CheckForCompletedTasks();
 
   FreeResourcesForReleasedTiles();
   CleanUpReleasedTiles();
 
   tile_task_manager_ = nullptr;
   resource_pool_ = nullptr;
   more_tiles_need_prepare_check_notifier_.Cancel();
   signals_check_notifier_.Cancel();
   task_set_finished_weak_ptr_factory_.InvalidateWeakPtrs();
+  ready_to_draw_callback_weak_ptr_factory_.InvalidateWeakPtrs();
   raster_buffer_provider_ = nullptr;
 
   image_controller_.SetImageDecodeCache(nullptr);
   locked_image_tasks_.clear();
 }
 
 void TileManager::SetResources(ResourcePool* resource_pool,
                                ImageDecodeCache* image_decode_cache,
                                TaskGraphRunner* task_graph_runner,
                                RasterBufferProvider* raster_buffer_provider,
                                size_t scheduled_raster_task_limit,
                                bool use_gpu_rasterization) {
   DCHECK(!tile_task_manager_);
   DCHECK(task_graph_runner);
 
   use_gpu_rasterization_ = use_gpu_rasterization;
   scheduled_raster_task_limit_ = scheduled_raster_task_limit;
   resource_pool_ = resource_pool;
   image_controller_.SetImageDecodeCache(image_decode_cache);
   tile_task_manager_ = TileTaskManagerImpl::Create(task_graph_runner);
   raster_buffer_provider_ = raster_buffer_provider;
 }
 
 void TileManager::Release(Tile* tile) {
   released_tiles_.push_back(tile);
+  // The |tile|'s owning PictureLayerTiling has released ownership and turned
+  // the

[vmpstr, 2017/01/05 22:00:46]

Reformat plz

[ericrk, 2017/01/09 23:05:21]

Done.

+  // |tile| over to TileManager for deletion.
+  tile->set_tiling(nullptr);
 }
 
 void TileManager::FreeResourcesForReleasedTiles() {
   for (auto* tile : released_tiles_)
     FreeResourcesForTile(tile);
 }
 
 void TileManager::CleanUpReleasedTiles() {
   std::vector<Tile*> tiles_to_retain;
   for (auto* tile : released_tiles_) {
     if (tile->HasRasterTask()) {

[vmpstr, 2017/01/05 22:00:46]

Can you file a bug to clean this up please? We changed the raster task
ownership, so afaik we don't actually need to retain the tiles since the task
will still be alive even if it's pending completion/rasterization.

Or am I missing stuff again?

[ericrk, 2017/01/09 23:05:21]

In RasterTaskCompleted, we use/dereference tile, so we'd need to do a bit more
work to handle this safely. I had started writing a patch, but got sidetracked.
I did open a bug here: crbug.com/672606, I'll add a TODO.

       tiles_to_retain.push_back(tile);
       continue;
     }
 
     DCHECK(!tile->draw_info().has_resource());
     DCHECK(tiles_.find(tile->id()) != tiles_.end());
     tiles_.erase(tile->id());
+    pending_ready_for_draw_tiles_.erase(tile);
 
     delete tile;
   }
   released_tiles_.swap(tiles_to_retain);
 }
 
 void TileManager::DidFinishRunningTileTasksRequiredForActivation() {
   TRACE_EVENT0("cc",
                "TileManager::DidFinishRunningTileTasksRequiredForActivation");
   TRACE_EVENT_ASYNC_STEP_INTO1("cc", "ScheduledTasks", this, "running", "state",
@@ skipping 56 matching lines @@
   // We need to call CheckForCompletedTasks() once in-between each call
   // to ScheduleTasks() to prevent canceled tasks from being scheduled.
   if (!did_check_for_completed_tasks_since_last_schedule_tasks_) {
     tile_task_manager_->CheckForCompletedTasks();
     did_check_for_completed_tasks_since_last_schedule_tasks_ = true;
   }
 
   FreeResourcesForReleasedTiles();
   CleanUpReleasedTiles();
 
+  // The required for activate/draw status of tiles may have changed any time
+  // PrepareTiles is called.
+  pending_tile_requirements_dirty_ = true;
+
   PrioritizedWorkToSchedule prioritized_work = AssignGpuMemoryToTiles();
 
   // Inform the client that will likely require a draw if the highest priority
   // tile that will be rasterized is required for draw.
   client_->SetIsLikelyToRequireADraw(
       !prioritized_work.tiles_to_raster.empty() &&
       prioritized_work.tiles_to_raster.front().tile()->required_for_draw());
 
   // Schedule tile tasks.
   ScheduleTasks(prioritized_work);
 
   TRACE_EVENT_INSTANT1("cc", "DidPrepareTiles", TRACE_EVENT_SCOPE_THREAD,
                        "state", BasicStateAsValue());
   return true;
 }
 
 void TileManager::Flush() {
   TRACE_EVENT0("cc", "TileManager::Flush");
 
   if (!tile_task_manager_) {
     TRACE_EVENT_INSTANT0("cc", "Flush aborted", TRACE_EVENT_SCOPE_THREAD);
     return;
   }
 
   tile_task_manager_->CheckForCompletedTasks();
   did_check_for_completed_tasks_since_last_schedule_tasks_ = true;
+  CheckPendingReadyToDrawTiles(true /* issue_signals */);
 
   TRACE_EVENT_INSTANT1("cc", "DidFlush", TRACE_EVENT_SCOPE_THREAD, "stats",
                        RasterTaskCompletionStatsAsValue(flush_stats_));
   flush_stats_ = RasterTaskCompletionStats();
 }
 
 std::unique_ptr<base::trace_event::ConvertableToTraceFormat>
 TileManager::BasicStateAsValue() const {
   std::unique_ptr<base::trace_event::TracedValue> value(
       new base::trace_event::TracedValue());
@@ skipping 123 matching lines @@
       // or drop the tile for scheduling and raster.
       // TODO(sohanjg): Check if we could use a shared analysis
       // canvas which is reset between tiles.
       tile->set_solid_color_analysis_performed(true);
       SkColor color = SK_ColorTRANSPARENT;
       bool is_solid_color =
           prioritized_tile.raster_source()->PerformSolidColorAnalysis(
               tile->content_rect(), tile->raster_scales(), &color);
       if (is_solid_color) {
         tile->draw_info().set_solid_color(color);
-        tile->draw_info().set_was_ever_ready_to_draw();
         if (!tile_is_needed_now)
           tile->draw_info().set_was_a_prepaint_tile();
         client_->NotifyTileStateChanged(tile);
         continue;
       }
     }
 
     // Prepaint tiles that are far away are only processed for images.
     if (!tile->required_for_activation() && !tile->required_for_draw() &&
         prioritized_tile.is_process_for_images_only()) {
@@ skipping 112 matching lines @@
   TRACE_EVENT_END2("cc", "TileManager::AssignGpuMemoryToTiles",
                    "all_tiles_that_need_to_be_rasterized_are_scheduled",
                    all_tiles_that_need_to_be_rasterized_are_scheduled_,
                    "had_enough_memory_to_schedule_tiles_needed_now",
                    had_enough_memory_to_schedule_tiles_needed_now);
   return work_to_schedule;
 }
 
 void TileManager::FreeResourcesForTile(Tile* tile) {
   TileDrawInfo& draw_info = tile->draw_info();
-  if (draw_info.resource_) {
-    resource_pool_->ReleaseResource(draw_info.resource_);
-    draw_info.resource_ = nullptr;
-  }
+  Resource* resource = draw_info.TakeResource();
+  if (resource)
+    resource_pool_->ReleaseResource(resource);
 }
 
 void TileManager::FreeResourcesForTileAndNotifyClientIfTileWasReadyToDraw(
     Tile* tile) {
   bool was_ready_to_draw = tile->draw_info().IsReadyToDraw();
   FreeResourcesForTile(tile);
   if (was_ready_to_draw)
     client_->NotifyTileStateChanged(tile);
 }
 
@@ skipping 37 matching lines @@
           &TileManager::DidFinishRunningTileTasksRequiredForDraw);
   scoped_refptr<TileTask> all_done_task =
       CreateTaskSetFinishedTask(&TileManager::DidFinishRunningAllTileTasks);
 
   // Build a new task queue containing all task currently needed. Tasks
   // are added in order of priority, highest priority task first.
   for (auto& prioritized_tile : tiles_that_need_to_be_rasterized) {
     Tile* tile = prioritized_tile.tile();
 
     DCHECK(tile->draw_info().requires_resource());
-    DCHECK(!tile->draw_info().resource_);
+    DCHECK(!tile->draw_info().resource());
 
     if (!tile->raster_task_)
       tile->raster_task_ = CreateRasterTask(
           prioritized_tile,
           prioritized_tile.raster_source()->HasImpliedColorSpace()
               ? prioritized_tile.raster_source()->GetImpliedColorSpace()
               : target_color_space);
 
     TileTask* task = tile->raster_task_.get();
 
@@ skipping 175 matching lines @@
 
   if (was_canceled) {
     ++flush_stats_.canceled_count;
     resource_pool_->ReleaseResource(resource);
     return;
   }
 
   resource_pool_->OnContentReplaced(resource->id(), tile->id());
   ++flush_stats_.completed_count;
 
-  draw_info.set_use_resource();
-  draw_info.resource_ = resource;
+  draw_info.set_resource(resource);
   draw_info.contents_swizzled_ = DetermineResourceRequiresSwizzle(tile);
-
-  DCHECK(draw_info.IsReadyToDraw());
-  draw_info.set_was_ever_ready_to_draw();
-
-  client_->NotifyTileStateChanged(tile);
+  if (global_state_.tree_priority == SMOOTHNESS_TAKES_PRIORITY &&
+      !raster_buffer_provider_->IsResourceReadyToDraw(resource->id())) {
+    // In SMOOTHNESS_TAKES_PRIORITY mode, we wait for GPU work to complete

[vmpstr, 2017/01/05 22:00:46]

Move this comment outside of the if plz

Also consider doing this pattern, but it's really up to you.

// If SMOOTHNESS ...
if (...) {
 pending_ready_for_draw_tiles_.insert(tile);
 return;
}

draw_info.set_resource_ready_for_draw();
client_->NotifyTileStateChanged(tile);

[ericrk, 2017/01/09 23:05:21]

good suggestion.

+    // for a tile before setting it as ready to draw.
+    pending_ready_for_draw_tiles_.insert(tile);
+  } else {
+    draw_info.set_resource_ready_for_draw();
+    client_->NotifyTileStateChanged(tile);
+  }
 }
 
 ScopedTilePtr TileManager::CreateTile(const Tile::CreateInfo& info,
                                       int layer_id,
                                       int source_frame_number,
                                       int flags) {
   // We need to have a tile task worker pool to do anything meaningful with
   // tiles.
   DCHECK(tile_task_manager_);
   ScopedTilePtr tile(
@@ skipping 28 matching lines @@
     Tile* tile = all_queue->Top().tile();
     DCHECK(!tile->required_for_activation() ||
            tile->draw_info().IsReadyToDraw());
   }
 #endif
   return true;
 }
 
 bool TileManager::IsReadyToActivate() const {
   TRACE_EVENT0("cc", "TileManager::IsReadyToActivate");
-  return AreRequiredTilesReadyToDraw(
-      RasterTilePriorityQueue::Type::REQUIRED_FOR_ACTIVATION);
+  return pending_required_for_activation_callback_id_ == 0 &&
+         AreRequiredTilesReadyToDraw(
+             RasterTilePriorityQueue::Type::REQUIRED_FOR_ACTIVATION);
 }
 
 bool TileManager::IsReadyToDraw() const {
   TRACE_EVENT0("cc", "TileManager::IsReadyToDraw");
-  return AreRequiredTilesReadyToDraw(
-      RasterTilePriorityQueue::Type::REQUIRED_FOR_DRAW);
+  return pending_required_for_draw_callback_id_ == 0 &&
+         AreRequiredTilesReadyToDraw(
+             RasterTilePriorityQueue::Type::REQUIRED_FOR_DRAW);
 }
 
 void TileManager::CheckAndIssueSignals() {
   TRACE_EVENT0("cc", "TileManager::CheckAndIssueSignals");
   tile_task_manager_->CheckForCompletedTasks();
   did_check_for_completed_tasks_since_last_schedule_tasks_ = true;
 
+  CheckPendingReadyToDrawTiles(false /* issue_signals */);
+
   // Ready to activate.
   if (signals_.ready_to_activate && !signals_.did_notify_ready_to_activate) {
     signals_.ready_to_activate = false;
     if (IsReadyToActivate()) {
       TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("cc.debug"),
                    "TileManager::CheckAndIssueSignals - ready to activate");
       signals_.did_notify_ready_to_activate = true;
       client_->NotifyReadyToActivate();
     }
   }
@@ skipping 133 matching lines @@
                     signals_.all_tile_tasks_completed);
   state->EndDictionary();
   return std::move(state);
 }
 
 bool TileManager::UsePartialRaster() const {
   return use_partial_raster_ &&
          raster_buffer_provider_->CanPartialRasterIntoProvidedResource();
 }
 
+void TileManager::CheckPendingReadyToDrawTiles(bool issue_signals) {
+  bool in_smoothness_mode =
+      global_state_.tree_priority == SMOOTHNESS_TAKES_PRIORITY;
+  ResourceProvider::ResourceIdArray required_for_activation_ids;
+  ResourceProvider::ResourceIdArray required_for_draw_ids;
+
+  for (auto it = pending_ready_for_draw_tiles_.begin();
+       it != pending_ready_for_draw_tiles_.end();) {
+    Tile* tile = *it;
+    const Resource* resource = tile->draw_info().resource();
+    if (!tile->tiling()) {
+      // The tile has been evicted or deleted. Remove it from our list.

[vmpstr, 2017/01/05 22:00:46]

nit: or is pending deletion. (you remove it when it's proper deleted)

[ericrk, 2017/01/09 23:05:22]

now that tile deletion has been cleaned up, this is gone.

+      it = pending_ready_for_draw_tiles_.erase(it);
+      continue;
+    }
+
+    if (!in_smoothness_mode ||

[vmpstr, 2017/01/05 22:00:46]

You can just do the gobal_state_ == ... check here and remove the variable.

[ericrk, 2017/01/09 23:05:21]

Done.

+        raster_buffer_provider_->IsResourceReadyToDraw(resource->id())) {
+      // Tile is ready to draw.
+      tile->draw_info().set_resource_ready_for_draw();
+      client_->NotifyTileStateChanged(tile);
+      it = pending_ready_for_draw_tiles_.erase(it);
+      continue;
+    }
+
+    if (pending_tile_requirements_dirty_)

[vmpstr, 2017/01/05 22:00:46]

I know we talked about this a bit, but is there a race here?
UpdateTilePriorities might have run and the scheduler is pending a PrepareTiles,
but one hasn't happened yet. Is that possible? What are the implications? It
seems that some times that _are_ now required might be missed.

[ericrk, 2017/01/09 23:05:21]

Ok, I think there was a race here (mainly because of the scheduler sometimes
skipping PrepareTiles) - added a TileManager::DidModifyTilePriorities to handle
this.

+      tile->tiling()->UpdateRequiredStatesOnTile(tile);
+    if (tile->required_for_activation())
+      required_for_activation_ids.push_back(resource->id());
+    if (tile->required_for_draw())
+      required_for_draw_ids.push_back(resource->id());
+
+    ++it;
+  }
+
+  if (required_for_activation_ids.empty()) {
+    pending_required_for_activation_callback_id_ = 0;

[vmpstr, 2017/01/05 22:00:46]

Maybe

pending_required_for_activation_callback_id_ = 0;
if (!required_for_activation_ids.empty()) {
  ...
}

WDYT?

[ericrk, 2017/01/09 23:05:21]

Done.

+  } else {
+    pending_required_for_activation_callback_id_ =
+        raster_buffer_provider_->SetReadyToDrawCallback(
+            required_for_activation_ids,
+            base::Bind(&TileManager::CheckPendingReadyToDrawTiles,
+                       ready_to_draw_callback_weak_ptr_factory_.GetWeakPtr(),
+                       true /* issue_signals */),
+            pending_required_for_activation_callback_id_);
+  }
+
+  if (required_for_draw_ids.empty()) {
+    pending_required_for_draw_callback_id_ = 0;
+  } else {
+    pending_required_for_draw_callback_id_ =
+        raster_buffer_provider_->SetReadyToDrawCallback(
+            required_for_draw_ids,
+            base::Bind(&TileManager::CheckPendingReadyToDrawTiles,
+                       ready_to_draw_callback_weak_ptr_factory_.GetWeakPtr(),
+                       true /* issue_signals */),
+            pending_required_for_draw_callback_id_);
+  }
+
+  // Update our signals now that we know whether we have pending resources.
+  signals_.ready_to_activate =
+      (pending_required_for_activation_callback_id_ == 0);
+  signals_.ready_to_draw = (pending_required_for_draw_callback_id_ == 0);
+
+  if (issue_signals && (signals_.ready_to_activate || signals_.ready_to_draw))
+    signals_check_notifier_.Schedule();
+
+  // We've just updated all pending tile requirements if necessary.
+  pending_tile_requirements_dirty_ = false;
+}
+
 // Utility function that can be used to create a "Task set finished" task that
 // posts |callback| to |task_runner| when run.
 scoped_refptr<TileTask> TileManager::CreateTaskSetFinishedTask(
     void (TileManager::*callback)()) {
   return make_scoped_refptr(new TaskSetFinishedTaskImpl(
       task_runner_,
       base::Bind(callback, task_set_finished_weak_ptr_factory_.GetWeakPtr())));
 }
 
 TileManager::MemoryUsage::MemoryUsage()
     : memory_bytes_(0), resource_count_(0) {}
 
 TileManager::MemoryUsage::MemoryUsage(size_t memory_bytes,
                                       size_t resource_count)
     : memory_bytes_(static_cast<int64_t>(memory_bytes)),
       resource_count_(static_cast<int>(resource_count)) {
   // MemoryUsage is constructed using size_ts, since it deals with memory and
   // the inputs are typically size_t. However, during the course of usage (in
-  // particular operator-=) can cause internal values to become negative. Thus,
-  // member variables are signed.
+  // particular operator-=) can cause internal values to become negative.
+  // Thus, member variables are signed.
   DCHECK_LE(memory_bytes,
             static_cast<size_t>(std::numeric_limits<int64_t>::max()));
   DCHECK_LE(resource_count,
             static_cast<size_t>(std::numeric_limits<int>::max()));
 }
 
 // static
 TileManager::MemoryUsage TileManager::MemoryUsage::FromConfig(
     const gfx::Size& size,
     ResourceFormat format) {
   // We can use UncheckedSizeInBytes here since this is used with a tile
-  // size which is determined by the compositor (it's at most max texture size).
+  // size which is determined by the compositor (it's at most max texture
+  // size).
   return MemoryUsage(ResourceUtil::UncheckedSizeInBytes<size_t>(size, format),
                      1);
 }
 
 // static
 TileManager::MemoryUsage TileManager::MemoryUsage::FromTile(const Tile* tile) {
   const TileDrawInfo& draw_info = tile->draw_info();
-  if (draw_info.resource_) {
-    return MemoryUsage::FromConfig(draw_info.resource_->size(),
-                                   draw_info.resource_->format());
+  if (draw_info.resource()) {
+    return MemoryUsage::FromConfig(draw_info.resource()->size(),
+                                   draw_info.resource()->format());
   }
   return MemoryUsage();
 }
 
 TileManager::MemoryUsage& TileManager::MemoryUsage::operator+=(
     const MemoryUsage& other) {
   memory_bytes_ += other.memory_bytes_;
   resource_count_ += other.resource_count_;
   return *this;
 }
@@ skipping 29 matching lines @@
   all_tile_tasks_completed = false;
   did_notify_all_tile_tasks_completed = false;
 }
 
 TileManager::PrioritizedWorkToSchedule::PrioritizedWorkToSchedule() = default;
 TileManager::PrioritizedWorkToSchedule::PrioritizedWorkToSchedule(
     PrioritizedWorkToSchedule&& other) = default;
 TileManager::PrioritizedWorkToSchedule::~PrioritizedWorkToSchedule() = default;
 
 }  // namespace cc