Commit 84156192 authored by Eric Karl's avatar Eric Karl Committed by Commit Bot

Don't log decode task umas for trivial tasks

These are throwing off our reporting metrics, and aren't interesting to
record.

Bug: 978225
Change-Id: I78882e53f32acd74e2cf8513b72774e8e7312b8c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1674785
Commit-Queue: Eric Karl <ericrk@chromium.org>
Reviewed-by: default avatarMadeleine Barowsky <mbarowsky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#678491}
parent c1a3b486
...@@ -43,6 +43,9 @@ ScopedImageDecodeTask::ScopedImageDecodeTask(const void* image_ptr, ...@@ -43,6 +43,9 @@ ScopedImageDecodeTask::ScopedImageDecodeTask(const void* image_ptr,
ScopedImageDecodeTask::~ScopedImageDecodeTask() { ScopedImageDecodeTask::~ScopedImageDecodeTask() {
TRACE_EVENT_END0(internal::CategoryName::kTimeline, TRACE_EVENT_END0(internal::CategoryName::kTimeline,
internal::kImageDecodeTask); internal::kImageDecodeTask);
if (suppress_metrics_)
return;
base::TimeDelta duration = base::TimeTicks::Now() - start_time_; base::TimeDelta duration = base::TimeTicks::Now() - start_time_;
switch (task_type_) { switch (task_type_) {
case kInRaster: case kInRaster:
......
...@@ -76,10 +76,15 @@ class CC_BASE_EXPORT ScopedImageDecodeTask { ...@@ -76,10 +76,15 @@ class CC_BASE_EXPORT ScopedImageDecodeTask {
ScopedImageDecodeTask& operator=(const ScopedImageDecodeTask&) = delete; ScopedImageDecodeTask& operator=(const ScopedImageDecodeTask&) = delete;
// Prevents logging duration metrics. Used in cases where a task performed
// uninteresting work or was terminated early.
void SuppressMetrics() { suppress_metrics_ = true; }
private: private:
const DecodeType decode_type_; const DecodeType decode_type_;
const TaskType task_type_; const TaskType task_type_;
const base::TimeTicks start_time_; const base::TimeTicks start_time_;
bool suppress_metrics_ = false;
}; };
class CC_BASE_EXPORT ScopedLayerTreeTask { class CC_BASE_EXPORT ScopedLayerTreeTask {
......
...@@ -76,7 +76,12 @@ class SoftwareImageDecodeTaskImpl : public TileTask { ...@@ -76,7 +76,12 @@ class SoftwareImageDecodeTaskImpl : public TileTask {
paint_image_.GetSkImage().get(), paint_image_.GetSkImage().get(),
devtools_instrumentation::ScopedImageDecodeTask::kSoftware, devtools_instrumentation::ScopedImageDecodeTask::kSoftware,
ImageDecodeCache::ToScopedTaskType(tracing_info_.task_type)); ImageDecodeCache::ToScopedTaskType(tracing_info_.task_type));
cache_->DecodeImageInTask(image_key_, paint_image_, task_type_); SoftwareImageDecodeCache::TaskProcessingResult result =
cache_->DecodeImageInTask(image_key_, paint_image_, task_type_);
// Do not log timing UMAs if we did not perform a full decode.
if (result != SoftwareImageDecodeCache::TaskProcessingResult::kFullDecode)
image_decode_task.SuppressMetrics();
} }
// Overridden from TileTask: // Overridden from TileTask:
...@@ -300,9 +305,10 @@ void SoftwareImageDecodeCache::UnrefImage(const CacheKey& key) { ...@@ -300,9 +305,10 @@ void SoftwareImageDecodeCache::UnrefImage(const CacheKey& key) {
} }
} }
void SoftwareImageDecodeCache::DecodeImageInTask(const CacheKey& key, SoftwareImageDecodeCache::TaskProcessingResult
const PaintImage& paint_image, SoftwareImageDecodeCache::DecodeImageInTask(const CacheKey& key,
DecodeTaskType task_type) { const PaintImage& paint_image,
DecodeTaskType task_type) {
TRACE_EVENT1("cc,benchmark", "SoftwareImageDecodeCache::DecodeImageInTask", TRACE_EVENT1("cc,benchmark", "SoftwareImageDecodeCache::DecodeImageInTask",
"key", key.ToString()); "key", key.ToString());
base::AutoLock lock(lock_); base::AutoLock lock(lock_);
...@@ -316,14 +322,16 @@ void SoftwareImageDecodeCache::DecodeImageInTask(const CacheKey& key, ...@@ -316,14 +322,16 @@ void SoftwareImageDecodeCache::DecodeImageInTask(const CacheKey& key,
DCHECK_GT(cache_entry->ref_count, 0); DCHECK_GT(cache_entry->ref_count, 0);
DCHECK(cache_entry->is_budgeted); DCHECK(cache_entry->is_budgeted);
DecodeImageIfNecessary(key, paint_image, cache_entry); TaskProcessingResult result =
DecodeImageIfNecessary(key, paint_image, cache_entry);
DCHECK(cache_entry->decode_failed || cache_entry->is_locked); DCHECK(cache_entry->decode_failed || cache_entry->is_locked);
return result;
} }
void SoftwareImageDecodeCache::DecodeImageIfNecessary( SoftwareImageDecodeCache::TaskProcessingResult
const CacheKey& key, SoftwareImageDecodeCache::DecodeImageIfNecessary(const CacheKey& key,
const PaintImage& paint_image, const PaintImage& paint_image,
CacheEntry* entry) { CacheEntry* entry) {
TRACE_EVENT1(TRACE_DISABLED_BY_DEFAULT("cc.debug"), TRACE_EVENT1(TRACE_DISABLED_BY_DEFAULT("cc.debug"),
"SoftwareImageDecodeCache::DecodeImageIfNecessary", "key", "SoftwareImageDecodeCache::DecodeImageIfNecessary", "key",
key.ToString()); key.ToString());
...@@ -334,18 +342,18 @@ void SoftwareImageDecodeCache::DecodeImageIfNecessary( ...@@ -334,18 +342,18 @@ void SoftwareImageDecodeCache::DecodeImageIfNecessary(
entry->decode_failed = true; entry->decode_failed = true;
if (entry->decode_failed) if (entry->decode_failed)
return; return TaskProcessingResult::kCancelled;
if (entry->memory) { if (entry->memory) {
if (entry->is_locked) if (entry->is_locked)
return; return TaskProcessingResult::kLockOnly;
bool lock_succeeded = entry->Lock(); bool lock_succeeded = entry->Lock();
// TODO(vmpstr): Deprecate the prepaint split, since it doesn't matter. // TODO(vmpstr): Deprecate the prepaint split, since it doesn't matter.
RecordLockExistingCachedImageHistogram(TilePriority::NOW, lock_succeeded); RecordLockExistingCachedImageHistogram(TilePriority::NOW, lock_succeeded);
if (lock_succeeded) if (lock_succeeded)
return; return TaskProcessingResult::kLockOnly;
} }
std::unique_ptr<CacheEntry> local_cache_entry; std::unique_ptr<CacheEntry> local_cache_entry;
...@@ -437,7 +445,7 @@ void SoftwareImageDecodeCache::DecodeImageIfNecessary( ...@@ -437,7 +445,7 @@ void SoftwareImageDecodeCache::DecodeImageIfNecessary(
if (!local_cache_entry) { if (!local_cache_entry) {
entry->decode_failed = true; entry->decode_failed = true;
return; return TaskProcessingResult::kCancelled;
} }
// Just in case someone else did this already, just unlock our work. // Just in case someone else did this already, just unlock our work.
...@@ -455,6 +463,8 @@ void SoftwareImageDecodeCache::DecodeImageIfNecessary( ...@@ -455,6 +463,8 @@ void SoftwareImageDecodeCache::DecodeImageIfNecessary(
local_cache_entry->MoveImageMemoryTo(entry); local_cache_entry->MoveImageMemoryTo(entry);
DCHECK(entry->is_locked); DCHECK(entry->is_locked);
} }
return TaskProcessingResult::kFullDecode;
} }
base::Optional<SoftwareImageDecodeCache::CacheKey> base::Optional<SoftwareImageDecodeCache::CacheKey>
......
...@@ -32,6 +32,10 @@ class CC_EXPORT SoftwareImageDecodeCache ...@@ -32,6 +32,10 @@ class CC_EXPORT SoftwareImageDecodeCache
enum class DecodeTaskType { USE_IN_RASTER_TASKS, USE_OUT_OF_RASTER_TASKS }; enum class DecodeTaskType { USE_IN_RASTER_TASKS, USE_OUT_OF_RASTER_TASKS };
// Identifies whether a decode task performed decode work, or was fulfilled /
// failed trivially.
enum class TaskProcessingResult { kFullDecode, kLockOnly, kCancelled };
SoftwareImageDecodeCache(SkColorType color_type, SoftwareImageDecodeCache(SkColorType color_type,
size_t locked_memory_limit_bytes, size_t locked_memory_limit_bytes,
PaintImage::GeneratorClientId generator_client_id); PaintImage::GeneratorClientId generator_client_id);
...@@ -56,9 +60,9 @@ class CC_EXPORT SoftwareImageDecodeCache ...@@ -56,9 +60,9 @@ class CC_EXPORT SoftwareImageDecodeCache
// Decode the given image and store it in the cache. This is only called by an // Decode the given image and store it in the cache. This is only called by an
// image decode task from a worker thread. // image decode task from a worker thread.
void DecodeImageInTask(const CacheKey& key, TaskProcessingResult DecodeImageInTask(const CacheKey& key,
const PaintImage& paint_image, const PaintImage& paint_image,
DecodeTaskType task_type); DecodeTaskType task_type);
void OnImageDecodeTaskCompleted(const CacheKey& key, void OnImageDecodeTaskCompleted(const CacheKey& key,
DecodeTaskType task_type); DecodeTaskType task_type);
...@@ -123,9 +127,9 @@ class CC_EXPORT SoftwareImageDecodeCache ...@@ -123,9 +127,9 @@ class CC_EXPORT SoftwareImageDecodeCache
CacheEntry* AddCacheEntry(const CacheKey& key); CacheEntry* AddCacheEntry(const CacheKey& key);
void DecodeImageIfNecessary(const CacheKey& key, TaskProcessingResult DecodeImageIfNecessary(const CacheKey& key,
const PaintImage& paint_image, const PaintImage& paint_image,
CacheEntry* cache_entry); CacheEntry* cache_entry);
void AddBudgetForImage(const CacheKey& key, CacheEntry* entry); void AddBudgetForImage(const CacheKey& key, CacheEntry* entry);
void RemoveBudgetForImage(const CacheKey& key, CacheEntry* entry); void RemoveBudgetForImage(const CacheKey& key, CacheEntry* entry);
base::Optional<CacheKey> FindCachedCandidate(const CacheKey& key); base::Optional<CacheKey> FindCachedCandidate(const CacheKey& key);
......
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