Reimplement sw::Resource with modern C++ primitives
Bug: b/133127573
Change-Id: I834939c708dcb1a075b397a919fa63c5aec652f5
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/31593
Reviewed-by: Nicolas Capens <nicolascapens@google.com>
Tested-by: Ben Clayton <bclayton@google.com>
Kokoro-Presubmit: kokoro <noreply+kokoro@google.com>
diff --git a/src/Common/Resource.hpp b/src/Common/Resource.hpp
index 2124058..2e8c349 100644
--- a/src/Common/Resource.hpp
+++ b/src/Common/Resource.hpp
@@ -30,7 +30,7 @@
// Resource is a form of shared mutex that guards an internally allocated
// buffer. Resource has an exclusive lock mode (sw::Accessor) and lock
// count, defaulting to sw::Accessor::PUBLIC and 0, respectively.
- // Resource does treat any of the sw::Accessor enumerator lock modes
+ // Resource doesn't treat any of the sw::Accessor enumerator lock modes
// differently, all semantic meaning comes from the usage of Resource.
// You can have multiple locks in mode sw::Accessor::EXCLUSIVE.
class Resource
diff --git a/src/System/Resource.cpp b/src/System/Resource.cpp
index 3a63810..d0bd655 100644
--- a/src/System/Resource.cpp
+++ b/src/System/Resource.cpp
@@ -21,12 +21,6 @@
{
Resource::Resource(size_t bytes) : size(bytes)
{
- blocked = 0;
-
- accessor = PUBLIC;
- count = 0;
- orphaned = false;
-
buffer = allocate(bytes);
}
@@ -37,144 +31,74 @@
void *Resource::lock(Accessor claimer)
{
- criticalSection.lock();
-
- while(count > 0 && accessor != claimer)
- {
- blocked++;
- criticalSection.unlock();
-
- unblock.wait();
-
- criticalSection.lock();
- blocked--;
- }
-
- accessor = claimer;
- count++;
-
- criticalSection.unlock();
-
- return buffer;
+ std::unique_lock<std::mutex> lock(mutex);
+ return acquire(lock, claimer);
}
void *Resource::lock(Accessor relinquisher, Accessor claimer)
{
- criticalSection.lock();
+ std::unique_lock<std::mutex> lock(mutex);
// Release
- while(count > 0 && accessor == relinquisher)
+ if (count > 0 && accessor == relinquisher)
{
- count--;
-
- if(count == 0)
- {
- if(blocked)
- {
- unblock.signal();
- }
- else if(orphaned)
- {
- criticalSection.unlock();
-
- delete this;
-
- return 0;
- }
- }
+ release(lock);
}
// Acquire
- while(count > 0 && accessor != claimer)
- {
- blocked++;
- criticalSection.unlock();
-
- unblock.wait();
-
- criticalSection.lock();
- blocked--;
- }
-
- accessor = claimer;
- count++;
-
- criticalSection.unlock();
+ acquire(lock, claimer);
return buffer;
}
void Resource::unlock()
{
- criticalSection.lock();
+ std::unique_lock<std::mutex> lock(mutex);
+ release(lock);
+ }
+
+ void *Resource::acquire(std::unique_lock<std::mutex> &lock, Accessor claimer)
+ {
+ while (count > 0 && accessor != claimer)
+ {
+ blocked++;
+ released.wait(lock, [&] { return count == 0 || accessor == claimer; });
+ blocked--;
+ }
+
+ accessor = claimer;
+ count++;
+ return buffer;
+ }
+
+ void Resource::release(std::unique_lock<std::mutex> &lock)
+ {
ASSERT(count > 0);
count--;
if(count == 0)
{
- if(blocked)
+ if(orphaned)
{
- unblock.signal();
- }
- else if(orphaned)
- {
- criticalSection.unlock();
-
+ lock.unlock();
delete this;
-
return;
}
+ released.notify_one();
}
-
- criticalSection.unlock();
- }
-
- void Resource::unlock(Accessor relinquisher)
- {
- criticalSection.lock();
- ASSERT(count > 0);
-
- while(count > 0 && accessor == relinquisher)
- {
- count--;
-
- if(count == 0)
- {
- if(blocked)
- {
- unblock.signal();
- }
- else if(orphaned)
- {
- criticalSection.unlock();
-
- delete this;
-
- return;
- }
- }
- }
-
- criticalSection.unlock();
}
void Resource::destruct()
{
- criticalSection.lock();
-
- if(count == 0 && !blocked)
+ std::unique_lock<std::mutex> lock(mutex);
+ if(count == 0 && blocked == 0)
{
- criticalSection.unlock();
-
+ lock.unlock();
delete this;
-
return;
}
-
orphaned = true;
-
- criticalSection.unlock();
}
const void *Resource::data() const
diff --git a/src/System/Resource.hpp b/src/System/Resource.hpp
index 7caec8a..bff79a1 100644
--- a/src/System/Resource.hpp
+++ b/src/System/Resource.hpp
@@ -15,8 +15,7 @@
#ifndef sw_Resource_hpp
#define sw_Resource_hpp
-#include "Synchronization.hpp"
-
+#include <condition_variable>
#include <mutex>
namespace sw
@@ -29,32 +28,78 @@
EXCLUSIVE
};
+ // Resource is a form of shared mutex that guards an internally allocated
+ // buffer. Resource has an exclusive lock mode (sw::Accessor) and lock
+ // count, defaulting to sw::Accessor::PUBLIC and 0, respectively.
+ // Resource doesn't treat any of the sw::Accessor enumerator lock modes
+ // differently, all semantic meaning comes from the usage of Resource.
+ // You can have multiple locks in mode sw::Accessor::EXCLUSIVE.
class Resource
{
public:
Resource(size_t bytes);
- void destruct(); // Asynchronous destructor
+ // destruct() is an asynchronous destructor, that will atomically:
+ // When the resource is unlocked:
+ // * Delete itself.
+ // When the resource is locked:
+ // * Flag itself for deletion when next fully unlocked.
+ void destruct();
+ // lock() will atomically:
+ // When the resource is unlocked OR the lock mode equals claimer:
+ // * Increment the lock count.
+ // * Return a pointer to the buffer.
+ // When the resource is locked AND the lock mode does not equal claimer:
+ // * Block until all existing locks are released (lock count equals 0).
+ // * Switch lock mode to claimer.
+ // * Increment the lock count.
+ // * Return a pointer to the buffer.
void *lock(Accessor claimer);
- void *lock(Accessor relinquisher, Accessor claimer);
- void unlock();
- void unlock(Accessor relinquisher);
+ // lock() will atomically:
+ // When the resource is unlocked OR the lock mode equals claimer:
+ // * Increment the lock count.
+ // * Return a pointer to the buffer.
+ // When the resource is locked AND the lock mode equals relinquisher:
+ // * Release *all* existing locks (regardless of prior count).
+ // * Delete itself and return nullptr if Resource::destruct() had been called while locked.
+ // * Switch lock mode to claimer.
+ // * Increment the lock count to 1.
+ // * Return a pointer to the buffer.
+ // When the resource is locked AND the lock mode does not equal relinquisher:
+ // * Block until all existing locks are released (lock count equals 0)
+ // * Switch lock mode to claimer
+ // * Increment the lock count to 1.
+ // * Return a pointer to the buffer.
+ void *lock(Accessor relinquisher, Accessor claimer);
+
+ // unlock() will atomically:
+ // * Assert if there are no locks.
+ // * Release a single lock.
+ // * Delete itself if Resource::destruct() had been called while locked.
+ void unlock();
+
+ // data() will return the Resource's buffer pointer regardless of lock
+ // state.
const void *data() const;
+
+ // size is the size in bytes of the Resource's buffer.
const size_t size;
private:
+ void *acquire(std::unique_lock<std::mutex> &lock, Accessor claimer);
+ void release(std::unique_lock<std::mutex> &lock);
+
~Resource(); // Always call destruct() instead
- std::mutex criticalSection;
- Event unblock;
- volatile int blocked;
+ std::mutex mutex;
+ std::condition_variable released;
- volatile Accessor accessor;
- volatile int count;
- bool orphaned;
-
+ Accessor accessor = PUBLIC; // guarded by mutex
+ int blocked = 0; // guarded by mutex
+ int count = 0; // guarded by mutex
+ bool orphaned = false; // guarded by mutex
void *buffer;
};
}
