Commit e898b99e authored by Khushal's avatar Khushal Committed by Commit Bot

cc: Rate limit raster metric queries.

Since queries for measuring GPU side raster duration is too expensive,
rate limit the tiles for which we measure this metric to 1%.

R=piman@chromium.org

Bug: 894200,896821
Change-Id: I0713e537447a8ca7ebec30747314810e1a881d6e
Reviewed-on: https://chromium-review.googlesource.com/c/1292110Reviewed-by: default avatarAntoine Labour <piman@chromium.org>
Commit-Queue: Khushal <khushalsagar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601319}
parent 432f4e5b
...@@ -337,7 +337,8 @@ GpuRasterBufferProvider::GpuRasterBufferProvider( ...@@ -337,7 +337,8 @@ GpuRasterBufferProvider::GpuRasterBufferProvider(
viz::ResourceFormat tile_format, viz::ResourceFormat tile_format,
const gfx::Size& max_tile_size, const gfx::Size& max_tile_size,
bool unpremultiply_and_dither_low_bit_depth_tiles, bool unpremultiply_and_dither_low_bit_depth_tiles,
bool enable_oop_rasterization) bool enable_oop_rasterization,
int raster_metric_frequency)
: compositor_context_provider_(compositor_context_provider), : compositor_context_provider_(compositor_context_provider),
worker_context_provider_(worker_context_provider), worker_context_provider_(worker_context_provider),
use_gpu_memory_buffer_resources_(use_gpu_memory_buffer_resources), use_gpu_memory_buffer_resources_(use_gpu_memory_buffer_resources),
...@@ -346,7 +347,8 @@ GpuRasterBufferProvider::GpuRasterBufferProvider( ...@@ -346,7 +347,8 @@ GpuRasterBufferProvider::GpuRasterBufferProvider(
max_tile_size_(max_tile_size), max_tile_size_(max_tile_size),
unpremultiply_and_dither_low_bit_depth_tiles_( unpremultiply_and_dither_low_bit_depth_tiles_(
unpremultiply_and_dither_low_bit_depth_tiles), unpremultiply_and_dither_low_bit_depth_tiles),
enable_oop_rasterization_(enable_oop_rasterization) { enable_oop_rasterization_(enable_oop_rasterization),
raster_metric_frequency_(raster_metric_frequency) {
DCHECK(compositor_context_provider); DCHECK(compositor_context_provider);
DCHECK(worker_context_provider); DCHECK(worker_context_provider);
} }
...@@ -464,7 +466,7 @@ gpu::SyncToken GpuRasterBufferProvider::PlaybackOnWorkerThread( ...@@ -464,7 +466,7 @@ gpu::SyncToken GpuRasterBufferProvider::PlaybackOnWorkerThread(
raster_dirty_rect, new_content_id, transform, playback_settings, url, raster_dirty_rect, new_content_id, transform, playback_settings, url,
&query); &query);
{ if (query.query_id != 0u) {
// Note that it is important to scope the raster context lock to // Note that it is important to scope the raster context lock to
// PlaybackOnWorkerThreadInternal and release it before acquiring this lock // PlaybackOnWorkerThreadInternal and release it before acquiring this lock
// to avoid a deadlock in CheckRasterFinishedQueries which acquires the // to avoid a deadlock in CheckRasterFinishedQueries which acquires the
...@@ -498,6 +500,13 @@ gpu::SyncToken GpuRasterBufferProvider::PlaybackOnWorkerThreadInternal( ...@@ -498,6 +500,13 @@ gpu::SyncToken GpuRasterBufferProvider::PlaybackOnWorkerThreadInternal(
gpu::raster::RasterInterface* ri = scoped_context.RasterInterface(); gpu::raster::RasterInterface* ri = scoped_context.RasterInterface();
DCHECK(ri); DCHECK(ri);
raster_tasks_count_++;
bool measure_raster_metric = false;
if (raster_tasks_count_ == raster_metric_frequency_) {
measure_raster_metric = true;
raster_tasks_count_ = 0;
}
gfx::Rect playback_rect = raster_full_rect; gfx::Rect playback_rect = raster_full_rect;
if (resource_has_previous_content) { if (resource_has_previous_content) {
playback_rect.Intersect(raster_dirty_rect); playback_rect.Intersect(raster_dirty_rect);
...@@ -519,12 +528,16 @@ gpu::SyncToken GpuRasterBufferProvider::PlaybackOnWorkerThreadInternal( ...@@ -519,12 +528,16 @@ gpu::SyncToken GpuRasterBufferProvider::PlaybackOnWorkerThreadInternal(
100.0f * fraction_saved); 100.0f * fraction_saved);
} }
// Use a query to time the GPU side work for rasterizing this tile. if (measure_raster_metric) {
ri->GenQueriesEXT(1, &query->query_id); // Use a query to time the GPU side work for rasterizing this tile.
ri->BeginQueryEXT(GL_COMMANDS_ISSUED_CHROMIUM, query->query_id); ri->GenQueriesEXT(1, &query->query_id);
ri->BeginQueryEXT(GL_COMMANDS_ISSUED_CHROMIUM, query->query_id);
}
{ {
base::ElapsedTimer timer; base::Optional<base::ElapsedTimer> timer;
if (measure_raster_metric)
timer.emplace();
if (enable_oop_rasterization_) { if (enable_oop_rasterization_) {
RasterizeSourceOOP(raster_source, resource_has_previous_content, mailbox, RasterizeSourceOOP(raster_source, resource_has_previous_content, mailbox,
sync_token, texture_target, sync_token, texture_target,
...@@ -542,7 +555,8 @@ gpu::SyncToken GpuRasterBufferProvider::PlaybackOnWorkerThreadInternal( ...@@ -542,7 +555,8 @@ gpu::SyncToken GpuRasterBufferProvider::PlaybackOnWorkerThreadInternal(
ShouldUnpremultiplyAndDitherResource(resource_format), ShouldUnpremultiplyAndDitherResource(resource_format),
max_tile_size_); max_tile_size_);
} }
query->worker_duration = timer.Elapsed(); if (measure_raster_metric)
query->worker_duration = timer->Elapsed();
} }
ri->EndQueryEXT(GL_COMMANDS_ISSUED_CHROMIUM); ri->EndQueryEXT(GL_COMMANDS_ISSUED_CHROMIUM);
......
...@@ -26,6 +26,7 @@ namespace cc { ...@@ -26,6 +26,7 @@ namespace cc {
class CC_EXPORT GpuRasterBufferProvider : public RasterBufferProvider { class CC_EXPORT GpuRasterBufferProvider : public RasterBufferProvider {
public: public:
static constexpr int kRasterMetricFrequency = 100;
GpuRasterBufferProvider(viz::ContextProvider* compositor_context_provider, GpuRasterBufferProvider(viz::ContextProvider* compositor_context_provider,
viz::RasterContextProvider* worker_context_provider, viz::RasterContextProvider* worker_context_provider,
bool use_gpu_memory_buffer_resources, bool use_gpu_memory_buffer_resources,
...@@ -33,7 +34,8 @@ class CC_EXPORT GpuRasterBufferProvider : public RasterBufferProvider { ...@@ -33,7 +34,8 @@ class CC_EXPORT GpuRasterBufferProvider : public RasterBufferProvider {
viz::ResourceFormat tile_format, viz::ResourceFormat tile_format,
const gfx::Size& max_tile_size, const gfx::Size& max_tile_size,
bool unpremultiply_and_dither_low_bit_depth_tiles, bool unpremultiply_and_dither_low_bit_depth_tiles,
bool enable_oop_rasterization); bool enable_oop_rasterization,
int raster_metric_frequency = kRasterMetricFrequency);
~GpuRasterBufferProvider() override; ~GpuRasterBufferProvider() override;
// Overridden from RasterBufferProvider: // Overridden from RasterBufferProvider:
...@@ -149,6 +151,7 @@ class CC_EXPORT GpuRasterBufferProvider : public RasterBufferProvider { ...@@ -149,6 +151,7 @@ class CC_EXPORT GpuRasterBufferProvider : public RasterBufferProvider {
const gfx::Size max_tile_size_; const gfx::Size max_tile_size_;
const bool unpremultiply_and_dither_low_bit_depth_tiles_; const bool unpremultiply_and_dither_low_bit_depth_tiles_;
const bool enable_oop_rasterization_; const bool enable_oop_rasterization_;
const int raster_metric_frequency_;
// Note that this lock should never be acquired while holding the raster // Note that this lock should never be acquired while holding the raster
// context lock. // context lock.
...@@ -156,6 +159,9 @@ class CC_EXPORT GpuRasterBufferProvider : public RasterBufferProvider { ...@@ -156,6 +159,9 @@ class CC_EXPORT GpuRasterBufferProvider : public RasterBufferProvider {
base::circular_deque<PendingRasterQuery> pending_raster_queries_ base::circular_deque<PendingRasterQuery> pending_raster_queries_
GUARDED_BY(pending_raster_queries_lock_); GUARDED_BY(pending_raster_queries_lock_);
// Accessed with the worker context lock acquired.
int raster_tasks_count_ = 0;
DISALLOW_COPY_AND_ASSIGN(GpuRasterBufferProvider); DISALLOW_COPY_AND_ASSIGN(GpuRasterBufferProvider);
}; };
......
...@@ -172,7 +172,7 @@ class RasterBufferProviderTest ...@@ -172,7 +172,7 @@ class RasterBufferProviderTest
Create3dResourceProvider(); Create3dResourceProvider();
raster_buffer_provider_ = std::make_unique<GpuRasterBufferProvider>( raster_buffer_provider_ = std::make_unique<GpuRasterBufferProvider>(
context_provider_.get(), worker_context_provider_.get(), false, 0, context_provider_.get(), worker_context_provider_.get(), false, 0,
viz::RGBA_8888, gfx::Size(), true, false); viz::RGBA_8888, gfx::Size(), true, false, 1);
break; break;
case RASTER_BUFFER_PROVIDER_TYPE_BITMAP: case RASTER_BUFFER_PROVIDER_TYPE_BITMAP:
CreateSoftwareResourceProvider(); CreateSoftwareResourceProvider();
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment