Add flag to switch op allocation to new
Added a flag to switch from using the memory pool to
using new and delete for GrOp allocation.
Just add the following to your gn args.
extra_cflags = [
"-DGR_OP_ALLOCATE_USE_NEW",
]
Change-Id: Icea4a6df047cff2cd5e50032f0bd4b714a5740d4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/322625
Commit-Queue: Herb Derby <herb@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
diff --git a/src/gpu/GrMemoryPool.cpp b/src/gpu/GrMemoryPool.cpp
index de7349a..2cdb6fa 100644
--- a/src/gpu/GrMemoryPool.cpp
+++ b/src/gpu/GrMemoryPool.cpp
@@ -132,9 +132,11 @@
return std::unique_ptr<GrOpMemoryPool>(new (mem) GrOpMemoryPool(preallocSize, minAllocSize));
}
-void GrOpMemoryPool::release(std::unique_ptr<GrOp> op) {
- GrOp* tmp = op.release();
- SkASSERT(tmp);
- tmp->~GrOp();
- fPool.release(tmp);
-}
+#if !defined(GR_OP_ALLOCATE_USE_NEW)
+ void GrOpMemoryPool::release(std::unique_ptr<GrOp> op) {
+ GrOp* tmp = op.release();
+ SkASSERT(tmp);
+ tmp->~GrOp();
+ fPool.release(tmp);
+ }
+#endif
diff --git a/src/gpu/GrMemoryPool.h b/src/gpu/GrMemoryPool.h
index 8650a74..2543fe5 100644
--- a/src/gpu/GrMemoryPool.h
+++ b/src/gpu/GrMemoryPool.h
@@ -121,14 +121,21 @@
template <typename Op, typename... OpArgs>
std::unique_ptr<Op> allocate(OpArgs&&... opArgs) {
- auto mem = this->allocate(sizeof(Op));
- return std::unique_ptr<Op>(new (mem) Op(std::forward<OpArgs>(opArgs)...));
+ #if defined(GR_OP_ALLOCATE_USE_NEW)
+ void* m = ::operator new(sizeof(Op));
+ Op* op = new (m) Op(std::forward<OpArgs>(opArgs)...);
+ return std::unique_ptr<Op>(op);
+ #else
+ auto mem = this->allocate(sizeof(Op));
+ return std::unique_ptr<Op>(new (mem) Op(std::forward<OpArgs>(opArgs)...));
+ #endif
}
void* allocate(size_t size) { return fPool.allocate(size); }
- void release(std::unique_ptr<GrOp> op);
-
+ #if !defined(GR_OP_ALLOCATE_USE_NEW)
+ void release(std::unique_ptr<GrOp> op);
+ #endif
bool isEmpty() const { return fPool.isEmpty(); }
private:
diff --git a/src/gpu/GrOpsTask.cpp b/src/gpu/GrOpsTask.cpp
index 64145d4..65241cd 100644
--- a/src/gpu/GrOpsTask.cpp
+++ b/src/gpu/GrOpsTask.cpp
@@ -154,7 +154,12 @@
void GrOpsTask::OpChain::deleteOps(GrOpMemoryPool* pool) {
while (!fList.empty()) {
- pool->release(fList.popHead());
+ #if defined(GR_OP_ALLOCATE_USE_NEW)
+ // Since the value goes out of scope immediately, the unique_ptr deletes the op.
+ fList.popHead();
+ #else
+ pool->release(fList.popHead());
+ #endif
}
}
@@ -197,7 +202,12 @@
if (merged) {
GR_AUDIT_TRAIL_OPS_RESULT_COMBINED(auditTrail, a, chainB.head());
if (canBackwardMerge) {
- arenas->opMemoryPool()->release(chainB.popHead());
+ #if defined(GR_OP_ALLOCATE_USE_NEW)
+ // The unique_ptr releases the op.
+ chainB.popHead();
+ #else
+ arenas->opMemoryPool()->release(chainB.popHead());
+ #endif
} else {
// We merged the contents of b's head into a. We will replace b's head with a in
// chain b.
@@ -206,7 +216,12 @@
origATail = a->prevInChain();
}
std::unique_ptr<GrOp> detachedA = chainA.removeOp(a);
- arenas->opMemoryPool()->release(chainB.popHead());
+ #if defined(GR_OP_ALLOCATE_USE_NEW)
+ // The unique_ptr releases the op.
+ chainB.popHead();
+ #else
+ arenas->opMemoryPool()->release(chainB.popHead());
+ #endif
chainB.pushHead(std::move(detachedA));
if (chainA.empty()) {
// We merged all the nodes in chain a to chain b.
@@ -282,7 +297,12 @@
list->tail()->name(), list->tail()->uniqueID(), list->head()->name(),
list->head()->uniqueID());
GR_AUDIT_TRAIL_OPS_RESULT_COMBINED(auditTrail, fList.tail(), list->head());
- arenas->opMemoryPool()->release(list->popHead());
+ #if defined(GR_OP_ALLOCATE_USE_NEW)
+ // The unique_ptr releases the op.
+ list->popHead();
+ #else
+ arenas->opMemoryPool()->release(list->popHead());
+ #endif
break;
}
}
@@ -796,7 +816,9 @@
// A closed GrOpsTask should never receive new/more ops
SkASSERT(!this->isClosed());
if (!op->bounds().isFinite()) {
- fArenas.opMemoryPool()->release(std::move(op));
+ #if !defined(GR_OP_ALLOCATE_USE_NEW)
+ fArenas.opMemoryPool()->release(std::move(op));
+ #endif
return;
}
diff --git a/src/gpu/GrRenderTargetContext.cpp b/src/gpu/GrRenderTargetContext.cpp
index ed0bedc..c5dcb39 100644
--- a/src/gpu/GrRenderTargetContext.cpp
+++ b/src/gpu/GrRenderTargetContext.cpp
@@ -1992,7 +1992,9 @@
const std::function<WillAddOpFn>& willAddFn) {
ASSERT_SINGLE_OWNER
if (fContext->abandoned()) {
- fContext->priv().opMemoryPool()->release(std::move(op));
+ #if !defined(GR_OP_ALLOCATE_USE_NEW)
+ fContext->priv().opMemoryPool()->release(std::move(op));
+ #endif
return;
}
SkDEBUGCODE(this->validate();)
@@ -2025,7 +2027,9 @@
}
if (skipDraw) {
- fContext->priv().opMemoryPool()->release(std::move(op));
+ #if !defined(GR_OP_ALLOCATE_USE_NEW)
+ fContext->priv().opMemoryPool()->release(std::move(op));
+ #endif
return;
}
@@ -2050,7 +2054,9 @@
GrXferProcessor::DstProxyView dstProxyView;
if (analysis.requiresDstTexture()) {
if (!this->setupDstProxyView(*op, &dstProxyView)) {
- fContext->priv().opMemoryPool()->release(std::move(op));
+ #if !defined(GR_OP_ALLOCATE_USE_NEW)
+ fContext->priv().opMemoryPool()->release(std::move(op));
+ #endif
return;
}
}
diff --git a/src/gpu/ops/GrOp.cpp b/src/gpu/ops/GrOp.cpp
index 98c88a4..ee2bd11 100644
--- a/src/gpu/ops/GrOp.cpp
+++ b/src/gpu/ops/GrOp.cpp
@@ -10,18 +10,18 @@
std::atomic<uint32_t> GrOp::gCurrOpClassID {GrOp::kIllegalOpID + 1};
std::atomic<uint32_t> GrOp::gCurrOpUniqueID{GrOp::kIllegalOpID + 1};
-#ifdef SK_DEBUG
-void* GrOp::operator new(size_t size) {
- // All GrOp-derived class should be allocated in a GrMemoryPool
- SkASSERT(0);
- return ::operator new(size);
-}
+#if !defined(GR_OP_ALLOCATE_USE_NEW) && defined(SK_DEBUG)
+ void* GrOp::operator new(size_t size) {
+ // All GrOp-derived class should be allocated in a GrMemoryPool
+ SkASSERT(0);
+ return ::operator new(size);
+ }
-void GrOp::operator delete(void* target) {
- // All GrOp-derived class should be released from their owning GrMemoryPool
- SkASSERT(0);
- ::operator delete(target);
-}
+ void GrOp::operator delete(void* target) {
+ // All GrOp-derived class should be released from their owning GrMemoryPool
+ SkASSERT(0);
+ ::operator delete(target);
+ }
#endif
GrOp::GrOp(uint32_t classID) : fClassID(classID) {
diff --git a/src/gpu/ops/GrOp.h b/src/gpu/ops/GrOp.h
index 96c3def..f1372e4 100644
--- a/src/gpu/ops/GrOp.h
+++ b/src/gpu/ops/GrOp.h
@@ -119,19 +119,24 @@
SkASSERT(fBoundsFlags != kUninitialized_BoundsFlag);
return SkToBool(fBoundsFlags & kZeroArea_BoundsFlag);
}
+ #if defined(GR_OP_ALLOCATE_USE_NEW)
+ // GrOps are allocated using ::operator new in the GrMemoryPool. Doing this style of memory
+ // allocation defeats the delete with size optimization.
+ void* operator new(size_t) { SK_ABORT("All GrOps are created by placement new."); }
+ void* operator new(size_t, void* p) { return p; }
+ void operator delete(void* p) { ::operator delete(p); }
+ #elif defined(SK_DEBUG)
+ // All GrOp-derived classes should be allocated in and deleted from a GrMemoryPool
+ void* operator new(size_t size);
+ void operator delete(void* target);
-#ifdef SK_DEBUG
- // All GrOp-derived classes should be allocated in and deleted from a GrMemoryPool
- void* operator new(size_t size);
- void operator delete(void* target);
-
- void* operator new(size_t size, void* placement) {
- return ::operator new(size, placement);
- }
- void operator delete(void* target, void* placement) {
- ::operator delete(target, placement);
- }
-#endif
+ void* operator new(size_t size, void* placement) {
+ return ::operator new(size, placement);
+ }
+ void operator delete(void* target, void* placement) {
+ ::operator delete(target, placement);
+ }
+ #endif
/**
* Helper for safely down-casting to a GrOp subclass
diff --git a/src/gpu/ops/GrSimpleMeshDrawOpHelper.h b/src/gpu/ops/GrSimpleMeshDrawOpHelper.h
index 679c6c6..feabe94 100644
--- a/src/gpu/ops/GrSimpleMeshDrawOpHelper.h
+++ b/src/gpu/ops/GrSimpleMeshDrawOpHelper.h
@@ -212,12 +212,21 @@
makeArgs.fProcessorSet = nullptr;
return pool->allocate<Op>(makeArgs, paint.getColor4f(), std::forward<OpArgs>(opArgs)...);
} else {
- char* mem = (char*) pool->allocate(sizeof(Op) + sizeof(GrProcessorSet));
- char* setMem = mem + sizeof(Op);
- auto color = paint.getColor4f();
- makeArgs.fProcessorSet = new (setMem) GrProcessorSet(std::move(paint));
- return std::unique_ptr<GrDrawOp>(new (mem) Op(makeArgs, color,
- std::forward<OpArgs>(opArgs)...));
+ #if defined(GR_OP_ALLOCATE_USE_NEW)
+ char* mem = (char*) ::operator new(sizeof(Op) + sizeof(GrProcessorSet));
+ char* setMem = mem + sizeof(Op);
+ auto color = paint.getColor4f();
+ makeArgs.fProcessorSet = new (setMem) GrProcessorSet(std::move(paint));
+ GrDrawOp* op = new (mem) Op(makeArgs, color, std::forward<OpArgs>(opArgs)...);
+ return std::unique_ptr<GrDrawOp>(op);
+ #else
+ char* mem = (char*) pool->allocate(sizeof(Op) + sizeof(GrProcessorSet));
+ char* setMem = mem + sizeof(Op);
+ auto color = paint.getColor4f();
+ makeArgs.fProcessorSet = new (setMem) GrProcessorSet(std::move(paint));
+ return std::unique_ptr<GrDrawOp>(new (mem) Op(makeArgs, color,
+ std::forward<OpArgs>(opArgs)...));
+ #endif
}
}
diff --git a/src/gpu/ops/GrTextureOp.cpp b/src/gpu/ops/GrTextureOp.cpp
index bc6c399..6120ea1 100644
--- a/src/gpu/ops/GrTextureOp.cpp
+++ b/src/gpu/ops/GrTextureOp.cpp
@@ -249,13 +249,17 @@
sk_sp<GrColorSpaceXform> textureColorSpaceXform) {
// Allocate size based on proxyRunCnt, since that determines number of ViewCountPairs.
SkASSERT(proxyRunCnt <= cnt);
-
size_t size = sizeof(TextureOp) + sizeof(ViewCountPair) * (proxyRunCnt - 1);
- GrOpMemoryPool* pool = context->priv().opMemoryPool();
- void* mem = pool->allocate(size);
+ #if defined(GR_OP_ALLOCATE_USE_NEW)
+ void* mem = ::operator new(size);
+ #else
+ GrOpMemoryPool* pool = context->priv().opMemoryPool();
+ void* mem = pool->allocate(size);
+ #endif
return std::unique_ptr<GrDrawOp>(
- new (mem) TextureOp(set, cnt, proxyRunCnt, filter, mm, saturate, aaType, constraint,
- viewMatrix, std::move(textureColorSpaceXform)));
+ new (mem) TextureOp(
+ set, cnt, proxyRunCnt, filter, mm, saturate, aaType, constraint,
+ viewMatrix, std::move(textureColorSpaceXform)));
}
~TextureOp() override {