remove the OpMemoryPool
Change-Id: I987384f8009445ba8308e85b75daf4880a8d36be
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/383957
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Herb Derby <herb@google.com>
diff --git a/src/gpu/GrDirectContextPriv.h b/src/gpu/GrDirectContextPriv.h
index 6299549..a6a8c02 100644
--- a/src/gpu/GrDirectContextPriv.h
+++ b/src/gpu/GrDirectContextPriv.h
@@ -53,7 +53,6 @@
// from GrRecordingContext
GrDrawingManager* drawingManager() { return fContext->drawingManager(); }
- GrMemoryPool* opMemoryPool() { return fContext->arenas().opMemoryPool(); }
SkArenaAlloc* recordTimeAllocator() { return fContext->arenas().recordTimeAllocator(); }
GrRecordingContext::Arenas arenas() { return fContext->arenas(); }
diff --git a/src/gpu/GrDrawingManager.cpp b/src/gpu/GrDrawingManager.cpp
index 9077e2c..8ff9ef1 100644
--- a/src/gpu/GrDrawingManager.cpp
+++ b/src/gpu/GrDrawingManager.cpp
@@ -215,15 +215,6 @@
}
this->removeRenderTasks();
-#ifdef SK_DEBUG
- // In non-DDL mode this checks that all the flushed ops have been freed from the memory pool.
- // When we move to partial flushes this assert will no longer be valid.
- // In DDL mode this check is somewhat superfluous since the memory for most of the ops/opsTasks
- // will be stored in the DDL's GrOpMemoryPools.
- GrMemoryPool* opMemoryPool = fContext->priv().opMemoryPool();
- opMemoryPool->isEmpty();
-#endif
-
gpu->executeFlushInfo(proxies, access, info, newState);
// Give the cache a chance to purge resources that become purgeable due to flushing.
diff --git a/src/gpu/GrRecordingContext.cpp b/src/gpu/GrRecordingContext.cpp
index d5a5ee2..8b9f622 100644
--- a/src/gpu/GrRecordingContext.cpp
+++ b/src/gpu/GrRecordingContext.cpp
@@ -96,14 +96,11 @@
fDrawingManager.reset();
}
-GrRecordingContext::Arenas::Arenas(GrMemoryPool* opMemoryPool,
- SkArenaAlloc* recordTimeAllocator,
+GrRecordingContext::Arenas::Arenas(SkArenaAlloc* recordTimeAllocator,
GrSubRunAllocator* subRunAllocator)
- : fOpMemoryPool(opMemoryPool)
- , fRecordTimeAllocator(recordTimeAllocator)
+ : fRecordTimeAllocator(recordTimeAllocator)
, fRecordTimeSubRunAllocator(subRunAllocator) {
// OwnedArenas should instantiate these before passing the bare pointer off to this struct.
- SkASSERT(opMemoryPool);
SkASSERT(recordTimeAllocator);
SkASSERT(subRunAllocator);
}
@@ -114,20 +111,12 @@
GrRecordingContext::OwnedArenas::~OwnedArenas() {}
GrRecordingContext::OwnedArenas& GrRecordingContext::OwnedArenas::operator=(OwnedArenas&& a) {
- fOpMemoryPool = std::move(a.fOpMemoryPool);
fRecordTimeAllocator = std::move(a.fRecordTimeAllocator);
fRecordTimeSubRunAllocator = std::move(a.fRecordTimeSubRunAllocator);
return *this;
}
GrRecordingContext::Arenas GrRecordingContext::OwnedArenas::get() {
- if (!fOpMemoryPool) {
- // DDL TODO: should the size of the memory pool be decreased in DDL mode? CPU-side memory
- // consumed in DDL mode vs. normal mode for a single skp might be a good metric of wasted
- // memory.
- fOpMemoryPool = GrMemoryPool::Make(16384, 16384);
- }
-
if (!fRecordTimeAllocator) {
// TODO: empirically determine a better number for SkArenaAlloc's firstHeapAllocation param
fRecordTimeAllocator = std::make_unique<SkArenaAlloc>(sizeof(GrPipeline) * 100);
@@ -137,7 +126,7 @@
fRecordTimeSubRunAllocator = std::make_unique<GrSubRunAllocator>();
}
- return {fOpMemoryPool.get(), fRecordTimeAllocator.get(), fRecordTimeSubRunAllocator.get()};
+ return {fRecordTimeAllocator.get(), fRecordTimeSubRunAllocator.get()};
}
GrRecordingContext::OwnedArenas&& GrRecordingContext::detachArenas() {
diff --git a/src/gpu/GrRecordingContextPriv.h b/src/gpu/GrRecordingContextPriv.h
index 3c1e5f2..08e1c6e 100644
--- a/src/gpu/GrRecordingContextPriv.h
+++ b/src/gpu/GrRecordingContextPriv.h
@@ -41,7 +41,6 @@
// from GrRecordingContext
GrDrawingManager* drawingManager() { return fContext->drawingManager(); }
- GrMemoryPool* opMemoryPool() { return fContext->arenas().opMemoryPool(); }
SkArenaAlloc* recordTimeAllocator() { return fContext->arenas().recordTimeAllocator(); }
GrSubRunAllocator* recordTimeSubRunAllocator() {
return fContext->arenas().recordTimeSubRunAllocator();
diff --git a/src/gpu/ops/GrAtlasTextOp.cpp b/src/gpu/ops/GrAtlasTextOp.cpp
index 3b037ba..b2b9b25 100644
--- a/src/gpu/ops/GrAtlasTextOp.cpp
+++ b/src/gpu/ops/GrAtlasTextOp.cpp
@@ -35,7 +35,7 @@
#include <utility>
// If we have thread local, then cache memory for a single GrAtlasTextOp.
-#if !defined(GR_OP_ALLOCATE_USE_POOL) && defined(GR_HAS_THREAD_LOCAL)
+#if defined(GR_HAS_THREAD_LOCAL)
static thread_local void* gCache = nullptr;
void* GrAtlasTextOp::operator new(size_t s) {
if (gCache != nullptr) {
diff --git a/src/gpu/ops/GrAtlasTextOp.h b/src/gpu/ops/GrAtlasTextOp.h
index 3a2406d..c586888 100644
--- a/src/gpu/ops/GrAtlasTextOp.h
+++ b/src/gpu/ops/GrAtlasTextOp.h
@@ -30,12 +30,10 @@
}
}
-#if !defined(GR_OP_ALLOCATE_USE_POOL) && defined(GR_HAS_THREAD_LOCAL)
+#if defined(GR_HAS_THREAD_LOCAL)
void* operator new(size_t s);
void operator delete(void* b) noexcept;
static void ClearCache();
-#else
- static void ClearCache() {}
#endif
static const int kVerticesPerGlyph = GrAtlasSubRun::kVerticesPerGlyph;
diff --git a/src/gpu/ops/GrOp.cpp b/src/gpu/ops/GrOp.cpp
index 9b1fda3..994cddf 100644
--- a/src/gpu/ops/GrOp.cpp
+++ b/src/gpu/ops/GrOp.cpp
@@ -10,29 +10,6 @@
std::atomic<uint32_t> GrOp::gCurrOpClassID {GrOp::kIllegalOpID + 1};
std::atomic<uint32_t> GrOp::gCurrOpUniqueID{GrOp::kIllegalOpID + 1};
-#if defined(GR_OP_ALLOCATE_USE_POOL)
- void GrOp::DeleteFromPool::operator() (GrOp* op) {
- if (op != nullptr) {
- op->~GrOp();
- fPool->release(op);
- }
- }
-#endif
-
-#if defined(GR_OP_ALLOCATE_USE_POOL) && 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);
- }
-#endif
-
GrOp::GrOp(uint32_t classID) : fClassID(classID) {
SkASSERT(classID == SkToU32(fClassID));
SkASSERT(classID);
diff --git a/src/gpu/ops/GrOp.h b/src/gpu/ops/GrOp.h
index 3ae4e3d..34acb43 100644
--- a/src/gpu/ops/GrOp.h
+++ b/src/gpu/ops/GrOp.h
@@ -68,28 +68,11 @@
class GrOp : private SkNoncopyable {
public:
- #if defined(GR_OP_ALLOCATE_USE_POOL)
- struct DeleteFromPool {
- DeleteFromPool() : fPool{nullptr} {}
- DeleteFromPool(GrMemoryPool* pool) : fPool{pool} {}
- void operator() (GrOp* op);
- GrMemoryPool* fPool;
- };
- using Owner = std::unique_ptr<GrOp, DeleteFromPool>;
- #else
using Owner = std::unique_ptr<GrOp>;
- #endif
template<typename Op, typename... Args>
static Owner Make(GrRecordingContext* context, Args&&... args) {
- #if defined(GR_OP_ALLOCATE_USE_POOL)
- GrMemoryPool* pool = context->priv().opMemoryPool();
- void* mem = pool->allocate(sizeof(Op));
- GrOp* op = new (mem) Op(std::forward<Args>(args)...);
- return Owner{op, pool};
- #else
- return Owner{new Op(std::forward<Args>(args)...)};
- #endif
+ return Owner{new Op(std::forward<Args>(args)...)};
}
template<typename Op, typename... Args>
@@ -97,23 +80,12 @@
GrRecordingContext* context, const SkPMColor4f& color,
GrPaint&& paint, Args&&... args);
- #if defined(GR_OP_ALLOCATE_USE_POOL)
- template<typename Op, typename... Args>
- static Owner MakeWithExtraMemory(
- GrRecordingContext* context, size_t extraSize, Args&&... args) {
- GrMemoryPool* pool = context->priv().opMemoryPool();
- void* mem = pool->allocate(sizeof(Op) + extraSize);
- GrOp* op = new (mem) Op(std::forward<Args>(args)...);
- return Owner{op, pool};
- }
- #else
- template<typename Op, typename... Args>
- static Owner MakeWithExtraMemory(
- GrRecordingContext* context, size_t extraSize, Args&&... args) {
- void* bytes = ::operator new(sizeof(Op) + extraSize);
- return Owner{new (bytes) Op(std::forward<Args>(args)...)};
- }
- #endif
+ template<typename Op, typename... Args>
+ static Owner MakeWithExtraMemory(
+ GrRecordingContext* context, size_t extraSize, Args&&... args) {
+ void* bytes = ::operator new(sizeof(Op) + extraSize);
+ return Owner{new (bytes) Op(std::forward<Args>(args)...)};
+ }
virtual ~GrOp() = default;
@@ -169,20 +141,7 @@
return SkToBool(fBoundsFlags & kZeroArea_BoundsFlag);
}
- #if defined(GR_OP_ALLOCATE_USE_POOL) && 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);
-
- void* operator new(size_t size, void* placement) {
- return ::operator new(size, placement);
- }
- void operator delete(void* target, void* placement) {
- ::operator delete(target, placement);
- }
- #else
- void operator delete(void* p) { ::operator delete(p); }
- #endif
+ void operator delete(void* p) { ::operator delete(p); }
/**
* Helper for safely down-casting to a GrOp subclass
diff --git a/src/gpu/ops/GrSimpleMeshDrawOpHelper.h b/src/gpu/ops/GrSimpleMeshDrawOpHelper.h
index a4910eb..8ea2390 100644
--- a/src/gpu/ops/GrSimpleMeshDrawOpHelper.h
+++ b/src/gpu/ops/GrSimpleMeshDrawOpHelper.h
@@ -196,18 +196,10 @@
GrOp::Owner GrOp::MakeWithProcessorSet(
GrRecordingContext* context, const SkPMColor4f& color,
GrPaint&& paint, Args&&... args) {
-#if defined(GR_OP_ALLOCATE_USE_POOL)
- GrMemoryPool* pool = context->priv().opMemoryPool();
- char* bytes = (char*)pool->allocate(sizeof(Op) + sizeof(GrProcessorSet));
- char* setMem = bytes + sizeof(Op);
- GrProcessorSet* processorSet = new (setMem) GrProcessorSet{std::move(paint)};
- return Owner{new (bytes) Op(processorSet, color, std::forward<Args>(args)...), pool};
-#else
char* bytes = (char*)::operator new(sizeof(Op) + sizeof(GrProcessorSet));
char* setMem = bytes + sizeof(Op);
GrProcessorSet* processorSet = new (setMem) GrProcessorSet{std::move(paint)};
return Owner{new (bytes) Op(processorSet, color, std::forward<Args>(args)...)};
-#endif
}
template <typename Op, typename... OpArgs>