AuroraMiddleware/skia2
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

4x allocation in PipeController is probably overkill.

Browse Source 
When verylargebitmap GM runs in cross-process pipe mode, we're
requestBlock()ing ~200M to carry the bitmaps.  The current
implementation ends up allocating ~800M, which is a bit wasteful.

SkGPipeWrite already rounds up to 16K, so just rely on that.

This change exposed several bugs in pipe:
  - we don't reserve enough space in drawVertices
  - we don't reserve enough space for factory names in cross-process mode
  - we don't quite have the right check in needOpBytes to see if we needed to send off the current block and allocate a new one

SETUP_NOTIFY and generally calling doNotify() more often than necessary made things hard to debug and understand.  Now the pipe always waits to send off its current block until it needs more space than that block can provide, or it's the final block.  We can put these back if we need the proactive flushing, but it seems not necessary?

Removed an assert in DeferredCanvasTest, which is somtimes 2 (Debug), sometimes 3 (Release).  It seemed like the other asserts were more essential, and this one was more of a white-box assertion.  Still sound if we remove it?

BUG=skia:2478
R=scroggo@google.com, mtklein@google.com, reed@google.com, junov@chromium.org

Author: mtklein@chromium.org

Review URL: https://codereview.chromium.org/267863002

git-svn-id: http://skia.googlecode.com/svn/trunk@14613 2bbb7eff-a529-9590-31e7-b0007b416f81
This commit is contained in:
commit-bot@chromium.org 2014-05-07 15:27:10 +00:00
parent 6e8f64cc91
commit 2d91efffdb
4 changed files with 58 additions and 79 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

110
src/pipe/SkGPipeWrite.cpp
View File

