diff options
author | reveman <reveman@chromium.org> | 2014-10-29 14:04:40 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2014-10-29 21:04:53 +0000 |
commit | 51883faa0a6104c390251f57ebc794f714cf98d0 (patch) | |
tree | e287b106ca95f8a6b799a3dd061107796dbfdf1a | |
parent | cae3ac4afe15aca930e55c72cc929a7b830be2db (diff) | |
download | chromium_src-51883faa0a6104c390251f57ebc794f714cf98d0.zip chromium_src-51883faa0a6104c390251f57ebc794f714cf98d0.tar.gz chromium_src-51883faa0a6104c390251f57ebc794f714cf98d0.tar.bz2 |
cc: Detect miss-behaving fence extensions when using 1-copy rasterizer.
This adds a wait_if_needed parameter to
ResourcePool::CheckBusyResources. CheckBusyResources will use existing
ResourceProvider API to wait for any read locks to complete if needed
by a resource prior to checking if it's still busy when this parameter
is true.
1-copy rasterizer will use this functionality after too many failed
attempts to check if copy operations have completed. This provides a
reliable method to detect drivers with miss-behaving fence extensions.
BUG=406404
Review URL: https://codereview.chromium.org/683263004
Cr-Commit-Position: refs/heads/master@{#301926}
-rw-r--r-- | cc/resources/one_copy_raster_worker_pool.cc | 32 | ||||
-rw-r--r-- | cc/resources/one_copy_raster_worker_pool.h | 5 | ||||
-rw-r--r-- | cc/resources/resource_pool.cc | 5 | ||||
-rw-r--r-- | cc/resources/resource_pool.h | 5 | ||||
-rw-r--r-- | cc/resources/tile_manager.cc | 2 | ||||
-rw-r--r-- | cc/trees/layer_tree_host_impl.cc | 6 |
6 files changed, 39 insertions, 16 deletions
diff --git a/cc/resources/one_copy_raster_worker_pool.cc b/cc/resources/one_copy_raster_worker_pool.cc index bbf3136..c803262 100644 --- a/cc/resources/one_copy_raster_worker_pool.cc +++ b/cc/resources/one_copy_raster_worker_pool.cc @@ -87,6 +87,10 @@ const int kMaxCopyOperations = 16; // Delay been checking for copy operations to complete. const int kCheckForCompletedCopyOperationsTickRateMs = 1; +// Number of failed attempts to allow before we perform a check that will +// wait for copy operations to complete if needed. +const int kFailedAttemptsBeforeWaitIfNeeded = 256; + } // namespace OneCopyRasterWorkerPool::CopyOperation::CopyOperation( @@ -195,7 +199,7 @@ void OneCopyRasterWorkerPool::ScheduleTasks(RasterTaskQueue* queue) { task_set)); } - resource_pool_->CheckBusyResources(); + resource_pool_->CheckBusyResources(false); for (RasterTaskQueue::Item::Vector::const_iterator it = queue->items.begin(); it != queue->items.end(); @@ -283,15 +287,22 @@ OneCopyRasterWorkerPool::PlaybackAndScheduleCopyOnWorkerThread( { base::AutoLock lock(lock_); + int failed_attempts = 0; while ((scheduled_copy_operation_count_ + issued_copy_operation_count_) >= kMaxCopyOperations) { // Ignore limit when shutdown is set. if (shutdown_) break; + ++failed_attempts; + + // Schedule a check that will also wait for operations to complete + // after too many failed attempts. + bool wait_if_needed = failed_attempts > kFailedAttemptsBeforeWaitIfNeeded; + // Schedule a check for completed copy operations if too many operations // are currently in-flight. - ScheduleCheckForCompletedCopyOperationsWithLockAcquired(); + ScheduleCheckForCompletedCopyOperationsWithLockAcquired(wait_if_needed); { TRACE_EVENT0("cc", "WaitingForCopyOperationsToComplete"); @@ -420,7 +431,8 @@ void OneCopyRasterWorkerPool::IssueCopyOperations(int64 count) { } void OneCopyRasterWorkerPool:: - ScheduleCheckForCompletedCopyOperationsWithLockAcquired() { + ScheduleCheckForCompletedCopyOperationsWithLockAcquired( + bool wait_if_needed) { lock_.AssertAcquired(); if (check_for_completed_copy_operations_pending_) @@ -440,7 +452,8 @@ void OneCopyRasterWorkerPool:: task_runner_->PostDelayedTask( FROM_HERE, base::Bind(&OneCopyRasterWorkerPool::CheckForCompletedCopyOperations, - weak_ptr_factory_.GetWeakPtr()), + weak_ptr_factory_.GetWeakPtr(), + wait_if_needed), next_check_for_completed_copy_operations_time - now); last_check_for_completed_copy_operations_time_ = @@ -448,11 +461,14 @@ void OneCopyRasterWorkerPool:: check_for_completed_copy_operations_pending_ = true; } -void OneCopyRasterWorkerPool::CheckForCompletedCopyOperations() { - TRACE_EVENT0("cc", - "OneCopyRasterWorkerPool::CheckForCompletedCopyOperations"); +void OneCopyRasterWorkerPool::CheckForCompletedCopyOperations( + bool wait_if_needed) { + TRACE_EVENT1("cc", + "OneCopyRasterWorkerPool::CheckForCompletedCopyOperations", + "wait_if_needed", + wait_if_needed); - resource_pool_->CheckBusyResources(); + resource_pool_->CheckBusyResources(wait_if_needed); { base::AutoLock lock(lock_); diff --git a/cc/resources/one_copy_raster_worker_pool.h b/cc/resources/one_copy_raster_worker_pool.h index 59c4236..1dc102f 100644 --- a/cc/resources/one_copy_raster_worker_pool.h +++ b/cc/resources/one_copy_raster_worker_pool.h @@ -94,8 +94,9 @@ class CC_EXPORT OneCopyRasterWorkerPool : public RasterWorkerPool, void OnRasterFinished(TaskSet task_set); void AdvanceLastFlushedCopyTo(CopySequenceNumber sequence); void IssueCopyOperations(int64 count); - void ScheduleCheckForCompletedCopyOperationsWithLockAcquired(); - void CheckForCompletedCopyOperations(); + void ScheduleCheckForCompletedCopyOperationsWithLockAcquired( + bool wait_if_needed); + void CheckForCompletedCopyOperations(bool wait_if_needed); scoped_refptr<base::debug::ConvertableToTraceFormat> StateAsValue() const; void StagingStateAsValueInto(base::debug::TracedValue* staging_state) const; diff --git a/cc/resources/resource_pool.cc b/cc/resources/resource_pool.cc index 31b9b27..45f36e2 100644 --- a/cc/resources/resource_pool.cc +++ b/cc/resources/resource_pool.cc @@ -105,12 +105,15 @@ bool ResourcePool::ResourceUsageTooHigh() { return false; } -void ResourcePool::CheckBusyResources() { +void ResourcePool::CheckBusyResources(bool wait_if_needed) { ResourceList::iterator it = busy_resources_.begin(); while (it != busy_resources_.end()) { ScopedResource* resource = *it; + if (wait_if_needed) + resource_provider_->WaitReadLockIfNeeded(resource->id()); + if (resource_provider_->CanLockForWrite(resource->id())) { DidFinishUsingResource(resource); it = busy_resources_.erase(it); diff --git a/cc/resources/resource_pool.h b/cc/resources/resource_pool.h index 102b242..e1ee35a 100644 --- a/cc/resources/resource_pool.h +++ b/cc/resources/resource_pool.h @@ -34,7 +34,10 @@ class CC_EXPORT ResourcePool { size_t max_resource_count); void ReduceResourceUsage(); - void CheckBusyResources(); + // This might block if |wait_if_needed| is true and one of the currently + // busy resources has a read lock fence that needs to be waited upon before + // it can be locked for write again. + void CheckBusyResources(bool wait_if_needed); size_t total_memory_usage_bytes() const { return memory_usage_bytes_; } size_t acquired_memory_usage_bytes() const { diff --git a/cc/resources/tile_manager.cc b/cc/resources/tile_manager.cc index 86874e7..c785cf6 100644 --- a/cc/resources/tile_manager.cc +++ b/cc/resources/tile_manager.cc @@ -541,7 +541,7 @@ void TileManager::AssignGpuMemoryToTiles( // If this operation becomes expensive too, only do this after some // resource(s) was returned. Note that in that case, one also need to // invalidate when releasing some resource from the pool. - resource_pool_->CheckBusyResources(); + resource_pool_->CheckBusyResources(false); // Now give memory out to the tiles until we're out, and build // the needs-to-be-rasterized queue. diff --git a/cc/trees/layer_tree_host_impl.cc b/cc/trees/layer_tree_host_impl.cc index acfa6bb..a13254f 100644 --- a/cc/trees/layer_tree_host_impl.cc +++ b/cc/trees/layer_tree_host_impl.cc @@ -1232,7 +1232,7 @@ void LayerTreeHostImpl::UpdateTileManagerMemoryPolicy( 100); DCHECK(resource_pool_); - resource_pool_->CheckBusyResources(); + resource_pool_->CheckBusyResources(false); // Soft limit is used for resource pool such that memory returns to soft // limit after going over. resource_pool_->SetResourceUsageLimits( @@ -1242,7 +1242,7 @@ void LayerTreeHostImpl::UpdateTileManagerMemoryPolicy( // Release all staging resources when invisible. if (staging_resource_pool_) { - staging_resource_pool_->CheckBusyResources(); + staging_resource_pool_->CheckBusyResources(false); staging_resource_pool_->SetResourceUsageLimits( std::numeric_limits<size_t>::max(), std::numeric_limits<size_t>::max(), @@ -1454,7 +1454,7 @@ void LayerTreeHostImpl::ReclaimResources(const CompositorFrameAck* ack) { if (tile_manager_) { DCHECK(resource_pool_); - resource_pool_->CheckBusyResources(); + resource_pool_->CheckBusyResources(false); resource_pool_->ReduceResourceUsage(); } // If we're not visible, we likely released resources, so we want to |