Skip to content

Commit

Permalink
Merge pull request #45314 from RandomShaper/modernize_rwlock
Browse files Browse the repository at this point in the history 
Modernize RWLock
  • Loading branch information
@akien-mga
akien-mga authored Jan 22, 2021
2 parents 2a3e771 + 8ed259b commit d39f638
Show file tree
Hide file tree
Showing 20 changed files with 103 additions and 518 deletions.
95 changes: 33 additions & 62 deletions core/io/resource.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,39 +52,39 @@ void Resource::set_path(const String &p_path, bool p_take_over) {
}

if (path_cache != "") {
ResourceCache::lock->write_lock();
ResourceCache::lock.write_lock();
ResourceCache::resources.erase(path_cache);
ResourceCache::lock->write_unlock();
ResourceCache::lock.write_unlock();
}

path_cache = "";

ResourceCache::lock->read_lock();
ResourceCache::lock.read_lock();
bool has_path = ResourceCache::resources.has(p_path);
ResourceCache::lock->read_unlock();
ResourceCache::lock.read_unlock();

if (has_path) {
if (p_take_over) {
ResourceCache::lock->write_lock();
ResourceCache::lock.write_lock();
Resource **res = ResourceCache::resources.getptr(p_path);
if (res) {
(*res)->set_name("");
}
ResourceCache::lock->write_unlock();
ResourceCache::lock.write_unlock();
} else {
ResourceCache::lock->read_lock();
ResourceCache::lock.read_lock();
bool exists = ResourceCache::resources.has(p_path);
ResourceCache::lock->read_unlock();
ResourceCache::lock.read_unlock();

ERR_FAIL_COND_MSG(exists, "Another resource is loaded from path '" + p_path + "' (possible cyclic resource inclusion).");
}
}
path_cache = p_path;

if (path_cache != "") {
ResourceCache::lock->write_lock();
ResourceCache::lock.write_lock();
ResourceCache::resources[path_cache] = this;
ResourceCache::lock->write_unlock();
ResourceCache::lock.write_unlock();
}

_change_notify("resource_path");
Expand Down Expand Up @@ -315,19 +315,15 @@ void Resource::set_as_translation_remapped(bool p_remapped) {
return;
}

if (ResourceCache::lock) {
ResourceCache::lock->write_lock();
}
ResourceCache::lock.write_lock();

if (p_remapped) {
ResourceLoader::remapped_list.add(&remapped_list);
} else {
ResourceLoader::remapped_list.remove(&remapped_list);
}

if (ResourceCache::lock) {
ResourceCache::lock->write_unlock();
}
ResourceCache::lock.write_unlock();
}

