Raster delay UMA should exclude at-raster decodes
This CL fixes the RasterTaskSchedulingDelay UMA to not count raster
tasks that depend on at-raster image decodes. This is because measuring
the delay for them may or may not include the decode/upload time
depending on whether we do a software or a hardware decode. At-raster
image decodes should be rare enough that we don't need to consider them.
Bug: 995155
Test: this CL adds unit testing for the new behavior.
Change-Id: I040ed17f647033aeed3314c76e9ac81d5c5ed2ea
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1924613
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Khushal <khushalsagar@chromium.org>
Commit-Queue: Andres Calderon Jaramillo <andrescj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#718826}
diff --git a/cc/raster/gpu_raster_buffer_provider.cc b/cc/raster/gpu_raster_buffer_provider.cc
index 7e6dcf5..5ef906c 100644
--- a/cc/raster/gpu_raster_buffer_provider.cc
+++ b/cc/raster/gpu_raster_buffer_provider.cc
@@ -284,13 +284,15 @@
GpuRasterBufferProvider* client,
const ResourcePool::InUsePoolResource& in_use_resource,
GpuRasterBacking* backing,
- bool resource_has_previous_content)
+ bool resource_has_previous_content,
+ bool depends_on_at_raster_decodes)
: client_(client),
backing_(backing),
resource_size_(in_use_resource.size()),
resource_format_(in_use_resource.format()),
color_space_(in_use_resource.color_space()),
resource_has_previous_content_(resource_has_previous_content),
+ depends_on_at_raster_decodes_(depends_on_at_raster_decodes),
before_raster_sync_token_(backing->returned_sync_token),
texture_target_(backing->texture_target),
texture_is_overlay_candidate_(backing->overlay_candidate),
@@ -338,7 +340,7 @@
before_raster_sync_token_, resource_size_, resource_format_, color_space_,
resource_has_previous_content_, raster_source, raster_full_rect,
raster_dirty_rect, new_content_id, transform, playback_settings, url,
- creation_time_);
+ creation_time_, depends_on_at_raster_decodes_);
}
GpuRasterBufferProvider::GpuRasterBufferProvider(
@@ -370,7 +372,8 @@
std::unique_ptr<RasterBuffer> GpuRasterBufferProvider::AcquireBufferForRaster(
const ResourcePool::InUsePoolResource& resource,
uint64_t resource_content_id,
- uint64_t previous_content_id) {
+ uint64_t previous_content_id,
+ bool depends_on_at_raster_decodes) {
if (!resource.gpu_backing()) {
auto backing = std::make_unique<GpuRasterBacking>();
backing->worker_context_provider = worker_context_provider_;
@@ -384,7 +387,8 @@
bool resource_has_previous_content =
resource_content_id && resource_content_id == previous_content_id;
return std::make_unique<RasterBufferImpl>(this, resource, backing,
- resource_has_previous_content);
+ resource_has_previous_content,
+ depends_on_at_raster_decodes);
}
void GpuRasterBufferProvider::Flush() {
@@ -461,14 +465,15 @@
const gfx::AxisTransform2d& transform,
const RasterSource::PlaybackSettings& playback_settings,
const GURL& url,
- base::TimeTicks raster_buffer_creation_time) {
+ base::TimeTicks raster_buffer_creation_time,
+ bool depends_on_at_raster_decodes) {
PendingRasterQuery query;
gpu::SyncToken raster_finished_token = PlaybackOnWorkerThreadInternal(
mailbox, texture_target, texture_is_overlay_candidate, sync_token,
resource_size, resource_format, color_space,
resource_has_previous_content, raster_source, raster_full_rect,
raster_dirty_rect, new_content_id, transform, playback_settings, url,
- &query);
+ depends_on_at_raster_decodes, &query);
if (query.raster_duration_query_id) {
if (query.raster_start_query_id)
@@ -502,6 +507,7 @@
const gfx::AxisTransform2d& transform,
const RasterSource::PlaybackSettings& playback_settings,
const GURL& url,
+ bool depends_on_at_raster_decodes,
PendingRasterQuery* query) {
viz::RasterContextProvider::ScopedRasterContextLock scoped_context(
worker_context_provider_, url.possibly_invalid_spec().c_str());
@@ -539,8 +545,11 @@
// enabled because we will use this timestamp to measure raster scheduling
// delay and we only need to collect that data to assess the impact of
// hardware acceleration of image decodes which work only in Chrome OS with
- // OOP-R.
- if (enable_oop_rasterization_) {
+ // OOP-R. Furthermore, we don't count raster work that depends on at-raster
+ // image decodes. This is because we want the delay to always include
+ // image decoding and uploading time, and at-raster decodes should be
+ // relatively rare.
+ if (enable_oop_rasterization_ && !depends_on_at_raster_decodes) {
ri->GenQueriesEXT(1, &query->raster_start_query_id);
DCHECK_GT(query->raster_start_query_id, 0u);
ri->QueryCounterEXT(query->raster_start_query_id,
@@ -663,8 +672,9 @@
// negative scheduling delay.
DCHECK_GE(raster_scheduling_delay.InMicroseconds(), 0u);
UMA_HISTOGRAM_RASTER_TIME_CUSTOM_MICROSECONDS(
- base::StringPrintf("Renderer4.%s.RasterTaskSchedulingDelay.All",
- client_name),
+ base::StringPrintf(
+ "Renderer4.%s.RasterTaskSchedulingDelayNoAtRasterDecodes.All",
+ client_name),
raster_scheduling_delay);
}