Remove the partial-flush machinery from GrResourceAllocator
Since review.skia.org/366716 this is unused and it makes our life
a whole lot simpler. And a little faster.
Bug: skia:10877
Change-Id: Ib7205bae57ce282f0e4f33c7c780a4c3e5159ea5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/369436
Commit-Queue: Adlai Holler <adlai@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
diff --git a/src/gpu/GrDrawingManager.cpp b/src/gpu/GrDrawingManager.cpp
index 0648771..bd3ae75 100644
--- a/src/gpu/GrDrawingManager.cpp
+++ b/src/gpu/GrDrawingManager.cpp
@@ -60,7 +60,7 @@
GrDrawingManager::~GrDrawingManager() {
this->closeAllTasks();
- this->removeRenderTasks(0, fDAG.count());
+ this->removeRenderTasks();
}
bool GrDrawingManager::wasAbandoned() const {
@@ -202,58 +202,38 @@
}
#endif
- int startIndex, stopIndex;
bool flushed = false;
{
GrResourceAllocator alloc(resourceProvider SkDEBUGCODE(, fDAG.count()));
- for (int i = 0; i < fDAG.count(); ++i) {
- if (fDAG[i]) {
- fDAG[i]->gatherProxyIntervals(&alloc);
- }
- alloc.markEndOfOpsTask(i);
+ for (const auto& task : fDAG) {
+ SkASSERT(task);
+ task->gatherProxyIntervals(&alloc);
}
- alloc.determineRecyclability();
GrResourceAllocator::AssignError error = GrResourceAllocator::AssignError::kNoError;
- int numRenderTasksExecuted = 0;
- if (alloc.assign(&startIndex, &stopIndex, &error)) {
- // TODO: If this always-execute-everything approach works out, remove the startIndex &
- // stopIndex from the GrResourceProvider API and simplify.
- SkASSERT(startIndex == 0 && stopIndex == fDAG.count());
- if (GrResourceAllocator::AssignError::kFailedProxyInstantiation == error) {
- for (int i = startIndex; i < stopIndex; ++i) {
- GrRenderTask* renderTask = fDAG[i].get();
- if (!renderTask) {
- continue;
- }
- if (!renderTask->isInstantiated()) {
- // No need to call the renderTask's handleInternalAllocationFailure
- // since we will already skip executing the renderTask since it is not
- // instantiated.
- continue;
- }
- renderTask->handleInternalAllocationFailure();
+ alloc.assign(&error);
+ if (GrResourceAllocator::AssignError::kFailedProxyInstantiation == error) {
+ for (const auto& renderTask : fDAG) {
+ SkASSERT(renderTask);
+ if (!renderTask->isInstantiated()) {
+ // No need to call the renderTask's handleInternalAllocationFailure
+ // since we will already skip executing the renderTask since it is not
+ // instantiated.
+ continue;
}
- this->removeRenderTasks(startIndex, stopIndex);
+ // TODO: If we're going to remove all the render tasks do we really need this call?
+ renderTask->handleInternalAllocationFailure();
}
+ this->removeRenderTasks();
+ }
- if (this->executeRenderTasks(
- startIndex, stopIndex, &flushState, &numRenderTasksExecuted)) {
- flushed = true;
- }
+ if (this->executeRenderTasks(&flushState)) {
+ flushed = true;
}
}
-#ifdef SK_DEBUG
- for (const auto& task : fDAG) {
- // All render tasks should have been cleared out by now – we only reset the array below to
- // reclaim storage.
- SkASSERT(!task);
- }
-#endif
- fLastRenderTasks.reset();
- fDAG.reset();
+ SkASSERT(fDAG.empty());
#ifdef SK_DEBUG
// In non-DDL mode this checks that all the flushed ops have been freed from the memory pool.
@@ -297,14 +277,10 @@
return gpu->submitToGpu(syncToCpu);
}
-bool GrDrawingManager::executeRenderTasks(int startIndex, int stopIndex, GrOpFlushState* flushState,
- int* numRenderTasksExecuted) {
- SkASSERT(startIndex <= stopIndex && stopIndex <= fDAG.count());
-
+bool GrDrawingManager::executeRenderTasks(GrOpFlushState* flushState) {
#if GR_FLUSH_TIME_OP_SPEW
- SkDebugf("Flushing opsTask: %d to %d out of [%d, %d]\n",
- startIndex, stopIndex, 0, fDAG.count());
- for (int i = startIndex; i < stopIndex; ++i) {
+ SkDebugf("Flushing %d opsTasks\n", fDAG.count());
+ for (int i = 0; i < fDAG.count(); ++i) {
if (fDAG[i]) {
SkString label;
label.printf("task %d/%d", i, fDAG.count());
@@ -315,8 +291,7 @@
bool anyRenderTasksExecuted = false;
- for (int i = startIndex; i < stopIndex; ++i) {
- GrRenderTask* renderTask = fDAG[i].get();
+ for (const auto& renderTask : fDAG) {
if (!renderTask || !renderTask->isInstantiated()) {
continue;
}
@@ -335,6 +310,7 @@
// put a cap on the number of oplists we will execute before flushing to the GPU to relieve some
// memory pressure.
static constexpr int kMaxRenderTasksBeforeFlush = 100;
+ int numRenderTasksExecuted = 0;
// Execute the onFlush renderTasks first, if any.
for (sk_sp<GrRenderTask>& onFlushRenderTask : fOnFlushRenderTasks) {
@@ -344,28 +320,26 @@
SkASSERT(onFlushRenderTask->unique());
onFlushRenderTask->disown(this);
onFlushRenderTask = nullptr;
- (*numRenderTasksExecuted)++;
- if (*numRenderTasksExecuted >= kMaxRenderTasksBeforeFlush) {
+ if (++numRenderTasksExecuted >= kMaxRenderTasksBeforeFlush) {
flushState->gpu()->submitToGpu(false);
- *numRenderTasksExecuted = 0;
+ numRenderTasksExecuted = 0;
}
}
fOnFlushRenderTasks.reset();
// Execute the normal op lists.
- for (int i = startIndex; i < stopIndex; ++i) {
- GrRenderTask* renderTask = fDAG[i].get();
- if (!renderTask || !renderTask->isInstantiated()) {
+ for (const auto& renderTask : fDAG) {
+ SkASSERT(renderTask);
+ if (!renderTask->isInstantiated()) {
continue;
}
if (renderTask->execute(flushState)) {
anyRenderTasksExecuted = true;
}
- (*numRenderTasksExecuted)++;
- if (*numRenderTasksExecuted >= kMaxRenderTasksBeforeFlush) {
+ if (++numRenderTasksExecuted >= kMaxRenderTasksBeforeFlush) {
flushState->gpu()->submitToGpu(false);
- *numRenderTasksExecuted = 0;
+ numRenderTasksExecuted = 0;
}
}
@@ -377,17 +351,14 @@
// resources are the last to be purged by the resource cache.
flushState->reset();
- this->removeRenderTasks(startIndex, stopIndex);
+ this->removeRenderTasks();
return anyRenderTasksExecuted;
}
-void GrDrawingManager::removeRenderTasks(int startIndex, int stopIndex) {
- for (int i = startIndex; i < stopIndex; ++i) {
- GrRenderTask* task = fDAG[i].get();
- if (!task) {
- continue;
- }
+void GrDrawingManager::removeRenderTasks() {
+ for (const auto& task : fDAG) {
+ SkASSERT(task);
if (!task->unique() || task->requiresExplicitCleanup()) {
// TODO: Eventually uniqueness should be guaranteed: http://skbug.com/7111.
// DDLs, however, will always require an explicit notification for when they
@@ -395,12 +366,9 @@
task->endFlush(this);
}
task->disown(this);
-
- // This doesn't cleanup any referring pointers (e.g. dependency pointers in the DAG).
- // It works right now bc this is only called after the topological sort is complete
- // (so the dangling pointers aren't used).
- fDAG[i] = nullptr;
}
+ fDAG.reset();
+ fLastRenderTasks.reset();
}
void GrDrawingManager::sortTasks() {
diff --git a/src/gpu/GrDrawingManager.h b/src/gpu/GrDrawingManager.h
index b91dd49..24595a7 100644
--- a/src/gpu/GrDrawingManager.h
+++ b/src/gpu/GrDrawingManager.h
@@ -149,10 +149,9 @@
void closeActiveOpsTask();
// return true if any GrRenderTasks were actually executed; false otherwise
- bool executeRenderTasks(int startIndex, int stopIndex, GrOpFlushState*,
- int* numRenderTasksExecuted);
+ bool executeRenderTasks(GrOpFlushState*);
- void removeRenderTasks(int startIndex, int stopIndex);
+ void removeRenderTasks();
void sortTasks();
void reorderTasks();
diff --git a/src/gpu/GrResourceAllocator.cpp b/src/gpu/GrResourceAllocator.cpp
index 37d9965..29bf950 100644
--- a/src/gpu/GrResourceAllocator.cpp
+++ b/src/gpu/GrResourceAllocator.cpp
@@ -49,19 +49,6 @@
}
}
-void GrResourceAllocator::markEndOfOpsTask(int opsTaskIndex) {
- SkASSERT(!fAssigned); // We shouldn't be adding any opsTasks after (or during) assignment
-
- SkASSERT(fEndOfOpsTaskOpIndices.count() == opsTaskIndex);
- if (!fEndOfOpsTaskOpIndices.empty()) {
- SkASSERT(fEndOfOpsTaskOpIndices.back() < this->curOp());
- }
-
- // This is the first op index of the next opsTask
- fEndOfOpsTaskOpIndices.push_back(this->curOp());
- SkASSERT(fEndOfOpsTaskOpIndices.count() <= fNumOpsTasks);
-}
-
GrResourceAllocator::~GrResourceAllocator() {
SkASSERT(fIntvlList.empty());
SkASSERT(fActiveIntvls.empty());
@@ -306,48 +293,28 @@
}
}
-bool GrResourceAllocator::assign(int* startIndex, int* stopIndex, AssignError* outError) {
+void GrResourceAllocator::assign(AssignError* outError) {
SkASSERT(outError);
*outError = fLazyInstantiationError ? AssignError::kFailedProxyInstantiation
: AssignError::kNoError;
- SkASSERT(fNumOpsTasks == fEndOfOpsTaskOpIndices.count());
-
fIntvlHash.reset(); // we don't need the interval hash anymore
- if (fCurOpsTaskIndex >= fEndOfOpsTaskOpIndices.count()) {
- return false; // nothing to render
- }
-
- *startIndex = fCurOpsTaskIndex;
- *stopIndex = fEndOfOpsTaskOpIndices.count();
-
- if (fIntvlList.empty()) {
- fCurOpsTaskIndex = fEndOfOpsTaskOpIndices.count();
- return true; // no resources to assign
- }
-
-#if GR_ALLOCATION_SPEW
- SkDebugf("assigning opsTasks %d through %d out of %d numOpsTasks\n",
- *startIndex, *stopIndex, fNumOpsTasks);
- SkDebugf("EndOfOpsTaskIndices: ");
- for (int i = 0; i < fEndOfOpsTaskOpIndices.count(); ++i) {
- SkDebugf("%d ", fEndOfOpsTaskOpIndices[i]);
- }
- SkDebugf("\n");
-#endif
-
SkDEBUGCODE(fAssigned = true;)
+ if (fIntvlList.empty()) {
+ return; // no resources to assign
+ }
+
#if GR_ALLOCATION_SPEW
+ SkDebugf("assigning %d ops\n", fNumOps);
this->dumpIntervals();
#endif
- while (Interval* cur = fIntvlList.popHead()) {
- while (fEndOfOpsTaskOpIndices[fCurOpsTaskIndex] <= cur->start()) {
- fCurOpsTaskIndex++;
- SkASSERT(fCurOpsTaskIndex < fNumOpsTasks);
- }
+ // TODO: Can this be done inline during the main iteration?
+ this->determineRecyclability();
+
+ while (Interval* cur = fIntvlList.popHead()) {
this->expire(cur->start());
if (cur->proxy()->isInstantiated()) {
@@ -389,7 +356,6 @@
// expire all the remaining intervals to drain the active interval list
this->expire(std::numeric_limits<unsigned int>::max());
- return true;
}
#if GR_ALLOCATION_SPEW
diff --git a/src/gpu/GrResourceAllocator.h b/src/gpu/GrResourceAllocator.h
index 92c2180..8b7071e 100644
--- a/src/gpu/GrResourceAllocator.h
+++ b/src/gpu/GrResourceAllocator.h
@@ -68,7 +68,7 @@
class GrResourceAllocator {
public:
GrResourceAllocator(GrResourceProvider* resourceProvider SkDEBUGCODE(, int numOpsTasks))
- : fResourceProvider(resourceProvider) SkDEBUGCODE(, fNumOpsTasks(numOpsTasks)) {}
+ : fResourceProvider(resourceProvider) {}
~GrResourceAllocator();
@@ -95,16 +95,9 @@
kFailedProxyInstantiation
};
- // Returns true when the opsTasks from 'startIndex' to 'stopIndex' should be executed;
- // false when nothing remains to be executed.
// If any proxy fails to instantiate, the AssignError will be set to kFailedProxyInstantiation.
// If this happens, the caller should remove all ops which reference an uninstantiated proxy.
- // This is used to execute a portion of the queued opsTasks in order to reduce the total
- // amount of GPU resources required.
- bool assign(int* startIndex, int* stopIndex, AssignError* outError);
-
- void determineRecyclability();
- void markEndOfOpsTask(int opsTaskIndex);
+ void assign(AssignError* outError);
#if GR_ALLOCATION_SPEW
void dumpIntervals();
@@ -120,6 +113,8 @@
void recycleSurface(sk_sp<GrSurface> surface);
sk_sp<GrSurface> findSurfaceFor(const GrSurfaceProxy* proxy);
+ void determineRecyclability();
+
struct FreePoolTraits {
static const GrScratchKey& GetKey(const GrSurface& s) {
return s.resourcePriv().getScratchKey();
@@ -206,6 +201,7 @@
static uint32_t Hash(const uint32_t& key) { return key; }
private:
+ // TODO: Do we really need this variable?
sk_sp<GrSurface> fAssignedSurface;
GrSurfaceProxy* fProxy;
uint32_t fProxyID; // This is here b.c. DynamicHash requires a ref to the key
@@ -259,9 +255,6 @@
IntervalList fActiveIntvls; // List of live intervals during assignment
// (sorted by increasing end)
unsigned int fNumOps = 0;
- SkTArray<unsigned int> fEndOfOpsTaskOpIndices;
- int fCurOpsTaskIndex = 0;
- SkDEBUGCODE(const int fNumOpsTasks = -1;)
SkDEBUGCODE(bool fAssigned = false;)