Allow SkTraceMemoryDump to exclude wrapped objects

Allow SkTraceMemoryDump to exclude wrapped objects from dumps. This
helps avoid duplicate dumping when Skia is wrapping an external object
which is already dumped externally.

Bug: 795358
Change-Id: Icbda96b564c81b958d40f74693280ac7d5ba7332
Reviewed-on: https://skia-review.googlesource.com/114681
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Eric Karl <ericrk@chromium.org>

7 files changed, 210 insertions, 46 deletions

diff --git a/include/core/SkTraceMemoryDump.h b/include/core/SkTraceMemoryDump.h
index 8383190ccc..03656b2c37 100644
--- a/include/core/SkTraceMemoryDump.h
+++ b/include/core/SkTraceMemoryDump.h
@@ -73,6 +73,12 @@ public:
      */
     virtual LevelOfDetail getRequestedDetails() const = 0;
 
+    /**
+     * Returns true if we should dump wrapped objects. Wrapped objects come from outside Skia, and
+     * may be independently tracked there.
+     */
+    virtual bool shouldDumpWrappedObjects() const { return true; }
+
 protected:
     virtual ~SkTraceMemoryDump() { }
 };
diff --git a/src/gpu/GrGpuResource.cpp b/src/gpu/GrGpuResource.cpp
index 3d3c88bbe2..d90498d6f4 100644
--- a/src/gpu/GrGpuResource.cpp
+++ b/src/gpu/GrGpuResource.cpp
@@ -69,6 +69,10 @@ void GrGpuResource::abandon() {
 }
 
 void GrGpuResource::dumpMemoryStatistics(SkTraceMemoryDump* traceMemoryDump) const {
+    if (this->fRefsWrappedObjects && !traceMemoryDump->shouldDumpWrappedObjects()) {
+        return;
+    }
+
     // Dump resource as "skia/gpu_resources/resource_#".
     SkString dumpName("skia/gpu_resources/resource_");
     dumpName.appendU32(this->uniqueID().asUInt());
diff --git a/src/gpu/gl/GrGLRenderTarget.cpp b/src/gpu/gl/GrGLRenderTarget.cpp
index e1a4eb0bae..9500e7d6fe 100644
--- a/src/gpu/gl/GrGLRenderTarget.cpp
+++ b/src/gpu/gl/GrGLRenderTarget.cpp
@@ -195,11 +195,21 @@ bool GrGLRenderTarget::canAttemptStencilAttachment() const {
 }
 
 void GrGLRenderTarget::dumpMemoryStatistics(SkTraceMemoryDump* traceMemoryDump) const {
-    // Don't log the backing texture's contribution to the memory size. This will be handled by the
-    // texture object.
+    // Don't check this->fRefsWrappedObjects, as we might be the base of a GrGLTextureRenderTarget
+    // which is multiply inherited from both ourselves and a texture. In these cases, one part
+    // (texture, rt) may be wrapped, while the other is owned by Skia.
+    bool refsWrappedRenderTargetObjects =
+            this->fRTFBOOwnership == GrBackendObjectOwnership::kBorrowed;
+    if (refsWrappedRenderTargetObjects && !traceMemoryDump->shouldDumpWrappedObjects()) {
+        return;
+    }
+
+    // Don't log the framebuffer, as the framebuffer itself doesn't contribute to meaningful
+    // memory usage. It is always a wrapper around either:
+    // - a texture, which is owned elsewhere, and will be dumped there
+    // - a renderbuffer, which will be dumped below.
 
-    // Log any renderbuffer's contribution to memory. We only do this if we own the renderbuffer
-    // (have a fMSColorRenderbufferID).
+    // Log any renderbuffer's contribution to memory.
     if (fMSColorRenderbufferID) {
         size_t size = GrSurface::ComputeSize(this->config(), this->width(), this->height(),
                                              this->msaaSamples(), GrMipMapped::kNo);
diff --git a/src/gpu/gl/GrGLTexture.cpp b/src/gpu/gl/GrGLTexture.cpp
index 0f36dd0f32..5175ea88e7 100644
--- a/src/gpu/gl/GrGLTexture.cpp
+++ b/src/gpu/gl/GrGLTexture.cpp
@@ -101,14 +101,6 @@ GrBackendTexture GrGLTexture::getBackendTexture() const {
     return GrBackendTexture(this->width(), this->height(), this->texturePriv().mipMapped(), fInfo);
 }
 
-void GrGLTexture::setMemoryBacking(SkTraceMemoryDump* traceMemoryDump,
-                                   const SkString& dumpName) const {
-    SkString texture_id;
-    texture_id.appendU32(this->textureID());
-    traceMemoryDump->setMemoryBacking(dumpName.c_str(), "gl_texture",
-                                      texture_id.c_str());
-}
-
 sk_sp<GrGLTexture> GrGLTexture::MakeWrapped(GrGLGpu* gpu, const GrSurfaceDesc& desc,
                                             GrMipMapsStatus mipMapsStatus, const IDDesc& idDesc) {
     return sk_sp<GrGLTexture>(new GrGLTexture(gpu, kWrapped, desc, mipMapsStatus, idDesc));
@@ -126,3 +118,35 @@ bool GrGLTexture::onStealBackendTexture(GrBackendTexture* backendTexture,
     this->GrGLTexture::onAbandon();
     return true;
 }
+
+void GrGLTexture::dumpMemoryStatistics(SkTraceMemoryDump* traceMemoryDump) const {
+    // Don't check this->fRefsWrappedObjects, as we might be the base of a GrGLTextureRenderTarget
+    // which is multiply inherited from both ourselves and a texture. In these cases, one part
+    // (texture, rt) may be wrapped, while the other is owned by Skia.
+    bool refsWrappedTextureObjects =
+            this->fTextureIDOwnership == GrBackendObjectOwnership::kBorrowed;
+    if (refsWrappedTextureObjects && !traceMemoryDump->shouldDumpWrappedObjects()) {
+        return;
+    }
+
+    // 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");
+
+    // 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);
+    }
+
+    SkString texture_id;
+    texture_id.appendU32(this->textureID());
+    traceMemoryDump->setMemoryBacking(dumpName.c_str(), "gl_texture", texture_id.c_str());
+}
diff --git a/src/gpu/gl/GrGLTexture.h b/src/gpu/gl/GrGLTexture.h
index 2b10be8689..7da2bb4428 100644
--- a/src/gpu/gl/GrGLTexture.h
+++ b/src/gpu/gl/GrGLTexture.h
@@ -70,6 +70,8 @@ public:
     static sk_sp<GrGLTexture> MakeWrapped(GrGLGpu*, const GrSurfaceDesc&, GrMipMapsStatus,
                                           const IDDesc&);
 
+    void dumpMemoryStatistics(SkTraceMemoryDump* traceMemoryDump) const override;
+
 protected:
     // Constructor for subclasses.
     GrGLTexture(GrGLGpu*, const GrSurfaceDesc&, const IDDesc&, GrMipMapsStatus);
@@ -82,8 +84,6 @@ protected:
 
     void onAbandon() override;
     void onRelease() override;
-    void setMemoryBacking(SkTraceMemoryDump* traceMemoryDump,
-                          const SkString& dumpName) const override;
 
     bool onStealBackendTexture(GrBackendTexture*, SkImage::BackendTextureReleaseProc*) override;
 
diff --git a/src/gpu/gl/GrGLTextureRenderTarget.cpp b/src/gpu/gl/GrGLTextureRenderTarget.cpp
index 3e7b89f089..e9f224b71b 100644
--- a/src/gpu/gl/GrGLTextureRenderTarget.cpp
+++ b/src/gpu/gl/GrGLTextureRenderTarget.cpp
@@ -35,33 +35,11 @@ GrGLTextureRenderTarget::GrGLTextureRenderTarget(GrGLGpu* gpu,
     this->registerWithCacheWrapped();
 }
 
-// GrGLTextureRenderTarget must dump both of its superclasses.
 void GrGLTextureRenderTarget::dumpMemoryStatistics(
     SkTraceMemoryDump* traceMemoryDump) const {
-  GrGLRenderTarget::dumpMemoryStatistics(traceMemoryDump);
-
-  // Also dump the GrGLTexture's memory. Due to this resource having both a
-  // texture and a
-  // renderbuffer component, dump as skia/gpu_resources/resource_#/texture
-  SkString dumpName("skia/gpu_resources/resource_");
-  dumpName.appendU32(this->uniqueID().asUInt());
-  dumpName.append("/texture");
-
-  // Use the texture's gpuMemorySize, not our own, which includes the
-  // renderbuffer as well.
-  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);
-  }
-
-  SkString texture_id;
-  texture_id.appendU32(this->textureID());
-  traceMemoryDump->setMemoryBacking(dumpName.c_str(), "gl_texture",
-                                    texture_id.c_str());
+    // Delegate to the base classes
+    GrGLRenderTarget::dumpMemoryStatistics(traceMemoryDump);
+    GrGLTexture::dumpMemoryStatistics(traceMemoryDump);
 }
 
 bool GrGLTextureRenderTarget::canAttemptStencilAttachment() const {
diff --git a/tests/TraceMemoryDumpTest.cpp b/tests/TraceMemoryDumpTest.cpp
index a8cdbfeb55..5cae99bca6 100644
--- a/tests/TraceMemoryDumpTest.cpp
+++ b/tests/TraceMemoryDumpTest.cpp
@@ -9,16 +9,32 @@
 
 #include "Test.h"
 
+// These tests are currently GPU-speicifc.
+#if SK_SUPPORT_GPU
+#include "GrContextPriv.h"
+#include "GrRenderTarget.h"
+#include "GrTexture.h"
+#include "gl/GrGLBuffer.h"
+#include "gl/GrGLDefines.h"
+#include "gl/GrGLGpu.h"
+
 /*
  * Build test for SkTraceMemoryDump.
  */
 class TestSkTraceMemoryDump : public SkTraceMemoryDump {
 public:
-    TestSkTraceMemoryDump() { }
+    TestSkTraceMemoryDump(bool shouldDumpWrappedObjects)
+            : fShouldDumpWrappedObjects(shouldDumpWrappedObjects) {}
     ~TestSkTraceMemoryDump() override { }
 
     void dumpNumericValue(const char* dumpName, const char* valueName, const char* units,
-                          uint64_t value) override { }
+                          uint64_t value) override {
+        // Only count "size" dumps, others are just providing metadata.
+        if (SkString("size") == SkString(valueName)) {
+            ++fNumDumpedObjects;
+            fDumpedObjectsSize += value;
+        }
+    }
     void setMemoryBacking(const char* dumpName, const char* backingType,
                           const char* backingObjectId) override { }
     void setDiscardableMemoryBacking(
@@ -27,12 +43,138 @@ public:
     LevelOfDetail getRequestedDetails() const override {
         return SkTraceMemoryDump::kObjectsBreakdowns_LevelOfDetail;
     }
+    bool shouldDumpWrappedObjects() const override { return fShouldDumpWrappedObjects; }
+
+    size_t numDumpedObjects() const { return fNumDumpedObjects; }
+    size_t dumpedObjectsSize() const { return fDumpedObjectsSize; }
+
+private:
+    bool fShouldDumpWrappedObjects;
+    size_t fNumDumpedObjects = 0;
+    size_t fDumpedObjectsSize = 0;
 };
 
-DEF_TEST(SkTraceMemoryDump, reporter) {
-    TestSkTraceMemoryDump x;
-    x.dumpNumericValue("foobar", "size", "bytes", 42);
-    if (x.getRequestedDetails() == SkTraceMemoryDump::kObjectsBreakdowns_LevelOfDetail) {
-        x.dumpNumericValue("foobar/object1", "size", "bytes", 23);
+void ValidateMemoryDumps(skiatest::Reporter* reporter, GrContext* context, size_t size,
+                         bool isOwned) {
+    TestSkTraceMemoryDump dump_with_wrapped(true /* shouldDumpWrappedObjects */);
+    context->dumpMemoryStatistics(&dump_with_wrapped);
+    REPORTER_ASSERT(reporter, 1 == dump_with_wrapped.numDumpedObjects());
+    REPORTER_ASSERT(reporter, size == dump_with_wrapped.dumpedObjectsSize());
+
+    TestSkTraceMemoryDump dump_no_wrapped(false /* shouldDumpWrappedObjects */);
+    context->dumpMemoryStatistics(&dump_no_wrapped);
+    if (isOwned) {
+        REPORTER_ASSERT(reporter, 1 == dump_no_wrapped.numDumpedObjects());
+        REPORTER_ASSERT(reporter, size == dump_no_wrapped.dumpedObjectsSize());
+    } else {
+        REPORTER_ASSERT(reporter, 0 == dump_no_wrapped.numDumpedObjects());
+        REPORTER_ASSERT(reporter, 0 == dump_no_wrapped.dumpedObjectsSize());
     }
 }
+
+DEF_GPUTEST_FOR_GL_RENDERING_CONTEXTS(SkTraceMemoryDump_ownedGLBuffer, reporter, ctxInfo) {
+    GrContext* context = ctxInfo.grContext();
+    GrGLGpu* gpu = static_cast<GrGLGpu*>(context->contextPriv().getGpu());
+    const size_t kMemorySize = 1024;
+    sk_sp<GrGLBuffer> buffer(
+            GrGLBuffer::Create(gpu, kMemorySize, kVertex_GrBufferType, kDynamic_GrAccessPattern));
+
+    ValidateMemoryDumps(reporter, context, kMemorySize, true /* isOwned */);
+}
+
+DEF_GPUTEST_FOR_GL_RENDERING_CONTEXTS(SkTraceMemoryDump_ownedGLTexture, reporter, ctxInfo) {
+    GrContext* context = ctxInfo.grContext();
+    GrGLGpu* gpu = static_cast<GrGLGpu*>(context->contextPriv().getGpu());
+
+    GrSurfaceDesc desc;
+    desc.fFlags = kNone_GrSurfaceFlags;
+    desc.fWidth = 64;
+    desc.fHeight = 64;
+    desc.fConfig = kRGBA_8888_GrPixelConfig;
+    desc.fSampleCnt = 1;
+
+    GrGLTextureInfo glInfo;
+    glInfo.fTarget = GR_GL_TEXTURE_2D;
+    glInfo.fID = 7;  // Arbitrary, we don't actually use the texture.
+    glInfo.fFormat = GR_GL_RGBA8;
+
+    GrGLTexture::IDDesc idDesc;
+    idDesc.fInfo = glInfo;
+    idDesc.fOwnership = GrBackendObjectOwnership::kOwned;
+
+    auto texture = sk_make_sp<GrGLTexture>(gpu, SkBudgeted::kNo, desc, idDesc,
+                                           GrMipMapsStatus::kNotAllocated);
+
+    ValidateMemoryDumps(reporter, context, texture->gpuMemorySize(), true /* isOwned */);
+}
+
+DEF_GPUTEST_FOR_GL_RENDERING_CONTEXTS(SkTraceMemoryDump_unownedGLTexture, reporter, ctxInfo) {
+    GrContext* context = ctxInfo.grContext();
+    GrGLGpu* gpu = static_cast<GrGLGpu*>(context->contextPriv().getGpu());
+
+    GrSurfaceDesc desc;
+    desc.fFlags = kNone_GrSurfaceFlags;
+    desc.fWidth = 64;
+    desc.fHeight = 64;
+    desc.fConfig = kRGBA_8888_GrPixelConfig;
+    desc.fSampleCnt = 1;
+
+    GrGLTextureInfo glInfo;
+    glInfo.fTarget = GR_GL_TEXTURE_2D;
+    glInfo.fID = 7;  // Arbitrary, we don't actually use the texture.
+    glInfo.fFormat = GR_GL_RGBA8;
+
+    GrGLTexture::IDDesc idDesc;
+    idDesc.fInfo = glInfo;
+    idDesc.fOwnership = GrBackendObjectOwnership::kBorrowed;
+
+    auto texture = GrGLTexture::MakeWrapped(gpu, desc, GrMipMapsStatus::kNotAllocated, idDesc);
+
+    ValidateMemoryDumps(reporter, context, texture->gpuMemorySize(), false /* isOwned */);
+}
+
+DEF_GPUTEST_FOR_GL_RENDERING_CONTEXTS(SkTraceMemoryDump_ownedGLRenderTarget, reporter, ctxInfo) {
+    GrContext* context = ctxInfo.grContext();
+    GrGLGpu* gpu = static_cast<GrGLGpu*>(context->contextPriv().getGpu());
+
+    GrSurfaceDesc sd;
+    sd.fFlags = kRenderTarget_GrSurfaceFlag;
+    sd.fWidth = 64;
+    sd.fHeight = 64;
+    sd.fConfig = kRGBA_8888_GrPixelConfig;
+
+    GrGLRenderTarget::IDDesc iddesc;
+    iddesc.fRTFBOID = 20;
+    iddesc.fRTFBOOwnership = GrBackendObjectOwnership::kOwned;
+    iddesc.fTexFBOID = GrGLRenderTarget::kUnresolvableFBOID;
+    iddesc.fMSColorRenderbufferID = 22;
+    iddesc.fIsMixedSampled = false;
+
+    sk_sp<GrGLRenderTarget> rt = GrGLRenderTarget::MakeWrapped(gpu, sd, iddesc, 0);
+
+    ValidateMemoryDumps(reporter, context, rt->gpuMemorySize(), true /* isOwned */);
+}
+
+DEF_GPUTEST_FOR_GL_RENDERING_CONTEXTS(SkTraceMemoryDump_unownedGLRenderTarget, reporter, ctxInfo) {
+    GrContext* context = ctxInfo.grContext();
+    GrGLGpu* gpu = static_cast<GrGLGpu*>(context->contextPriv().getGpu());
+
+    GrSurfaceDesc sd;
+    sd.fFlags = kRenderTarget_GrSurfaceFlag;
+    sd.fWidth = 64;
+    sd.fHeight = 64;
+    sd.fConfig = kRGBA_8888_GrPixelConfig;
+
+    GrGLRenderTarget::IDDesc iddesc;
+    iddesc.fRTFBOID = 20;
+    iddesc.fRTFBOOwnership = GrBackendObjectOwnership::kBorrowed;
+    iddesc.fTexFBOID = GrGLRenderTarget::kUnresolvableFBOID;
+    iddesc.fMSColorRenderbufferID = 22;
+    iddesc.fIsMixedSampled = false;
+
+    sk_sp<GrGLRenderTarget> rt = GrGLRenderTarget::MakeWrapped(gpu, sd, iddesc, 0);
+
+    ValidateMemoryDumps(reporter, context, rt->gpuMemorySize(), false /* isOwned */);
+}
+
+#endif