Dump additional details about the gpu resources.
GpuResources now dump optional string values that describe the type and
category of the resource. The type provides a description of the kind
of resource it is (e.g. texture, buffer object, stencil, etc.) and the
category describes what the resource is currently tasked to do (e.g.
path masks, images, scratch, etc.)
This CL also refactors the dump logic in an attempt to consolidate
duplicated code into GrGpuResources.cpp.
Bug: b/74435803
Change-Id: I83cae825f41e6450a21398ab3ecea349c7c61c15
Reviewed-on: https://skia-review.googlesource.com/115989
Commit-Queue: Derek Sollenberger <djsollen@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
diff --git a/bench/GrResourceCacheBench.cpp b/bench/GrResourceCacheBench.cpp
index a530e62..cc185cf 100644
--- a/bench/GrResourceCacheBench.cpp
+++ b/bench/GrResourceCacheBench.cpp
@@ -38,6 +38,7 @@
private:
size_t onGpuMemorySize() const override { return 100; }
+ const char* getResourceType() const override { return "bench"; }
typedef GrGpuResource INHERITED;
};
diff --git a/include/core/SkTraceMemoryDump.h b/include/core/SkTraceMemoryDump.h
index 03656b2..e9cb5b2 100644
--- a/include/core/SkTraceMemoryDump.h
+++ b/include/core/SkTraceMemoryDump.h
@@ -50,6 +50,10 @@
const char* units,
uint64_t value) = 0;
+ virtual void dumpStringValue(const char* /*dumpName*/,
+ const char* /*valueName*/,
+ const char* /*value*/) { }
+
/**
* Sets the memory backing for an existing dump.
* backingType and backingObjectId are used by the embedder to associate the memory dumped via
diff --git a/include/gpu/GrGpuResource.h b/include/gpu/GrGpuResource.h
index 70c94c0..2da8085 100644
--- a/include/gpu/GrGpuResource.h
+++ b/include/gpu/GrGpuResource.h
@@ -249,6 +249,15 @@
**/
virtual void dumpMemoryStatistics(SkTraceMemoryDump* traceMemoryDump) const;
+ /**
+ * Describes the type of gpu resource that is represented by the implementing
+ * class (e.g. texture, buffer object, stencil). This data is used for diagnostic
+ * purposes by dumpMemoryStatistics().
+ *
+ * The value returned is expected to be long lived and will not be copied by the caller.
+ */
+ virtual const char* getResourceType() const = 0;
+
static uint32_t CreateUniqueID();
protected:
@@ -280,11 +289,24 @@
void didChangeGpuMemorySize() const;
/**
- * Allows subclasses to add additional backing information to the SkTraceMemoryDump. Called by
- * onMemoryDump. The default implementation adds no backing information.
+ * Allows subclasses to add additional backing information to the SkTraceMemoryDump.
**/
virtual void setMemoryBacking(SkTraceMemoryDump*, const SkString&) const {}
+ /**
+ * Returns a string that uniquely identifies this resource.
+ */
+ SkString getResourceName() const;
+
+ /**
+ * A helper for subclasses that override dumpMemoryStatistics(). This method using a format
+ * consistent with the default implementation of dumpMemoryStatistics() but allows the caller
+ * to customize various inputs.
+ */
+ void dumpMemoryStatisticsPriv(SkTraceMemoryDump* traceMemoryDump, const SkString& resourceName,
+ const char* type, size_t size) const;
+
+
private:
/**
* Called by the registerWithCache if the resource is available to be used as scratch.
diff --git a/include/gpu/GrResourceKey.h b/include/gpu/GrResourceKey.h
index c0a08f8..ef319ad 100644
--- a/include/gpu/GrResourceKey.h
+++ b/include/gpu/GrResourceKey.h
@@ -241,7 +241,7 @@
GrUniqueKey& operator=(const GrUniqueKey& that) {
this->INHERITED::operator=(that);
this->setCustomData(sk_ref_sp(that.getCustomData()));
- SkDEBUGCODE(fTag = that.fTag;)
+ fTag = that.fTag;
return *this;
}
@@ -257,14 +257,13 @@
return fData.get();
}
- SkDEBUGCODE(const char* tag() const { return fTag.c_str(); })
+ const char* tag() const { return fTag; }
class Builder : public INHERITED::Builder {
public:
Builder(GrUniqueKey* key, Domain type, int data32Count, const char* tag = nullptr)
: INHERITED::Builder(key, type, data32Count) {
- SkDEBUGCODE(key->fTag = tag;)
- (void) tag; // suppress unused named param warning.
+ key->fTag = tag;
}
/** Used to build a key that wraps another key and adds additional data. */
@@ -277,8 +276,7 @@
const uint32_t* srcData = innerKey.data();
(*innerKeyData++) = innerKey.domain();
memcpy(innerKeyData, srcData, innerKey.dataSize());
- SkDEBUGCODE(key->fTag = tag;)
- (void) tag; // suppress unused named param warning.
+ key->fTag = tag;
}
private:
@@ -290,7 +288,7 @@
private:
sk_sp<SkData> fData;
- SkDEBUGCODE(SkString fTag;)
+ const char* fTag;
};
/**
diff --git a/include/gpu/GrSurface.h b/include/gpu/GrSurface.h
index 5fc365b..46d3945 100644
--- a/include/gpu/GrSurface.h
+++ b/include/gpu/GrSurface.h
@@ -114,6 +114,8 @@
void onAbandon() override;
private:
+ const char* getResourceType() const override { return "Surface"; }
+
GrPixelConfig fConfig;
int fWidth;
int fHeight;
diff --git a/src/gpu/GrBuffer.h b/src/gpu/GrBuffer.h
index b2201a1..33a7f35 100644
--- a/src/gpu/GrBuffer.h
+++ b/src/gpu/GrBuffer.h
@@ -123,6 +123,7 @@
virtual bool onUpdateData(const void* src, size_t srcSizeInBytes);
size_t onGpuMemorySize() const override { return fSizeInBytes; } // TODO: zero for cpu backed?
+ const char* getResourceType() const override { return "Buffer Object"; }
void computeScratchKey(GrScratchKey* key) const override;
size_t fSizeInBytes;
diff --git a/src/gpu/GrGpuResource.cpp b/src/gpu/GrGpuResource.cpp
index d90498d..ec71135 100644
--- a/src/gpu/GrGpuResource.cpp
+++ b/src/gpu/GrGpuResource.cpp
@@ -73,20 +73,33 @@
return;
}
- // Dump resource as "skia/gpu_resources/resource_#".
- SkString dumpName("skia/gpu_resources/resource_");
- dumpName.appendU32(this->uniqueID().asUInt());
+ this->dumpMemoryStatisticsPriv(traceMemoryDump, this->getResourceName(),
+ this->getResourceType(), this->gpuMemorySize());
+}
- traceMemoryDump->dumpNumericValue(dumpName.c_str(), "size", "bytes", this->gpuMemorySize());
-
- if (this->isPurgeable()) {
- traceMemoryDump->dumpNumericValue(dumpName.c_str(), "purgeable_size", "bytes",
- this->gpuMemorySize());
+void GrGpuResource::dumpMemoryStatisticsPriv(SkTraceMemoryDump* traceMemoryDump,
+ const SkString& resourceName,
+ const char* type, size_t size) const {
+ const char* tag = "Scratch";
+ if (fUniqueKey.isValid()) {
+ tag = (fUniqueKey.tag() != nullptr) ? fUniqueKey.tag() : "Other";
}
- // Call setMemoryBacking to allow sub-classes with implementation specific backings (such as GL
- // objects) to provide additional information.
- this->setMemoryBacking(traceMemoryDump, dumpName);
+ traceMemoryDump->dumpNumericValue(resourceName.c_str(), "size", "bytes", size);
+ traceMemoryDump->dumpStringValue(resourceName.c_str(), "type", type);
+ traceMemoryDump->dumpStringValue(resourceName.c_str(), "category", tag);
+ if (this->isPurgeable()) {
+ traceMemoryDump->dumpNumericValue(resourceName.c_str(), "purgeable_size", "bytes", size);
+ }
+
+ this->setMemoryBacking(traceMemoryDump, resourceName);
+}
+
+SkString GrGpuResource::getResourceName() const {
+ // Dump resource as "skia/gpu_resources/resource_#".
+ SkString resourceName("skia/gpu_resources/resource_");
+ resourceName.appendU32(this->uniqueID().asUInt());
+ return resourceName;
}
const GrContext* GrGpuResource::getContext() const {
diff --git a/src/gpu/GrPath.cpp b/src/gpu/GrPath.cpp
index 836cc5e..c0c5643 100644
--- a/src/gpu/GrPath.cpp
+++ b/src/gpu/GrPath.cpp
@@ -25,7 +25,7 @@
return;
}
static const GrUniqueKey::Domain kGeneralPathDomain = GrUniqueKey::GenerateDomain();
- GrUniqueKey::Builder builder(key, kGeneralPathDomain, geoCnt + styleCnt);
+ GrUniqueKey::Builder builder(key, kGeneralPathDomain, geoCnt + styleCnt, "Path");
shape.writeUnstyledKey(&builder[0]);
if (styleCnt) {
write_style_key(&builder[geoCnt], shape.style());
diff --git a/src/gpu/GrPath.h b/src/gpu/GrPath.h
index 1953837..cf4a11d 100644
--- a/src/gpu/GrPath.h
+++ b/src/gpu/GrPath.h
@@ -51,6 +51,7 @@
#endif
private:
+ const char* getResourceType() const override { return "Path Data"; }
typedef GrGpuResource INHERITED;
};
diff --git a/src/gpu/GrSoftwarePathRenderer.cpp b/src/gpu/GrSoftwarePathRenderer.cpp
index 228625f..15ea55d 100644
--- a/src/gpu/GrSoftwarePathRenderer.cpp
+++ b/src/gpu/GrSoftwarePathRenderer.cpp
@@ -290,14 +290,16 @@
// Fractional translate does not affect caching on Android. This is done for better cache
// hit ratio and speed, but it is matching HWUI behavior, which doesn't consider the matrix
// at all when caching paths.
- GrUniqueKey::Builder builder(&maskKey, kDomain, 4 + args.fShape->unstyledKeySize());
+ GrUniqueKey::Builder builder(&maskKey, kDomain, 4 + args.fShape->unstyledKeySize(),
+ "SW Path Mask");
#else
SkScalar tx = args.fViewMatrix->get(SkMatrix::kMTransX);
SkScalar ty = args.fViewMatrix->get(SkMatrix::kMTransY);
// Allow 8 bits each in x and y of subpixel positioning.
SkFixed fracX = SkScalarToFixed(SkScalarFraction(tx)) & 0x0000FF00;
SkFixed fracY = SkScalarToFixed(SkScalarFraction(ty)) & 0x0000FF00;
- GrUniqueKey::Builder builder(&maskKey, kDomain, 5 + args.fShape->unstyledKeySize());
+ GrUniqueKey::Builder builder(&maskKey, kDomain, 5 + args.fShape->unstyledKeySize(),
+ "SW Path Mask");
#endif
builder[0] = SkFloat2Bits(sx);
builder[1] = SkFloat2Bits(sy);
diff --git a/src/gpu/GrStencilAttachment.h b/src/gpu/GrStencilAttachment.h
index df5677d..e8c56cc 100644
--- a/src/gpu/GrStencilAttachment.h
+++ b/src/gpu/GrStencilAttachment.h
@@ -45,6 +45,7 @@
}
private:
+ const char* getResourceType() const override { return "Stencil"; }
int fWidth;
int fHeight;
diff --git a/src/gpu/SkGr.cpp b/src/gpu/SkGr.cpp
index ec06561..b4cb22b 100644
--- a/src/gpu/SkGr.cpp
+++ b/src/gpu/SkGr.cpp
@@ -57,7 +57,7 @@
SkASSERT(imageID);
SkASSERT(!imageBounds.isEmpty());
static const GrUniqueKey::Domain kImageIDDomain = GrUniqueKey::GenerateDomain();
- GrUniqueKey::Builder builder(key, kImageIDDomain, 5);
+ GrUniqueKey::Builder builder(key, kImageIDDomain, 5, "Image");
builder[0] = imageID;
builder[1] = imageBounds.fLeft;
builder[2] = imageBounds.fTop;
diff --git a/src/gpu/effects/GrCircleBlurFragmentProcessor.cpp b/src/gpu/effects/GrCircleBlurFragmentProcessor.cpp
index d44f431..618da34 100644
--- a/src/gpu/effects/GrCircleBlurFragmentProcessor.cpp
+++ b/src/gpu/effects/GrCircleBlurFragmentProcessor.cpp
@@ -200,7 +200,7 @@
static const GrUniqueKey::Domain kDomain = GrUniqueKey::GenerateDomain();
GrUniqueKey key;
- GrUniqueKey::Builder builder(&key, kDomain, 1);
+ GrUniqueKey::Builder builder(&key, kDomain, 1, "1-D Circular Blur");
builder[0] = sigmaToCircleRRatioFixed;
builder.finish();
diff --git a/src/gpu/effects/GrRRectBlurEffect.h b/src/gpu/effects/GrRRectBlurEffect.h
index 1d382a0..b59d1be 100644
--- a/src/gpu/effects/GrRRectBlurEffect.h
+++ b/src/gpu/effects/GrRRectBlurEffect.h
@@ -34,7 +34,7 @@
float xformedSigma) {
static const GrUniqueKey::Domain kDomain = GrUniqueKey::GenerateDomain();
GrUniqueKey key;
- GrUniqueKey::Builder builder(&key, kDomain, 9);
+ GrUniqueKey::Builder builder(&key, kDomain, 9, "RoundRect Blur Mask");
builder[0] = SkScalarCeilToInt(xformedSigma - 1 / 6.0f);
int index = 1;
diff --git a/src/gpu/effects/GrRectBlurEffect.h b/src/gpu/effects/GrRectBlurEffect.h
index 123c91f..e6d7779 100644
--- a/src/gpu/effects/GrRectBlurEffect.h
+++ b/src/gpu/effects/GrRectBlurEffect.h
@@ -25,7 +25,7 @@
static const GrUniqueKey::Domain kDomain = GrUniqueKey::GenerateDomain();
GrUniqueKey key;
- GrUniqueKey::Builder builder(&key, kDomain, 1);
+ GrUniqueKey::Builder builder(&key, kDomain, 1, "Rect Blur Mask");
builder[0] = profileSize;
builder.finish();
diff --git a/src/gpu/gl/GrGLRenderTarget.cpp b/src/gpu/gl/GrGLRenderTarget.cpp
index 9500e7d..5513e8a 100644
--- a/src/gpu/gl/GrGLRenderTarget.cpp
+++ b/src/gpu/gl/GrGLRenderTarget.cpp
@@ -216,19 +216,14 @@
// Due to this resource having both a texture and a renderbuffer component, dump as
// skia/gpu_resources/resource_#/renderbuffer
- SkString dumpName("skia/gpu_resources/resource_");
- dumpName.appendU32(this->uniqueID().asUInt());
- dumpName.append("/renderbuffer");
+ SkString resourceName = this->getResourceName();
+ resourceName.append("/renderbuffer");
- traceMemoryDump->dumpNumericValue(dumpName.c_str(), "size", "bytes", size);
-
- if (this->isPurgeable()) {
- traceMemoryDump->dumpNumericValue(dumpName.c_str(), "purgeable_size", "bytes", size);
- }
+ this->dumpMemoryStatisticsPriv(traceMemoryDump, resourceName, "RenderTarget", size);
SkString renderbuffer_id;
renderbuffer_id.appendU32(fMSColorRenderbufferID);
- traceMemoryDump->setMemoryBacking(dumpName.c_str(), "gl_renderbuffer",
+ traceMemoryDump->setMemoryBacking(resourceName.c_str(), "gl_renderbuffer",
renderbuffer_id.c_str());
}
}
diff --git a/src/gpu/gl/GrGLTexture.cpp b/src/gpu/gl/GrGLTexture.cpp
index 61bce35..69b214d 100644
--- a/src/gpu/gl/GrGLTexture.cpp
+++ b/src/gpu/gl/GrGLTexture.cpp
@@ -138,21 +138,16 @@
// Dump as skia/gpu_resources/resource_#/texture, to avoid conflicts in the
// GrGLTextureRenderTarget case, where multiple things may dump to the same resource. This
// has no downside in the normal case.
- SkString dumpName("skia/gpu_resources/resource_");
- dumpName.appendU32(this->uniqueID().asUInt());
- dumpName.append("/texture");
+ SkString resourceName = this->getResourceName();
+ resourceName.append("/texture");
// As we are only dumping our texture memory (not any additional memory tracked by classes
// which may inherit from us), specifically call GrGLTexture::gpuMemorySize to avoid
// hitting an override.
- size_t size = GrGLTexture::gpuMemorySize();
- traceMemoryDump->dumpNumericValue(dumpName.c_str(), "size", "bytes", size);
-
- if (this->isPurgeable()) {
- traceMemoryDump->dumpNumericValue(dumpName.c_str(), "purgeable_size", "bytes", size);
- }
+ this->dumpMemoryStatisticsPriv(traceMemoryDump, resourceName, "Texture",
+ GrGLTexture::gpuMemorySize());
SkString texture_id;
texture_id.appendU32(this->textureID());
- traceMemoryDump->setMemoryBacking(dumpName.c_str(), "gl_texture", texture_id.c_str());
+ traceMemoryDump->setMemoryBacking(resourceName.c_str(), "gl_texture", texture_id.c_str());
}
diff --git a/src/gpu/gl/GrGLTextureRenderTarget.cpp b/src/gpu/gl/GrGLTextureRenderTarget.cpp
index e9f224b..049cb31 100644
--- a/src/gpu/gl/GrGLTextureRenderTarget.cpp
+++ b/src/gpu/gl/GrGLTextureRenderTarget.cpp
@@ -37,9 +37,16 @@
void GrGLTextureRenderTarget::dumpMemoryStatistics(
SkTraceMemoryDump* traceMemoryDump) const {
+#ifndef SK_BUILD_FOR_ANDROID_FRAMEWORK
// Delegate to the base classes
GrGLRenderTarget::dumpMemoryStatistics(traceMemoryDump);
GrGLTexture::dumpMemoryStatistics(traceMemoryDump);
+#else
+ SkString resourceName = this->getResourceName();
+ resourceName.append("/texture_renderbuffer");
+ this->dumpMemoryStatisticsPriv(traceMemoryDump, resourceName, "RenderTarget",
+ this->gpuMemorySize());
+#endif
}
bool GrGLTextureRenderTarget::canAttemptStencilAttachment() const {
diff --git a/src/gpu/ops/GrTessellatingPathRenderer.cpp b/src/gpu/ops/GrTessellatingPathRenderer.cpp
index 8eaadf0..651d34f 100644
--- a/src/gpu/ops/GrTessellatingPathRenderer.cpp
+++ b/src/gpu/ops/GrTessellatingPathRenderer.cpp
@@ -245,7 +245,7 @@
static constexpr int kClipBoundsCnt = sizeof(fDevClipBounds) / sizeof(uint32_t);
int shapeKeyDataCnt = fShape.unstyledKeySize();
SkASSERT(shapeKeyDataCnt >= 0);
- GrUniqueKey::Builder builder(&key, kDomain, shapeKeyDataCnt + kClipBoundsCnt);
+ GrUniqueKey::Builder builder(&key, kDomain, shapeKeyDataCnt + kClipBoundsCnt, "Path");
fShape.writeUnstyledKey(&builder[0]);
// For inverse fills, the tessellation is dependent on clip bounds.
if (inverseFill) {
diff --git a/tests/ResourceCacheTest.cpp b/tests/ResourceCacheTest.cpp
index c56bd01..11dcdd4 100644
--- a/tests/ResourceCacheTest.cpp
+++ b/tests/ResourceCacheTest.cpp
@@ -340,6 +340,7 @@
}
size_t onGpuMemorySize() const override { return fSize; }
+ const char* getResourceType() const override { return "Test"; }
TestResource* fToDelete;
size_t fSize;
@@ -1661,19 +1662,24 @@
GrResourceCache* cache = mock.cache();
GrGpu* gpu = context->contextPriv().getGpu();
+ // tag strings are expected to be long lived
+ std::vector<SkString> tagStrings;
+
SkString tagStr;
int tagIdx = 0;
int currTagCnt = 0;
for (int i = 0; i < kNumResources; ++i, ++currTagCnt) {
+
sk_sp<GrGpuResource> resource(new TestResource(gpu));
GrUniqueKey key;
if (currTagCnt == tagIdx) {
tagIdx += 1;
currTagCnt = 0;
tagStr.printf("tag%d", tagIdx);
+ tagStrings.emplace_back(tagStr);
}
- make_unique_key<1>(&key, i, tagStr.c_str());
+ make_unique_key<1>(&key, i, tagStrings.back().c_str());
resource->resourcePriv().setUniqueKey(key);
}
SkASSERT(kLastTagIdx == tagIdx);