Skip to content

Commit

Permalink
Refactor the ResourceCache class to prevent potential crashes when re…
Browse files Browse the repository at this point in the history
…leasing resources in multiple threads. (#90)
  • Loading branch information
domchen authored Jan 4, 2024
1 parent 4e16416 commit f9399dc
Show file tree
Hide file tree
Showing 10 changed files with 85 additions and 148 deletions.
2 changes: 0 additions & 2 deletions include/tgfx/gpu/Context.h
Original file line number Diff line number Diff line change
Expand Up @@ -184,8 +184,6 @@ class Context {
ProxyProvider* _proxyProvider = nullptr;

void releaseAll(bool releaseGPU);
void onLocked();
void onUnlocked();

friend class Device;

Expand Down
7 changes: 5 additions & 2 deletions include/tgfx/gpu/Resource.h
Original file line number Diff line number Diff line change
Expand Up @@ -86,21 +86,24 @@ class Resource {
}

private:
std::weak_ptr<Resource> weakThis;
std::shared_ptr<Resource> reference;
ScratchKey scratchKey = {};
UniqueKey uniqueKey = {};
std::atomic_uint32_t uniqueKeyGeneration = 0;
std::list<Resource*>* cachedList = nullptr;
std::list<Resource*>::iterator cachedPosition;
int64_t lastUsedTime = 0;

bool isPurgeable() const {
return weakThis.expired();
return reference.use_count() <= 1;
}

bool hasValidUniqueKey() const {
return !uniqueKey.empty() && !uniqueKey.unique() && uniqueKey.uniqueID() == uniqueKeyGeneration;
}

void release(bool releaseGPU);

/**
* Overridden to free GPU resources in the backend API.
*/
Expand Down
15 changes: 4 additions & 11 deletions include/tgfx/gpu/ResourceCache.h
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ class ResourceCache {
void purgeNotUsedSince(int64_t purgeTime, bool scratchResourcesOnly = false);

/**
* Purge unreferenced resources from the cache until the the provided bytesLimit has been reached
* Purge unreferenced resources from the cache until the provided bytesLimit has been reached,
* or we have purged all unreferenced resources. Returns true if the total resource bytes is not
* over the specified bytesLimit after purging.
* @param bytesLimit The desired number of bytes after puring.
Expand All @@ -103,24 +103,18 @@ class ResourceCache {
size_t maxBytes = 0;
size_t totalBytes = 0;
size_t purgeableBytes = 0;
bool purgingResource = false;
std::vector<std::shared_ptr<Resource>> strongReferences = {};
std::list<Resource*> nonpurgeableResources = {};
std::list<Resource*> purgeableResources = {};
ScratchKeyMap<std::vector<Resource*>> scratchKeyMap = {};
std::unordered_map<uint32_t, Resource*> uniqueKeyMap = {};
std::mutex removeLocker = {};
std::vector<Resource*> pendingPurgeableResources = {};

static void AddToList(std::list<Resource*>& list, Resource* resource);
static void RemoveFromList(std::list<Resource*>& list, Resource* resource);
static void NotifyReferenceReachedZero(Resource* resource);
static bool InList(const std::list<Resource*>& list, Resource* resource);

void attachToCurrentThread();
void detachFromCurrentThread();
void releaseAll(bool releaseGPU);
void processUnreferencedResource(Resource* resource);
std::shared_ptr<Resource> wrapResource(Resource* resource);
void processUnreferencedResources();
std::shared_ptr<Resource> refResource(Resource* resource);
std::shared_ptr<Resource> addResource(Resource* resource);
void removeResource(Resource* resource);
void purgeResourcesByLRU(bool scratchResourcesOnly,
Expand All @@ -132,6 +126,5 @@ class ResourceCache {

friend class Resource;
friend class Context;
friend class PurgeGuard;
};
} // namespace tgfx
26 changes: 15 additions & 11 deletions src/gpu/Context.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,24 @@ Context::~Context() {
}

bool Context::flush(BackendSemaphore* signalSemaphore) {
// Clean up all unreferenced resources before flushing, allowing them to be reused. This is
// particularly crucial for texture resources that are bound to render targets. Only after the
// cleanup can they be unbound and reused.
_resourceCache->processUnreferencedResources();
auto flushed = _drawingManager->flush();
if (signalSemaphore == nullptr) {
return false;
}
auto semaphore = Semaphore::Wrap(signalSemaphore);
auto success = _drawingManager->flush(semaphore.get());
if (signalSemaphore != nullptr) {
auto semaphoreInserted = caps()->semaphoreSupport && _gpu->insertSemaphore(semaphore.get());
if (semaphoreInserted) {
*signalSemaphore = semaphore->getBackendSemaphore();
}
return success;
if (flushed) {
// Clean up all unreferenced resources after flushing to reduce memory usage.
_resourceCache->processUnreferencedResources();
}
return semaphoreInserted;
}

bool Context::submit(bool syncCpu) {
Expand All @@ -73,14 +85,6 @@ bool Context::wait(const BackendSemaphore& waitSemaphore) {
return caps()->semaphoreSupport && _gpu->waitSemaphore(semaphore.get());
}

void Context::onLocked() {
_resourceCache->attachToCurrentThread();
}

void Context::onUnlocked() {
_resourceCache->detachFromCurrentThread();
}

size_t Context::memoryUsage() const {
return _resourceCache->getResourceBytes();
}
Expand Down
2 changes: 0 additions & 2 deletions src/gpu/Device.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,11 @@ Context* Device::lockContext() {
locker.unlock();
return nullptr;
}
context->onLocked();
return context;
}

void Device::unlock() {
if (contextLocked) {
context->onUnlocked();
contextLocked = false;
onUnlockContext();
}
Expand Down
9 changes: 6 additions & 3 deletions src/gpu/DrawingManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,10 @@ std::shared_ptr<OpsTask> DrawingManager::newOpsTask(
return opsTask;
}

bool DrawingManager::flush(Semaphore* signalSemaphore) {
auto* gpu = context->gpu();
bool DrawingManager::flush() {
if (tasks.empty()) {
return false;
}
closeAllTasks();
activeOpsTask = nullptr;
std::vector<ProxyBase*> proxies = {};
Expand All @@ -64,10 +66,11 @@ bool DrawingManager::flush(Semaphore* signalSemaphore) {
LOGE("DrawingManager::flush() Failed to instantiate proxy!");
}
});
auto gpu = context->gpu();
std::for_each(tasks.begin(), tasks.end(),
[gpu](std::shared_ptr<RenderTask>& task) { task->execute(gpu); });
removeAllTasks();
return context->caps()->semaphoreSupport && gpu->insertSemaphore(signalSemaphore);
return true;
}

void DrawingManager::closeAllTasks() {
Expand Down
5 changes: 4 additions & 1 deletion src/gpu/DrawingManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,10 @@ class DrawingManager {

std::shared_ptr<OpsTask> newOpsTask(std::shared_ptr<RenderTargetProxy> renderTargetProxy);

bool flush(Semaphore* signalSemaphore);
/**
* Returns true if any render tasks were executed.
*/
bool flush();

void newTextureResolveRenderTask(std::shared_ptr<RenderTargetProxy> renderTargetProxy);

Expand Down
10 changes: 10 additions & 0 deletions src/gpu/Resource.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,4 +42,14 @@ void Resource::markUniqueKeyExpired() {
bool Resource::hasUniqueKey(const UniqueKey& newKey) const {
return !newKey.empty() && newKey.uniqueID() == uniqueKeyGeneration;
}

void Resource::release(bool releaseGPU) {
if (releaseGPU) {
onReleaseGPU();
}
context = nullptr;
// Set the reference to nullptr, allowing the resource to be deleted immediately or later when the
// last external reference is released.
reference = nullptr;
}
} // namespace tgfx
Loading

0 comments on commit f9399dc

Please sign in to comment.