aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorjunov <junov@chromium.org>2015-06-02 11:47:45 -0700
committerCommit bot <commit-bot@chromium.org>2015-06-02 11:47:45 -0700
commitd26c9fa66c45b5a050580772acfbcc1b5271543e (patch)
tree8704bc7f574c6300f57819e21613856c1bc75396
parenta66cc7e1e01bab8590fdcfafb269e21bfe8782fa (diff)
downloadskia-d26c9fa66c45b5a050580772acfbcc1b5271543e.tar.gz
Fixing leaky handling of SkImage in SkDeferredCanvas.
Long lived SkImageHeap objects currently accumulate refs indefinitely. This leads to massive memory leaks in the gpu-accelerated 2D canvas code path. This CL does not implement a general fix for SkGPipe, but it resolves the leak in SkDeferredCanvas (currently the only user of SkGPipe) by resetting the image heap when the deferral queue is flushed. This change also fixes the accounting of bytes allocated by referenced images in order to trigger flushing heuristics appropriately. BUG=crbug.com/494148 Review URL: https://codereview.chromium.org/1145893007
-rw-r--r--include/pipe/SkGPipe.h6
-rw-r--r--src/pipe/SkGPipePriv.h3
-rw-r--r--src/pipe/SkGPipeWrite.cpp36
-rw-r--r--src/utils/SkDeferredCanvas.cpp2
-rw-r--r--tests/DeferredCanvasTest.cpp17
5 files changed, 62 insertions, 2 deletions
diff --git a/include/pipe/SkGPipe.h b/include/pipe/SkGPipe.h
index 98e081da5..9446b8159 100644
--- a/include/pipe/SkGPipe.h
+++ b/include/pipe/SkGPipe.h
@@ -87,6 +87,12 @@ public:
virtual void notifyWritten(size_t bytes) = 0;
virtual int numberOfReaders() const { return 1; }
+ /**
+ * Release resource references that are held in internal caches.
+ * This must only be called after the pipe has been completely flushed.
+ */
+ void purgeCaches();
+
private:
friend class SkGPipeWriter;
void setCanvas(SkGPipeCanvas*);
diff --git a/src/pipe/SkGPipePriv.h b/src/pipe/SkGPipePriv.h
index c3919f635..5f6e45b15 100644
--- a/src/pipe/SkGPipePriv.h
+++ b/src/pipe/SkGPipePriv.h
@@ -222,6 +222,8 @@ public:
SkImageHeap();
virtual ~SkImageHeap();
+ size_t bytesInCache() const { return fBytesInCache; }
+ void reset();
// slot must be "valid" -- 0 is never valid
const SkImage* get(int32_t slot) const;
// returns 0 if not found, else returns slot
@@ -231,6 +233,7 @@ public:
private:
SkTDArray<const SkImage*> fArray;
+ size_t fBytesInCache;
};
///////////////////////////////////////////////////////////////////////////////
diff --git a/src/pipe/SkGPipeWrite.cpp b/src/pipe/SkGPipeWrite.cpp
index 2e73be83f..a8a21141a 100644
--- a/src/pipe/SkGPipeWrite.cpp
+++ b/src/pipe/SkGPipeWrite.cpp
@@ -228,7 +228,14 @@ public:
size_t freeMemoryIfPossible(size_t bytesToFree);
size_t storageAllocatedForRecording() {
- return (NULL == fBitmapHeap) ? 0 : fBitmapHeap->bytesAllocated();
+ size_t bytesAllocated = 0;
+ if (NULL != fBitmapHeap) {
+ bytesAllocated += fBitmapHeap->bytesAllocated();
+ }
+ if (NULL != fImageHeap) {
+ bytesAllocated += fImageHeap->bytesInCache();
+ }
+ return bytesAllocated;
}
void beginCommentGroup(const char* description) override;
@@ -241,6 +248,8 @@ public:
*/
bool shuttleBitmap(const SkBitmap&, int32_t slot);
+ void resetImageHeap();
+
protected:
void willSave() override;
SaveLayerStrategy willSaveLayer(const SkRect*, const SkPaint*, SaveFlags) override;
@@ -1157,6 +1166,12 @@ void SkGPipeCanvas::flushRecording(bool detachCurrentBlock) {
}
}
+void SkGPipeCanvas::resetImageHeap() {
+ if (fImageHeap) {
+ fImageHeap->reset();
+ }
+}
+
size_t SkGPipeCanvas::freeMemoryIfPossible(size_t bytesToFree) {
return (NULL == fBitmapHeap) ? 0 : fBitmapHeap->freeMemoryIfPossible(bytesToFree);
}
@@ -1320,6 +1335,14 @@ void SkGPipeController::setCanvas(SkGPipeCanvas* canvas) {
SkRefCnt_SafeAssign(fCanvas, canvas);
}
+void SkGPipeController::purgeCaches()
+{
+ fCanvas->resetImageHeap();
+ // Other caches are self-purging with a small MRU pool
+ // We could purge them as well, but it is not clear whether
+ // that would be a win.
+}
+
///////////////////////////////////////////////////////////////////////////////
SkGPipeWriter::SkGPipeWriter()
@@ -1393,12 +1416,18 @@ void BitmapShuttle::removeCanvas() {
///////////////////////////////////////////////////////////////////////////////////////////////////
-SkImageHeap::SkImageHeap() {}
+SkImageHeap::SkImageHeap() : fBytesInCache (0) {}
SkImageHeap::~SkImageHeap() {
fArray.unrefAll();
}
+void SkImageHeap::reset() {
+ fArray.unrefAll();
+ fArray.rewind();
+ fBytesInCache = 0;
+}
+
const SkImage* SkImageHeap::get(int32_t slot) const {
SkASSERT(slot > 0);
return fArray[slot - 1];
@@ -1417,7 +1446,10 @@ int32_t SkImageHeap::insert(const SkImage* img) {
if (slot) {
return slot;
}
+ // TODO: SkImage does not expose bytes per pixel, 4 is just a best guess.
+ fBytesInCache += img->width() * img->height() * 4;
*fArray.append() = SkRef(img);
+ printf("Images reff'ed: %d \n", fArray.count());
return fArray.count(); // slot is always index+1
}
diff --git a/src/utils/SkDeferredCanvas.cpp b/src/utils/SkDeferredCanvas.cpp
index 40004fbdd..b07a94a52 100644
--- a/src/utils/SkDeferredCanvas.cpp
+++ b/src/utils/SkDeferredCanvas.cpp
@@ -151,6 +151,8 @@ void DeferredPipeController::playback(bool silent) {
// Release all allocated blocks
fAllocator.reset();
+
+ this->purgeCaches();
}
//-----------------------------------------------------------------------------
diff --git a/tests/DeferredCanvasTest.cpp b/tests/DeferredCanvasTest.cpp
index 56b1bea93..60d7d0a03 100644
--- a/tests/DeferredCanvasTest.cpp
+++ b/tests/DeferredCanvasTest.cpp
@@ -689,6 +689,22 @@ static void TestDeferredCanvasBitmapSizeThreshold(skiatest::Reporter* reporter)
}
}
+static void TestDeferredCanvasImageFreeAfterFlush(skiatest::Reporter* reporter) {
+ SkAutoTUnref<SkSurface> surface(SkSurface::NewRasterN32Premul(100, 100));
+ SkAutoTUnref<SkSurface> sourceSurface(SkSurface::NewRasterN32Premul(100, 100));
+ SkAutoTUnref<SkImage> sourceImage(sourceSurface->newImageSnapshot());
+ SkAutoTUnref<SkDeferredCanvas> canvas(SkDeferredCanvas::Create(surface.get()));
+
+ canvas->drawImage(sourceImage, 0, 0, NULL);
+
+ size_t newBytesAllocated = canvas->storageAllocatedForRecording();
+ REPORTER_ASSERT(reporter, newBytesAllocated > 0);
+
+ canvas->flush();
+
+ newBytesAllocated = canvas->storageAllocatedForRecording();
+ REPORTER_ASSERT(reporter, newBytesAllocated == 0);
+}
typedef const void* PixelPtr;
// Returns an opaque pointer which, either points to a GrTexture or RAM pixel
@@ -930,6 +946,7 @@ DEF_TEST(DeferredCanvas_CPU, reporter) {
TestDeferredCanvasSkip(reporter);
TestDeferredCanvasBitmapShaderNoLeak(reporter);
TestDeferredCanvasBitmapSizeThreshold(reporter);
+ TestDeferredCanvasImageFreeAfterFlush(reporter);
TestDeferredCanvasCreateCompatibleDevice(reporter);
TestDeferredCanvasWritePixelsToSurface(reporter);
TestDeferredCanvasGetCanvasSize(reporter);