From 369e9375a3ab7bb56580fc6b22690a76ad759240 Mon Sep 17 00:00:00 2001 From: ericrk Date: Fri, 5 Feb 2016 15:32:36 -0800 Subject: [PATCH] Add Histogram Macros to Skia Adds a set of histogram macros to Skia, modeled after Chrome's UMA_HISTOGRAM_* macros. These allow logging of high frequency events, and are useful to analyze real world usage of certain features. By default, these macros are no-ops. Users can provide a custom header file which defines these macros if they wish to collect histogram data. Chrome will provide such a header. I've currently only added two macros: - SK_HISTOGRAM_BOOLEAN - logs a true/false type relationship (whether we are tiling a texture or not on each draw). - SK_HISTOGRAM_ENUMERATION - logs a set of potential values (which of a number of choices were selected for the texture upload path). We could add more unused macros at the moment, but it seems easier to add these as needed, WDYT? BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1652053004 Review URL: https://codereview.chromium.org/1652053004 --- include/config/SkUserConfig.h | 8 ++++++++ include/core/SkPostConfig.h | 10 ++++++++++ src/core/SkImageCacherator.cpp | 25 +++++++++++++++++++++++++ src/gpu/SkGpuDevice.cpp | 4 ++++ src/gpu/SkGpuDevice_drawTexture.cpp | 3 +++ 5 files changed, 50 insertions(+) diff --git a/include/config/SkUserConfig.h b/include/config/SkUserConfig.h index a0f6258281..a3e40ec5db 100644 --- a/include/config/SkUserConfig.h +++ b/include/config/SkUserConfig.h @@ -146,4 +146,12 @@ */ //#define SK_PDF_USE_PATHOPS_CLIPPING +/* Skia makes use of histogram logging macros to trace the frequency of + * events. By default, Skia provides no-op versions of these macros. + * Skia consumers can provide their own definitions of these macros to + * integrate with their histogram collection backend. + */ +//#define SK_HISTOGRAM_BOOLEAN(name, value) +//#define SK_HISTOGRAM_ENUMERATION(name, value, boundary_value) + #endif diff --git a/include/core/SkPostConfig.h b/include/core/SkPostConfig.h index 68354a6156..a7fbba7c78 100644 --- a/include/core/SkPostConfig.h +++ b/include/core/SkPostConfig.h @@ -344,4 +344,14 @@ # define GR_TEST_UTILS 1 #endif +////////////////////////////////////////////////////////////////////// + +#ifndef SK_HISTOGRAM_BOOLEAN +# define SK_HISTOGRAM_BOOLEAN(name, value) +#endif + +#ifndef SK_HISTOGRAM_ENUMERATION +# define SK_HISTOGRAM_ENUMERATION(name, value, boundary_value) +#endif + #endif // SkPostConfig_DEFINED diff --git a/src/core/SkImageCacherator.cpp b/src/core/SkImageCacherator.cpp index 572778e19c..dc831d7cbf 100644 --- a/src/core/SkImageCacherator.cpp +++ b/src/core/SkImageCacherator.cpp @@ -240,9 +240,24 @@ static GrTexture* set_key_and_return(GrTexture* tex, const GrUniqueKey& key) { */ GrTexture* SkImageCacherator::lockTexture(GrContext* ctx, const GrUniqueKey& key, const SkImage* client, SkImage::CachingHint chint) { + // Values representing the various texture lock paths we can take. Used for logging the path + // taken to a histogram. + enum LockTexturePath { + kFailure_LockTexturePath, + kPreExisting_LockTexturePath, + kNative_LockTexturePath, + kCompressed_LockTexturePath, + kYUV_LockTexturePath, + kRGBA_LockTexturePath, + }; + + enum { kLockTexturePathCount = kRGBA_LockTexturePath + 1 }; + // 1. Check the cache for a pre-existing one if (key.isValid()) { if (GrTexture* tex = ctx->textureProvider()->findAndRefTextureByUniqueKey(key)) { + SK_HISTOGRAM_ENUMERATION("LockTexturePath", kPreExisting_LockTexturePath, + kLockTexturePathCount); return tex; } } @@ -252,6 +267,8 @@ GrTexture* SkImageCacherator::lockTexture(GrContext* ctx, const GrUniqueKey& key ScopedGenerator generator(this); SkIRect subset = SkIRect::MakeXYWH(fOrigin.x(), fOrigin.y(), fInfo.width(), fInfo.height()); if (GrTexture* tex = generator->generateTexture(ctx, &subset)) { + SK_HISTOGRAM_ENUMERATION("LockTexturePath", kNative_LockTexturePath, + kLockTexturePathCount); return set_key_and_return(tex, key); } } @@ -263,6 +280,8 @@ GrTexture* SkImageCacherator::lockTexture(GrContext* ctx, const GrUniqueKey& key if (data) { GrTexture* tex = load_compressed_into_texture(ctx, data, desc); if (tex) { + SK_HISTOGRAM_ENUMERATION("LockTexturePath", kCompressed_LockTexturePath, + kLockTexturePathCount); return set_key_and_return(tex, key); } } @@ -273,6 +292,8 @@ GrTexture* SkImageCacherator::lockTexture(GrContext* ctx, const GrUniqueKey& key Generator_GrYUVProvider provider(generator); GrTexture* tex = provider.refAsTexture(ctx, desc, true); if (tex) { + SK_HISTOGRAM_ENUMERATION("LockTexturePath", kYUV_LockTexturePath, + kLockTexturePathCount); return set_key_and_return(tex, key); } } @@ -282,9 +303,13 @@ GrTexture* SkImageCacherator::lockTexture(GrContext* ctx, const GrUniqueKey& key if (this->tryLockAsBitmap(&bitmap, client, chint)) { GrTexture* tex = GrUploadBitmapToTexture(ctx, bitmap); if (tex) { + SK_HISTOGRAM_ENUMERATION("LockTexturePath", kRGBA_LockTexturePath, + kLockTexturePathCount); return set_key_and_return(tex, key); } } + SK_HISTOGRAM_ENUMERATION("LockTexturePath", kFailure_LockTexturePath, + kLockTexturePathCount); return nullptr; } diff --git a/src/gpu/SkGpuDevice.cpp b/src/gpu/SkGpuDevice.cpp index c0a05797ea..060b2a1e45 100644 --- a/src/gpu/SkGpuDevice.cpp +++ b/src/gpu/SkGpuDevice.cpp @@ -925,6 +925,10 @@ void SkGpuDevice::drawTiledBitmap(const SkBitmap& bitmap, int tileSize, bool bicubic) { ASSERT_SINGLE_OWNER + + // This is the funnel for all paths that draw tiled bitmaps/images. Log histogram entry. + SK_HISTOGRAM_BOOLEAN("DrawTiled", true); + // The following pixel lock is technically redundant, but it is desirable // to lock outside of the tile loop to prevent redecoding the whole image // at each tile in cases where 'bitmap' holds an SkDiscardablePixelRef that diff --git a/src/gpu/SkGpuDevice_drawTexture.cpp b/src/gpu/SkGpuDevice_drawTexture.cpp index efbc6c98d5..5c5fb12421 100644 --- a/src/gpu/SkGpuDevice_drawTexture.cpp +++ b/src/gpu/SkGpuDevice_drawTexture.cpp @@ -94,6 +94,9 @@ void SkGpuDevice::drawTextureProducer(GrTextureProducer* producer, const SkMatrix& viewMatrix, const GrClip& clip, const SkPaint& paint) { + // This is the funnel for all non-tiled bitmap/image draw calls. Log a histogram entry. + SK_HISTOGRAM_BOOLEAN("DrawTiled", false); + // Figure out the actual dst and src rect by clipping the src rect to the bounds of the // adjuster. If the src rect is clipped then the dst rect must be recomputed. Also determine // the matrix that maps the src rect to the dst rect.