bool Resource::is_translation_remapped() const {
Expand All @@ -338,38 +334,24 @@ bool Resource::is_translation_remapped() const {
//helps keep IDs same number when loading/saving scenes. -1 clears ID and it Returns -1 when no id stored
void Resource::set_id_for_path(const String &p_path, int p_id) {
if (p_id == -1) {
if (ResourceCache::path_cache_lock) {
ResourceCache::path_cache_lock->write_lock();
}
ResourceCache::path_cache_lock.write_lock();
ResourceCache::resource_path_cache[p_path].erase(get_path());
if (ResourceCache::path_cache_lock) {
ResourceCache::path_cache_lock->write_unlock();
}
ResourceCache::path_cache_lock.write_unlock();
} else {
if (ResourceCache::path_cache_lock) {
ResourceCache::path_cache_lock->write_lock();
}
ResourceCache::path_cache_lock.write_lock();
ResourceCache::resource_path_cache[p_path][get_path()] = p_id;
if (ResourceCache::path_cache_lock) {
ResourceCache::path_cache_lock->write_unlock();
}
ResourceCache::path_cache_lock.write_unlock();
}
}

int Resource::get_id_for_path(const String &p_path) const {
if (ResourceCache::path_cache_lock) {
ResourceCache::path_cache_lock->read_lock();
}
ResourceCache::path_cache_lock.read_lock();
if (ResourceCache::resource_path_cache[p_path].has(get_path())) {
int result = ResourceCache::resource_path_cache[p_path][get_path()];
if (ResourceCache::path_cache_lock) {
ResourceCache::path_cache_lock->read_unlock();
}
ResourceCache::path_cache_lock.read_unlock();
return result;
} else {
if (ResourceCache::path_cache_lock) {
ResourceCache::path_cache_lock->read_unlock();
}
ResourceCache::path_cache_lock.read_unlock();
return -1;
}
}
Expand Down Expand Up @@ -403,9 +385,9 @@ Resource::Resource() :

Resource::~Resource() {
if (path_cache != "") {
ResourceCache::lock->write_lock();
ResourceCache::lock.write_lock();
ResourceCache::resources.erase(path_cache);
ResourceCache::lock->write_unlock();
ResourceCache::lock.write_unlock();
}
if (owners.size()) {
WARN_PRINT("Resource is still owned.");
Expand All @@ -417,18 +399,11 @@ HashMap<String, Resource *> ResourceCache::resources;
HashMap<String, HashMap<String, int>> ResourceCache::resource_path_cache;
#endif

RWLock *ResourceCache::lock = nullptr;
RWLock ResourceCache::lock;
#ifdef TOOLS_ENABLED
RWLock *ResourceCache::path_cache_lock = nullptr;
RWLock ResourceCache::path_cache_lock;
#endif

void ResourceCache::setup() {
lock = RWLock::create();
#ifdef TOOLS_ENABLED
path_cache_lock = RWLock::create();
#endif
}

void ResourceCache::clear() {
if (resources.size()) {
ERR_PRINT("Resources still in use at exit (run with --verbose for details).");
Expand All @@ -442,29 +417,25 @@ void ResourceCache::clear() {
}

resources.clear();
memdelete(lock);
#ifdef TOOLS_ENABLED
memdelete(path_cache_lock);
#endif
}

void ResourceCache::reload_externals() {
}

bool ResourceCache::has(const String &p_path) {
lock->read_lock();
lock.read_lock();
bool b = resources.has(p_path);
lock->read_unlock();
lock.read_unlock();

return b;
}

Resource *ResourceCache::get(const String &p_path) {
lock->read_lock();
lock.read_lock();

Resource **res = resources.getptr(p_path);

lock->read_unlock();
lock.read_unlock();

if (!res) {
return nullptr;
Expand All @@ -474,26 +445,26 @@ Resource *ResourceCache::get(const String &p_path) {
}

void ResourceCache::get_cached_resources(List<Ref<Resource>> *p_resources) {
lock->read_lock();
lock.read_lock();
const String *K = nullptr;
while ((K = resources.next(K))) {
Resource *r = resources[*K];
p_resources->push_back(Ref<Resource>(r));
}
lock->read_unlock();
lock.read_unlock();
}

int ResourceCache::get_cached_resource_count() {
lock->read_lock();
lock.read_lock();
int rc = resources.size();
lock->read_unlock();
lock.read_unlock();

return rc;
}

void ResourceCache::dump(const char *p_file, bool p_short) {
#ifdef DEBUG_ENABLED
lock->read_lock();
lock.read_lock();

Map<String, int> type_count;

Expand Down Expand Up @@ -530,6 +501,6 @@ void ResourceCache::dump(const char *p_file, bool p_short) {
memdelete(f);
}

lock->read_unlock();
lock.read_unlock();
#endif
}
5 changes: 2 additions & 3 deletions core/io/resource.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,16 +149,15 @@ typedef Ref<Resource> RES;
class ResourceCache {
friend class Resource;
friend class ResourceLoader; //need the lock
static RWLock *lock;
static RWLock lock;
static HashMap<String, Resource *> resources;
#ifdef TOOLS_ENABLED
static HashMap<String, HashMap<String, int>> resource_path_cache; // each tscn has a set of resource paths and IDs
static RWLock *path_cache_lock;
static RWLock path_cache_lock;
#endif // TOOLS_ENABLED
friend void unregister_core_types();
static void clear();
friend void register_core_types();
static void setup();

public:
static void reload_externals();
Expand Down
28 changes: 7 additions & 21 deletions core/io/resource_loader.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -323,9 +323,7 @@ Error ResourceLoader::load_threaded_request(const String &p_path, const String &
ERR_FAIL_V_MSG(ERR_INVALID_PARAMETER, "Attempted to load a resource already being loaded from this thread, cyclic reference?");
}
//lock first if possible
if (ResourceCache::lock) {
ResourceCache::lock->read_lock();
}
ResourceCache::lock.read_lock();

//get ptr
Resource **rptr = ResourceCache::resources.getptr(local_path);
Expand All @@ -340,9 +338,7 @@ Error ResourceLoader::load_threaded_request(const String &p_path, const String &
load_task.progress = 1.0;
}
}
if (ResourceCache::lock) {
ResourceCache::lock->read_unlock();
}
ResourceCache::lock.read_unlock();
}

if (p_source_resource != String()) {
Expand Down Expand Up @@ -535,9 +531,7 @@ RES ResourceLoader::load(const String &p_path, const String &p_type_hint, bool p
}

//Is it cached?
if (ResourceCache::lock) {
ResourceCache::lock->read_lock();
}
ResourceCache::lock.read_lock();

Resource **rptr = ResourceCache::resources.getptr(local_path);

Expand All @@ -546,9 +540,7 @@ RES ResourceLoader::load(const String &p_path, const String &p_type_hint, bool p

//it is possible this resource was just freed in a thread. If so, this referencing will not work and resource is considered not cached
if (res.is_valid()) {
if (ResourceCache::lock) {
ResourceCache::lock->read_unlock();
}
ResourceCache::lock.read_unlock();
thread_load_mutex->unlock();

if (r_error) {
Expand All @@ -559,9 +551,7 @@ RES ResourceLoader::load(const String &p_path, const String &p_type_hint, bool p
}
}

if (ResourceCache::lock) {
ResourceCache::lock->read_unlock();
}
ResourceCache::lock.read_unlock();

//load using task (but this thread)
ThreadLoadTask load_task;
Expand Down Expand Up @@ -955,9 +945,7 @@ String ResourceLoader::path_remap(const String &p_path) {
}

void ResourceLoader::reload_translation_remaps() {
if (ResourceCache::lock) {
ResourceCache::lock->read_lock();
}
ResourceCache::lock.read_lock();

List<Resource *> to_reload;
SelfList<Resource> *E = remapped_list.first();
Expand All @@ -967,9 +955,7 @@ void ResourceLoader::reload_translation_remaps() {
E = E->next();
}

if (ResourceCache::lock) {
ResourceCache::lock->read_unlock();
}
ResourceCache::lock.read_unlock();

//now just make sure to not delete any of these resources while changing locale..
while (to_reload.front()) {
Expand Down
12 changes: 3 additions & 9 deletions core/object/class_db.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -983,9 +983,9 @@ void ClassDB::add_property_subgroup(StringName p_class, const String &p_name, co

// NOTE: For implementation simplicity reasons, this method doesn't allow setters to have optional arguments at the end.
void ClassDB::add_property(StringName p_class, const PropertyInfo &p_pinfo, const StringName &p_setter, const StringName &p_getter, int p_index) {
lock->read_lock();
lock.read_lock();
ClassInfo *type = classes.getptr(p_class);
lock->read_unlock();
lock.read_unlock();

ERR_FAIL_COND(!type);

Expand Down Expand Up @@ -1541,11 +1541,7 @@ Variant ClassDB::class_get_default_property_value(const StringName &p_class, con
return var;
}

RWLock *ClassDB::lock = nullptr;

void ClassDB::init() {
lock = RWLock::create();
}
RWLock ClassDB::lock;

void ClassDB::cleanup_defaults() {
default_values.clear();
Expand All @@ -1568,8 +1564,6 @@ void ClassDB::cleanup() {
classes.clear();
resource_base_extensions.clear();
compat_classes.clear();

memdelete(lock);
}

//
3 changes: 1 addition & 2 deletions core/object/class_db.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ class ClassDB {
return memnew(T);
}

static RWLock *lock;
static RWLock lock;
static HashMap<StringName, ClassInfo> classes;
static HashMap<StringName, StringName> resource_base_extensions;
static HashMap<StringName, StringName> compat_classes;
Expand Down Expand Up @@ -387,7 +387,6 @@ class ClassDB {
static void get_extensions_for_type(const StringName &p_class, List<String> *p_extensions);

static void add_compatibility_class(const StringName &p_class, const StringName &p_fallback);
static void init();

static void set_current_api(APIType p_api);
static APIType get_current_api();
Expand Down
Loading

0 comments on commit d39f638

Please sign in to comment.