@ -29,6 +29,14 @@
#include "SkTypeface.h"
#include "SkWriter32.h"
#if SK_DEBUG
// When debugging, allocate snuggly.
static const size_t kMinBlockSize = 0;
#else
// For peformance, make large allocations.
static const size_t kMinBlockSize = 16 * 1024;
#endif
enum {
kSizeOfFlatRRect = sizeof(SkRect) + 4 * sizeof(SkVector)
};
@ -350,15 +358,6 @@ private:
SkPaint fPaint;
void writePaint(const SkPaint&);
class AutoPipeNotify {
public:
AutoPipeNotify(SkGPipeCanvas* canvas) : fCanvas(canvas) {}
~AutoPipeNotify() { fCanvas->doNotify(); }
private:
SkGPipeCanvas* fCanvas;
};
friend class AutoPipeNotify;
typedef SkCanvas INHERITED;
};
@ -366,7 +365,7 @@ void SkGPipeCanvas::flattenFactoryNames() {
const char* name;
while ((name = fFactorySet->getNextAddedFactoryName()) != NULL) {
size_t len = strlen(name);
if (this->needOpBytes(len)) {
if (this->needOpBytes(SkWriter32::WriteStringSize(name, len))) {
this->writeOp(kDef_Factory_DrawOp);
fWriter.writeString(name, len);
}
@ -421,7 +420,6 @@ int SkGPipeCanvas::flattenToIndex(SkFlattenable* obj, PaintFlats paintflat) {
///////////////////////////////////////////////////////////////////////////////
#define MIN_BLOCK_SIZE (16 * 1024)
#define BITMAPS_TO_KEEP 5
#define FLATTENABLES_TO_KEEP 10
@ -459,7 +457,6 @@ SkGPipeCanvas::SkGPipeCanvas(SkGPipeController* controller,
}
}
fFlattenableHeap.setBitmapStorage(fBitmapHeap);
this->doNotify();
}
SkGPipeCanvas::~SkGPipeCanvas() {
@ -474,12 +471,13 @@ bool SkGPipeCanvas::needOpBytes(size_t needed) {
}
needed += 4; // size of DrawOp atom
needed = SkTMax(kMinBlockSize, needed);
needed = SkAlign4(needed);
if (fWriter.bytesWritten() + needed > fBlockSize) {
// Before we wipe out any data that has already been written, read it
// out.
// We're making a new block. First push out the current block.
this->doNotify();
size_t blockSize = SkTMax<size_t>(MIN_BLOCK_SIZE, needed);
void* block = fController->requestBlock(blockSize, &fBlockSize);
void* block = fController->requestBlock(needed, &fBlockSize);
if (NULL == block) {
// Do not notify the readers, which would call this function again.
this->finish(false);
@ -510,11 +508,7 @@ uint32_t SkGPipeCanvas::getTypefaceID(SkTypeface* face) {
///////////////////////////////////////////////////////////////////////////////
#define NOTIFY_SETUP(canvas) \
AutoPipeNotify apn(canvas)
void SkGPipeCanvas::willSave(SaveFlags flags) {
NOTIFY_SETUP(this);
if (this->needOpBytes()) {
this->writeOp(kSave_DrawOp, 0, flags);
}
@ -524,7 +518,6 @@ void SkGPipeCanvas::willSave(SaveFlags flags) {
SkCanvas::SaveLayerStrategy SkGPipeCanvas::willSaveLayer(const SkRect* bounds, const SkPaint* paint,
SaveFlags saveFlags) {
NOTIFY_SETUP(this);
size_t size = 0;
unsigned opFlags = 0;
@ -554,7 +547,6 @@ SkCanvas::SaveLayerStrategy SkGPipeCanvas::willSaveLayer(const SkRect* bounds, c
}
void SkGPipeCanvas::willRestore() {
NOTIFY_SETUP(this);
if (this->needOpBytes()) {
this->writeOp(kRestore_DrawOp);
}
@ -595,7 +587,6 @@ void SkGPipeCanvas::recordConcat(const SkMatrix& m) {
void SkGPipeCanvas::didConcat(const SkMatrix& matrix) {
if (!matrix.isIdentity()) {
NOTIFY_SETUP(this);
switch (matrix.getType()) {
case SkMatrix::kTranslate_Mask:
this->recordTranslate(matrix);
@ -613,7 +604,6 @@ void SkGPipeCanvas::didConcat(const SkMatrix& matrix) {
}
void SkGPipeCanvas::didSetMatrix(const SkMatrix& matrix) {
NOTIFY_SETUP(this);
if (this->needOpBytes(matrix.writeToMemory(NULL))) {
this->writeOp(kSetMatrix_DrawOp);
fWriter.writeMatrix(matrix);
@ -623,7 +613,6 @@ void SkGPipeCanvas::didSetMatrix(const SkMatrix& matrix) {
void SkGPipeCanvas::onClipRect(const SkRect& rect, SkRegion::Op rgnOp,
ClipEdgeStyle edgeStyle) {
NOTIFY_SETUP(this);
if (this->needOpBytes(sizeof(SkRect))) {
unsigned flags = 0;
if (kSoft_ClipEdgeStyle == edgeStyle) {
@ -637,7 +626,6 @@ void SkGPipeCanvas::onClipRect(const SkRect& rect, SkRegion::Op rgnOp,
void SkGPipeCanvas::onClipRRect(const SkRRect& rrect, SkRegion::Op rgnOp,
ClipEdgeStyle edgeStyle) {
NOTIFY_SETUP(this);
if (this->needOpBytes(kSizeOfFlatRRect)) {
unsigned flags = 0;
if (kSoft_ClipEdgeStyle == edgeStyle) {
@ -651,7 +639,6 @@ void SkGPipeCanvas::onClipRRect(const SkRRect& rrect, SkRegion::Op rgnOp,
void SkGPipeCanvas::onClipPath(const SkPath& path, SkRegion::Op rgnOp,
ClipEdgeStyle edgeStyle) {
NOTIFY_SETUP(this);
if (this->needOpBytes(path.writeToMemory(NULL))) {
unsigned flags = 0;
if (kSoft_ClipEdgeStyle == edgeStyle) {
@ -665,7 +652,6 @@ void SkGPipeCanvas::onClipPath(const SkPath& path, SkRegion::Op rgnOp,
}
void SkGPipeCanvas::onClipRegion(const SkRegion& region, SkRegion::Op rgnOp) {
NOTIFY_SETUP(this);
if (this->needOpBytes(region.writeToMemory(NULL))) {
this->writeOp(kClipRegion_DrawOp, 0, rgnOp);
fWriter.writeRegion(region);
@ -676,7 +662,6 @@ void SkGPipeCanvas::onClipRegion(const SkRegion& region, SkRegion::Op rgnOp) {
///////////////////////////////////////////////////////////////////////////////
void SkGPipeCanvas::clear(SkColor color) {
NOTIFY_SETUP(this);
unsigned flags = 0;
if (color) {
flags |= kClear_HasColor_DrawOpFlag;
@ -690,7 +675,6 @@ void SkGPipeCanvas::clear(SkColor color) {
}
void SkGPipeCanvas::drawPaint(const SkPaint& paint) {
NOTIFY_SETUP(this);
this->writePaint(paint);
if (this->needOpBytes()) {
this->writeOp(kDrawPaint_DrawOp);
@ -700,7 +684,6 @@ void SkGPipeCanvas::drawPaint(const SkPaint& paint) {
void SkGPipeCanvas::drawPoints(PointMode mode, size_t count,
const SkPoint pts[], const SkPaint& paint) {
if (count) {
NOTIFY_SETUP(this);
this->writePaint(paint);
if (this->needOpBytes(4 + count * sizeof(SkPoint))) {
this->writeOp(kDrawPoints_DrawOp, mode, 0);
@ -711,7 +694,6 @@ void SkGPipeCanvas::drawPoints(PointMode mode, size_t count,
}
void SkGPipeCanvas::drawOval(const SkRect& rect, const SkPaint& paint) {
NOTIFY_SETUP(this);
this->writePaint(paint);
if (this->needOpBytes(sizeof(SkRect))) {
this->writeOp(kDrawOval_DrawOp);
@ -720,7 +702,6 @@ void SkGPipeCanvas::drawOval(const SkRect& rect, const SkPaint& paint) {
}
void SkGPipeCanvas::drawRect(const SkRect& rect, const SkPaint& paint) {
NOTIFY_SETUP(this);
this->writePaint(paint);
if (this->needOpBytes(sizeof(SkRect))) {
this->writeOp(kDrawRect_DrawOp);
@ -729,7 +710,6 @@ void SkGPipeCanvas::drawRect(const SkRect& rect, const SkPaint& paint) {
}
void SkGPipeCanvas::drawRRect(const SkRRect& rrect, const SkPaint& paint) {
NOTIFY_SETUP(this);
this->writePaint(paint);
if (this->needOpBytes(kSizeOfFlatRRect)) {
this->writeOp(kDrawRRect_DrawOp);
@ -739,7 +719,6 @@ void SkGPipeCanvas::drawRRect(const SkRRect& rrect, const SkPaint& paint) {
void SkGPipeCanvas::onDrawDRRect(const SkRRect& outer, const SkRRect& inner,
const SkPaint& paint) {
NOTIFY_SETUP(this);
this->writePaint(paint);
if (this->needOpBytes(kSizeOfFlatRRect * 2)) {
this->writeOp(kDrawDRRect_DrawOp);
@ -749,7 +728,6 @@ void SkGPipeCanvas::onDrawDRRect(const SkRRect& outer, const SkRRect& inner,
}
void SkGPipeCanvas::drawPath(const SkPath& path, const SkPaint& paint) {
NOTIFY_SETUP(this);
this->writePaint(paint);
if (this->needOpBytes(path.writeToMemory(NULL))) {
this->writeOp(kDrawPath_DrawOp);
@ -761,16 +739,24 @@ bool SkGPipeCanvas::commonDrawBitmap(const SkBitmap& bm, DrawOps op,
unsigned flags,
size_t opBytesNeeded,
const SkPaint* paint) {
if (fDone) {
return false;
}
if (paint != NULL) {
flags |= kDrawBitmap_HasPaint_DrawOpFlag;
this->writePaint(*paint);
}
// This needs to run first so its calls to needOpBytes() and writes
// don't interlace with the needOpBytes() and writes below.
SkASSERT(fBitmapHeap != NULL);
int32_t bitmapIndex = fBitmapHeap->insert(bm);
if (SkBitmapHeap::INVALID_SLOT == bitmapIndex) {
return false;
}
if (this->needOpBytes(opBytesNeeded)) {
SkASSERT(fBitmapHeap != NULL);
int32_t bitmapIndex = fBitmapHeap->insert(bm);
if (SkBitmapHeap::INVALID_SLOT == bitmapIndex) {
return false;
}
this->writeOp(op, flags, bitmapIndex);
return true;
}
@ -779,7 +765,6 @@ bool SkGPipeCanvas::commonDrawBitmap(const SkBitmap& bm, DrawOps op,
void SkGPipeCanvas::drawBitmap(const SkBitmap& bm, SkScalar left, SkScalar top,
const SkPaint* paint) {
NOTIFY_SETUP(this);
size_t opBytesNeeded = sizeof(SkScalar) * 2;
if (this->commonDrawBitmap(bm, kDrawBitmap_DrawOp, 0, opBytesNeeded, paint)) {
@ -791,7 +776,6 @@ void SkGPipeCanvas::drawBitmap(const SkBitmap& bm, SkScalar left, SkScalar top,
void SkGPipeCanvas::drawBitmapRectToRect(const SkBitmap& bm, const SkRect* src,
const SkRect& dst, const SkPaint* paint,
DrawBitmapRectFlags dbmrFlags) {
NOTIFY_SETUP(this);
size_t opBytesNeeded = sizeof(SkRect);
bool hasSrc = src != NULL;
unsigned flags;
@ -815,7 +799,6 @@ void SkGPipeCanvas::drawBitmapRectToRect(const SkBitmap& bm, const SkRect* src,
void SkGPipeCanvas::drawBitmapMatrix(const SkBitmap& bm, const SkMatrix& matrix,
const SkPaint* paint) {
NOTIFY_SETUP(this);
size_t opBytesNeeded = matrix.writeToMemory(NULL);
if (this->commonDrawBitmap(bm, kDrawBitmapMatrix_DrawOp, 0, opBytesNeeded, paint)) {
@ -825,7 +808,6 @@ void SkGPipeCanvas::drawBitmapMatrix(const SkBitmap& bm, const SkMatrix& matrix,
void SkGPipeCanvas::drawBitmapNine(const SkBitmap& bm, const SkIRect& center,
const SkRect& dst, const SkPaint* paint) {
NOTIFY_SETUP(this);
size_t opBytesNeeded = sizeof(int32_t) * 4 + sizeof(SkRect);
if (this->commonDrawBitmap(bm, kDrawBitmapNine_DrawOp, 0, opBytesNeeded, paint)) {
@ -839,7 +821,6 @@ void SkGPipeCanvas::drawBitmapNine(const SkBitmap& bm, const SkIRect& center,
void SkGPipeCanvas::drawSprite(const SkBitmap& bm, int left, int top,
const SkPaint* paint) {
NOTIFY_SETUP(this);
size_t opBytesNeeded = sizeof(int32_t) * 2;
if (this->commonDrawBitmap(bm, kDrawSprite_DrawOp, 0, opBytesNeeded, paint)) {
@ -851,7 +832,6 @@ void SkGPipeCanvas::drawSprite(const SkBitmap& bm, int left, int top,
void SkGPipeCanvas::onDrawText(const void* text, size_t byteLength, SkScalar x, SkScalar y,
const SkPaint& paint) {
if (byteLength) {
NOTIFY_SETUP(this);
this->writePaint(paint);
if (this->needOpBytes(4 + SkAlign4(byteLength) + 2 * sizeof(SkScalar))) {
this->writeOp(kDrawText_DrawOp);
@ -866,7 +846,6 @@ void SkGPipeCanvas::onDrawText(const void* text, size_t byteLength, SkScalar x,
void SkGPipeCanvas::onDrawPosText(const void* text, size_t byteLength, const SkPoint pos[],
const SkPaint& paint) {
if (byteLength) {
NOTIFY_SETUP(this);
this->writePaint(paint);
int count = paint.textToGlyphs(text, byteLength, NULL);
if (this->needOpBytes(4 + SkAlign4(byteLength) + 4 + count * sizeof(SkPoint))) {
@ -882,7 +861,6 @@ void SkGPipeCanvas::onDrawPosText(const void* text, size_t byteLength, const SkP
void SkGPipeCanvas::onDrawPosTextH(const void* text, size_t byteLength, const SkScalar xpos[],
SkScalar constY, const SkPaint& paint) {
if (byteLength) {
NOTIFY_SETUP(this);
this->writePaint(paint);
int count = paint.textToGlyphs(text, byteLength, NULL);
if (this->needOpBytes(4 + SkAlign4(byteLength) + 4 + count * sizeof(SkScalar) + 4)) {
@ -899,7 +877,6 @@ void SkGPipeCanvas::onDrawPosTextH(const void* text, size_t byteLength, const Sk
void SkGPipeCanvas::onDrawTextOnPath(const void* text, size_t byteLength, const SkPath& path,
const SkMatrix* matrix, const SkPaint& paint) {
if (byteLength) {
NOTIFY_SETUP(this);
unsigned flags = 0;
size_t size = 4 + SkAlign4(byteLength) + path.writeToMemory(NULL);
if (matrix) {
@ -935,10 +912,14 @@ void SkGPipeCanvas::drawVertices(VertexMode vmode, int vertexCount,
return;
}
NOTIFY_SETUP(this);
size_t size = 4 + vertexCount * sizeof(SkPoint);
this->writePaint(paint);
unsigned flags = 0;
unsigned flags = 0; // flags pack with the op, so don't need extra space themselves
size_t size = 0;
size += 4; // vmode
size += 4; // vertexCount
size += vertexCount * sizeof(SkPoint); // vertices
if (texs) {
flags |= kDrawVertices_HasTexs_DrawOpFlag;
size += vertexCount * sizeof(SkPoint);
@ -947,13 +928,14 @@ void SkGPipeCanvas::drawVertices(VertexMode vmode, int vertexCount,
flags |= kDrawVertices_HasColors_DrawOpFlag;
size += vertexCount * sizeof(SkColor);
}
if (indices && indexCount > 0) {
flags |= kDrawVertices_HasIndices_DrawOpFlag;
size += 4 + SkAlign4(indexCount * sizeof(uint16_t));
}
if (xfer && !SkXfermode::IsMode(xfer, SkXfermode::kModulate_Mode)) {
flags |= kDrawVertices_HasXfermode_DrawOpFlag;
size += sizeof(int32_t); // mode enum
size += sizeof(int32_t); // SkXfermode::Mode
}
if (indices && indexCount > 0) {
flags |= kDrawVertices_HasIndices_DrawOpFlag;
size += 4; // indexCount
size += SkAlign4(indexCount * sizeof(uint16_t)); // indices
}
if (this->needOpBytes(size)) {
@ -961,18 +943,18 @@ void SkGPipeCanvas::drawVertices(VertexMode vmode, int vertexCount,
fWriter.write32(vmode);
fWriter.write32(vertexCount);
fWriter.write(vertices, vertexCount * sizeof(SkPoint));
if (texs) {
if (flags & kDrawVertices_HasTexs_DrawOpFlag) {
fWriter.write(texs, vertexCount * sizeof(SkPoint));
}
if (colors) {
if (flags & kDrawVertices_HasColors_DrawOpFlag) {
fWriter.write(colors, vertexCount * sizeof(SkColor));
}
if (flags & kDrawVertices_HasXfermode_DrawOpFlag) {
SkXfermode::Mode mode = SkXfermode::kModulate_Mode;
(void)xfer->asMode(&mode);
SkAssertResult(xfer->asMode(&mode));
fWriter.write32(mode);
}
if (indices && indexCount > 0) {
if (flags & kDrawVertices_HasIndices_DrawOpFlag) {
fWriter.write32(indexCount);
fWriter.writePad(indices, indexCount * sizeof(uint16_t));
}
@ -981,7 +963,6 @@ void SkGPipeCanvas::drawVertices(VertexMode vmode, int vertexCount,
void SkGPipeCanvas::drawData(const void* ptr, size_t size) {
if (size && ptr) {
NOTIFY_SETUP(this);
unsigned data = 0;
if (size < (1 << DRAWOPS_DATA_BITS)) {
data = (unsigned)size;
@ -1009,7 +990,7 @@ void SkGPipeCanvas::endCommentGroup() {
}
void SkGPipeCanvas::flushRecording(bool detachCurrentBlock) {
doNotify();
this->doNotify();
if (detachCurrentBlock) {
// force a new block to be requested for the next recorded command
fBlockSize = 0;
@ -1249,3 +1230,4 @@ void BitmapShuttle::removeCanvas() {
fCanvas->unref();
fCanvas = NULL;
}

17
src/pipe/utils/SamplePipeControllers.cpp
View File

@ -13,23 +13,16 @@
#include "SkMatrix.h"
PipeController::PipeController(SkCanvas* target, SkPicture::InstallPixelRefProc proc)
:fReader(target) {
fBlock = NULL;
fBlockSize = fBytesWritten = 0;
: fReader(target), fBlockSize(0), fBytesWritten(0) {
fReader.setBitmapDecoder(proc);
}
PipeController::~PipeController() {
sk_free(fBlock);
}
void* PipeController::requestBlock(size_t minRequest, size_t *actual) {
sk_free(fBlock);
fBlockSize = minRequest * 4;
fBlock = sk_malloc_throw(fBlockSize);
void* PipeController::requestBlock(size_t minRequest, size_t* actual) {
fBlockSize = minRequest;
fBlock.reset(fBlockSize);
fBytesWritten = 0;
*actual = fBlockSize;
return fBlock;
return fBlock.get();
}
void PipeController::notifyWritten(size_t bytes) {

6
src/pipe/utils/SamplePipeControllers.h
View File

@ -17,14 +17,14 @@ class SkMatrix;
class PipeController : public SkGPipeController {
public:
PipeController(SkCanvas* target, SkPicture::InstallPixelRefProc proc = NULL);
virtual ~PipeController();
virtual void* requestBlock(size_t minRequest, size_t* actual) SK_OVERRIDE;
virtual void notifyWritten(size_t bytes) SK_OVERRIDE;
protected:
const void* getData() { return (const char*) fBlock + fBytesWritten; }
const void* getData() { return (const char*) fBlock.get() + fBytesWritten; }
SkGPipeReader fReader;
private:
void* fBlock;
SkAutoMalloc fBlock;
size_t fBlockSize;
size_t fBytesWritten;
SkGPipeReader::Status fStatus;

4
tests/DeferredCanvasTest.cpp
View File

@ -547,7 +547,11 @@ static void TestDeferredCanvasBitmapCaching(skiatest::Reporter* reporter) {
canvas->drawBitmap(sourceImages[0], 0, 0, NULL);
REPORTER_ASSERT(reporter, 2 == notificationCounter.fStorageAllocatedChangedCount);
canvas->drawBitmap(sourceImages[0], 0, 0, NULL);
#ifdef SK_DEBUG
REPORTER_ASSERT(reporter, 2 == notificationCounter.fStorageAllocatedChangedCount);
#else
REPORTER_ASSERT(reporter, 3 == notificationCounter.fStorageAllocatedChangedCount);
#endif
REPORTER_ASSERT(reporter, 1 == notificationCounter.fFlushedDrawCommandsCount);
REPORTER_ASSERT(reporter, canvas->storageAllocatedForRecording() < 2 * bitmapSize);