From 73d928006dda16d3e9c930c1ad6d37439fb2b919 Mon Sep 17 00:00:00 2001 From: DAShoe1 <117487773+DAShoe1@users.noreply.github.com> Date: Mon, 24 Feb 2025 21:49:32 -0500 Subject: [PATCH 1/3] PR: godotengine/godot#98469 backportored to 4.3 --- core/object/object.cpp | 216 +++++++++++++++++++++---------- core/object/object.h | 62 +++++---- core/os/rw_lock.cpp | 120 +++++++++++++++++ core/os/rw_lock.h | 36 +++--- core/variant/variant.cpp | 33 +++-- core/variant/variant.h | 17 ++- core/variant/variant_internal.h | 14 +- modules/gdscript/gdscript_vm.cpp | 14 +- 8 files changed, 359 insertions(+), 153 deletions(-) create mode 100644 core/os/rw_lock.cpp diff --git a/core/object/object.cpp b/core/object/object.cpp index 29e8cb41770..fb486730644 100644 --- a/core/object/object.cpp +++ b/core/object/object.cpp @@ -614,7 +614,11 @@ Variant Object::_call_bind(const Variant **p_args, int p_argcount, Callable::Cal return Variant(); } - StringName method = *p_args[0]; + if (p_args[0]->get_type() == Variant::STRING_NAME) { + const StringName &method = *VariantInternal::get_string_name(p_args[0]); + return callp(method, &p_args[1], p_argcount - 1, r_error); + } + const StringName method = *p_args[0]; return callp(method, &p_args[1], p_argcount - 1, r_error); } @@ -635,9 +639,16 @@ Variant Object::_call_deferred_bind(const Variant **p_args, int p_argcount, Call r_error.error = Callable::CallError::CALL_OK; - StringName method = *p_args[0]; + const StringName *method_ptr; + StringName method; + if (p_args[0]->get_type() == Variant::STRING_NAME) { + method_ptr = VariantInternal::get_string_name(p_args[0]); + } else { + method = StringName(*p_args[0]); + method_ptr = &method; + } - MessageQueue::get_singleton()->push_callp(get_instance_id(), method, &p_args[1], p_argcount - 1, true); + MessageQueue::get_singleton()->push_callp(get_instance_id(), *method_ptr, &p_args[1], p_argcount - 1, true); return Variant(); } @@ -2138,15 +2149,17 @@ void postinitialize_handler(Object *p_object) { } void ObjectDB::debug_objects(DebugFunc p_func) { - spin_lock.lock(); + mutex.lock(); - for (uint32_t i = 0, count = slot_count; i < slot_max && count != 0; i++) { - if (object_slots[i].validator) { - p_func(object_slots[i].object); - count--; + for (uint32_t i = 1; i < block_count; i++) { + for (uint32_t j = 0; j < blocks_max_sizes[i]; j++) { + if (blocks[i][j].validator) { + Object *obj = blocks[i][j].object; + p_func(obj); + } } } - spin_lock.unlock(); + mutex.unlock(); } #ifdef TOOLS_ENABLED @@ -2195,48 +2208,95 @@ void Object::get_argument_options(const StringName &p_function, int p_idx, List< } #endif -SpinLock ObjectDB::spin_lock; +BinaryMutex ObjectDB::mutex; uint32_t ObjectDB::slot_count = 0; +uint32_t ObjectDB::block_count = 0; uint32_t ObjectDB::slot_max = 0; -ObjectDB::ObjectSlot *ObjectDB::object_slots = nullptr; +uint32_t ObjectDB::block_max = 0; uint64_t ObjectDB::validator_counter = 0; +const uint32_t ObjectDB::blocks_max_sizes[OBJECTDB_MAX_BLOCKS] = { + 0, + 128, + 256, + 512, + 1024, + 1536, + 2304, + 3456, + 5184, + 7776, + 11664, + 17496, + 26244, + 39366, + 59049, + 88573, + 132859, + 199288, + 298932, + 448398, + 672597, + 1008895, + 1513342, + 2270013, + 3405019, + 5107528, + 7661292, + 11491938, + 17237907, + 25856860, + 38785290, + 67108864, +}; + +ObjectDB::ObjectSlot *ObjectDB::blocks[OBJECTDB_MAX_BLOCKS] = { nullptr }; + int ObjectDB::get_object_count() { return slot_count; } ObjectID ObjectDB::add_instance(Object *p_object) { - spin_lock.lock(); - if (unlikely(slot_count == slot_max)) { + mutex.lock(); + if (slot_count == blocks_max_sizes[block_count] && blocks[block_count + 1] != nullptr) { + slot_count = 0; + block_count++; + } + if (unlikely(slot_count == blocks_max_sizes[block_max])) { + block_max++; + CRASH_COND(block_max == OBJECTDB_MAX_BLOCKS); CRASH_COND(slot_count == (1 << OBJECTDB_SLOT_MAX_COUNT_BITS)); - - uint32_t new_slot_max = slot_max > 0 ? slot_max * 2 : 1; - object_slots = (ObjectSlot *)memrealloc(object_slots, sizeof(ObjectSlot) * new_slot_max); - for (uint32_t i = slot_max; i < new_slot_max; i++) { - object_slots[i].object = nullptr; - object_slots[i].is_ref_counted = false; - object_slots[i].next_free = i; - object_slots[i].validator = 0; + blocks[block_max] = (ObjectSlot *)memalloc(sizeof(ObjectSlot) * blocks_max_sizes[block_max]); + uint32_t new_slot_max = blocks_max_sizes[block_max]; + for (uint32_t i = 0; i < new_slot_max; i++) { + blocks[block_max][i].object = nullptr; + blocks[block_max][i].is_ref_counted = false; + blocks[block_max][i].next_free.block_number = block_max; + blocks[block_max][i].next_free.block_position = i; + blocks[block_max][i].validator = 0; } slot_max = new_slot_max; + block_count = block_max; + slot_count = 0; } - uint32_t slot = object_slots[slot_count].next_free; - if (object_slots[slot].object != nullptr) { - spin_lock.unlock(); - ERR_FAIL_COND_V(object_slots[slot].object != nullptr, ObjectID()); + NextFree slot = blocks[block_count][slot_count].next_free; + Object *o = blocks[slot.block_number][slot.block_position].object; + if (o != nullptr) { + mutex.unlock(); + ERR_FAIL_COND_V(o != nullptr, ObjectID()); } - object_slots[slot].object = p_object; - object_slots[slot].is_ref_counted = p_object->is_ref_counted(); + blocks[slot.block_number][slot.block_position].object = p_object; + blocks[slot.block_number][slot.block_position].is_ref_counted = p_object->is_ref_counted(); validator_counter = (validator_counter + 1) & OBJECTDB_VALIDATOR_MASK; if (unlikely(validator_counter == 0)) { validator_counter = 1; } - object_slots[slot].validator = validator_counter; + blocks[slot.block_number][slot.block_position].validator = validator_counter; uint64_t id = validator_counter; - id <<= OBJECTDB_SLOT_MAX_COUNT_BITS; - id |= uint64_t(slot); + id <<= OBJECTDB_SLOT_MAX_POSITION_BITS; + id |= uint64_t(slot.position); if (p_object->is_ref_counted()) { id |= OBJECTDB_REFERENCE_BIT; @@ -2244,42 +2304,47 @@ ObjectID ObjectDB::add_instance(Object *p_object) { slot_count++; - spin_lock.unlock(); + mutex.unlock(); return ObjectID(id); } void ObjectDB::remove_instance(Object *p_object) { uint64_t t = p_object->get_instance_id(); - uint32_t slot = t & OBJECTDB_SLOT_MAX_COUNT_MASK; //slot is always valid on valid object + NextFree slot; + slot.position = t & OBJECTDB_SLOT_MAX_POSITION_MASK; - spin_lock.lock(); + mutex.lock(); #ifdef DEBUG_ENABLED - if (object_slots[slot].object != p_object) { - spin_lock.unlock(); - ERR_FAIL_COND(object_slots[slot].object != p_object); + if (blocks[slot.block_number][slot.block_position].object != p_object) { + mutex.unlock(); + ERR_FAIL_COND(blocks[slot.block_number][slot.block_position].object != p_object); } { - uint64_t validator = (t >> OBJECTDB_SLOT_MAX_COUNT_BITS) & OBJECTDB_VALIDATOR_MASK; - if (object_slots[slot].validator != validator) { - spin_lock.unlock(); - ERR_FAIL_COND(object_slots[slot].validator != validator); + uint64_t validator = (t >> OBJECTDB_SLOT_MAX_POSITION_BITS) & OBJECTDB_VALIDATOR_MASK; + if (blocks[slot.block_number][slot.block_position].validator != validator) { + mutex.unlock(); + ERR_FAIL_COND(blocks[slot.block_number][slot.block_position].validator != validator); } } #endif //decrease slot count + if (slot_count == 0) { + block_count--; + slot_count = blocks_max_sizes[block_count]; + } slot_count--; //set the free slot properly - object_slots[slot_count].next_free = slot; + blocks[block_count][slot_count].next_free = slot; //invalidate, so checks against it fail - object_slots[slot].validator = 0; - object_slots[slot].is_ref_counted = false; - object_slots[slot].object = nullptr; + blocks[slot.block_number][slot.block_position].validator = 0; + blocks[slot.block_number][slot.block_position].is_ref_counted = false; + blocks[slot.block_number][slot.block_position].object = nullptr; - spin_lock.unlock(); + mutex.unlock(); } void ObjectDB::setup() { @@ -2287,7 +2352,7 @@ void ObjectDB::setup() { } void ObjectDB::cleanup() { - spin_lock.lock(); + mutex.lock(); if (slot_count > 0) { WARN_PRINT("ObjectDB instances leaked at exit (run with --verbose for details)."); @@ -2299,32 +2364,51 @@ void ObjectDB::cleanup() { MethodBind *resource_get_path = ClassDB::get_method("Resource", "get_path"); Callable::CallError call_error; - for (uint32_t i = 0, count = slot_count; i < slot_max && count != 0; i++) { - if (object_slots[i].validator) { - Object *obj = object_slots[i].object; - - String extra_info; - if (obj->is_class("Node")) { - extra_info = " - Node name: " + String(node_get_name->call(obj, nullptr, 0, call_error)); + for (uint32_t i = 1; i < block_count; i++) { + for (uint32_t j = 0; j < blocks_max_sizes[i]; j++) { + if (blocks[i][j].validator) { + Object *obj = blocks[i][j].object; + + String extra_info; + if (obj->is_class("Node")) { + extra_info = " - Node path: " + String(node_get_name->call(obj, nullptr, 0, call_error)); + } + if (obj->is_class("Resource")) { + extra_info = " - Resource path: " + String(resource_get_path->call(obj, nullptr, 0, call_error)); + } + + uint64_t id = uint64_t(i) | (uint64_t(blocks[i][j].validator) << OBJECTDB_SLOT_MAX_COUNT_BITS) | (blocks[i][j].is_ref_counted ? OBJECTDB_REFERENCE_BIT : 0); + DEV_ASSERT(id == (uint64_t)obj->get_instance_id()); // We could just use the id from the object, but this check may help catching memory corruption catastrophes. + print_line("Leaked instance: " + String(obj->get_class()) + ":" + uitos(id) + extra_info); } - if (obj->is_class("Resource")) { - extra_info = " - Resource path: " + String(resource_get_path->call(obj, nullptr, 0, call_error)); - } - - uint64_t id = uint64_t(i) | (uint64_t(object_slots[i].validator) << OBJECTDB_SLOT_MAX_COUNT_BITS) | (object_slots[i].is_ref_counted ? OBJECTDB_REFERENCE_BIT : 0); - DEV_ASSERT(id == (uint64_t)obj->get_instance_id()); // We could just use the id from the object, but this check may help catching memory corruption catastrophes. - print_line("Leaked instance: " + String(obj->get_class()) + ":" + uitos(id) + extra_info); - - count--; } } print_line("Hint: Leaked instances typically happen when nodes are removed from the scene tree (with `remove_child()`) but not freed (with `free()` or `queue_free()`)."); } } - if (object_slots) { - memfree(object_slots); + for (uint32_t i = 1; i < block_count; i++) { + memfree(blocks[i]); } - - spin_lock.unlock(); + mutex.unlock(); } + +Object *ObjectDB::get_instance(ObjectID p_instance_id) { + uint64_t id = p_instance_id; + NextFree slot; + slot.position = id & OBJECTDB_SLOT_MAX_POSITION_MASK; + + ERR_FAIL_COND_V(slot.block_number > block_max, nullptr); // This should never happen unless RID is corrupted. + ERR_FAIL_COND_V(slot.block_position >= slot_max, nullptr); // Same here. + + if (unlikely(slot.block_number == 0)) { // Null Object. + return nullptr; + } + uint64_t validator = (id >> OBJECTDB_SLOT_MAX_POSITION_BITS) & OBJECTDB_VALIDATOR_MASK; + if (unlikely(blocks[slot.block_number][slot.block_position].validator != validator)) { + return nullptr; + } + Object *object = blocks[slot.block_number][slot.block_position].object; + + return object; +} \ No newline at end of file diff --git a/core/object/object.h b/core/object/object.h index d401b86ea9d..fcaff17c9a5 100644 --- a/core/object/object.h +++ b/core/object/object.h @@ -36,11 +36,13 @@ #include "core/extension/gdextension_interface.h" #include "core/object/message_queue.h" #include "core/object/object_id.h" +#include "core/os/mutex.h" #include "core/os/rw_lock.h" #include "core/os/spin_lock.h" #include "core/templates/hash_map.h" #include "core/templates/hash_set.h" #include "core/templates/list.h" +#include "core/templates/paged_array.h" #include "core/templates/rb_map.h" #include "core/templates/safe_refcount.h" #include "core/variant/callable_bind.h" @@ -1014,23 +1016,39 @@ void postinitialize_handler(Object *p_object); class ObjectDB { // This needs to add up to 63, 1 bit is for reference. -#define OBJECTDB_VALIDATOR_BITS 39 +#define OBJECTDB_VALIDATOR_BITS 32 #define OBJECTDB_VALIDATOR_MASK ((uint64_t(1) << OBJECTDB_VALIDATOR_BITS) - 1) -#define OBJECTDB_SLOT_MAX_COUNT_BITS 24 -#define OBJECTDB_SLOT_MAX_COUNT_MASK ((uint64_t(1) << OBJECTDB_SLOT_MAX_COUNT_BITS) - 1) -#define OBJECTDB_REFERENCE_BIT (uint64_t(1) << (OBJECTDB_SLOT_MAX_COUNT_BITS + OBJECTDB_VALIDATOR_BITS)) - - struct ObjectSlot { // 128 bits per slot. - uint64_t validator : OBJECTDB_VALIDATOR_BITS; - uint64_t next_free : OBJECTDB_SLOT_MAX_COUNT_BITS; - uint64_t is_ref_counted : 1; +#define OBJECTDB_SLOT_MAX_BLOCKS_COUNT_BITS 5 +#define OBJECTDB_SLOT_MAX_COUNT_BITS 26 +#define OBJECTDB_SLOT_MAX_POSITION_BITS (OBJECTDB_SLOT_MAX_BLOCKS_COUNT_BITS + OBJECTDB_SLOT_MAX_COUNT_BITS) +#define OBJECTDB_SLOT_MAX_BLOCKS_COUNT_MASK ((uint64_t(1) << OBJECTDB_SLOT_MAX_BLOCKS_COUNT_BITS) - 1) +#define OBJECTDB_SLOT_MAX_POSITION_MASK ((uint64_t(1) << OBJECTDB_SLOT_MAX_POSITION_BITS) - 1) +#define OBJECTDB_REFERENCE_BIT (uint64_t(1) << (OBJECTDB_SLOT_MAX_POSITION_BITS + OBJECTDB_VALIDATOR_BITS)) +#define OBJECTDB_MAX_BLOCKS 32 + + union NextFree { + uint32_t position : OBJECTDB_SLOT_MAX_POSITION_BITS; + struct { + uint32_t block_number : OBJECTDB_SLOT_MAX_BLOCKS_COUNT_BITS; + uint32_t block_position : OBJECTDB_SLOT_MAX_COUNT_BITS; + }; + }; + + struct ObjectSlot { + uint64_t validator; + NextFree next_free; + bool is_ref_counted; Object *object = nullptr; }; - static SpinLock spin_lock; + static ObjectSlot *blocks[OBJECTDB_MAX_BLOCKS]; + static const uint32_t blocks_max_sizes[OBJECTDB_MAX_BLOCKS]; static uint32_t slot_count; + static uint32_t block_count; static uint32_t slot_max; - static ObjectSlot *object_slots; + static uint32_t block_max; + + static BinaryMutex mutex; static uint64_t validator_counter; friend class Object; @@ -1046,27 +1064,7 @@ class ObjectDB { public: typedef void (*DebugFunc)(Object *p_obj); - _ALWAYS_INLINE_ static Object *get_instance(ObjectID p_instance_id) { - uint64_t id = p_instance_id; - uint32_t slot = id & OBJECTDB_SLOT_MAX_COUNT_MASK; - - ERR_FAIL_COND_V(slot >= slot_max, nullptr); // This should never happen unless RID is corrupted. - - spin_lock.lock(); - - uint64_t validator = (id >> OBJECTDB_SLOT_MAX_COUNT_BITS) & OBJECTDB_VALIDATOR_MASK; - - if (unlikely(object_slots[slot].validator != validator)) { - spin_lock.unlock(); - return nullptr; - } - - Object *object = object_slots[slot].object; - - spin_lock.unlock(); - - return object; - } + static Object *get_instance(ObjectID p_instance_id); static void debug_objects(DebugFunc p_func); static int get_object_count(); }; diff --git a/core/os/rw_lock.cpp b/core/os/rw_lock.cpp new file mode 100644 index 00000000000..a83bb184c88 --- /dev/null +++ b/core/os/rw_lock.cpp @@ -0,0 +1,120 @@ +/**************************************************************************/ +/* rw_lock.cpp */ +/**************************************************************************/ +/* This file is part of: */ +/* REDOT ENGINE */ +/* https://redotengine.org */ +/**************************************************************************/ +/* Copyright (c) 2024-present Redot Engine contributors */ +/* (see REDOT_AUTHORS.md) */ +/* Copyright (c) 2014-present Godot Engine contributors (see AUTHORS.md). */ +/* Copyright (c) 2007-2014 Juan Linietsky, Ariel Manzur. */ +/* */ +/* Permission is hereby granted, free of charge, to any person obtaining */ +/* a copy of this software and associated documentation files (the */ +/* "Software"), to deal in the Software without restriction, including */ +/* without limitation the rights to use, copy, modify, merge, publish, */ +/* distribute, sublicense, and/or sell copies of the Software, and to */ +/* permit persons to whom the Software is furnished to do so, subject to */ +/* the following conditions: */ +/* */ +/* The above copyright notice and this permission notice shall be */ +/* included in all copies or substantial portions of the Software. */ +/* */ +/* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, */ +/* EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF */ +/* MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. */ +/* IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY */ +/* CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, */ +/* TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE */ +/* SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */ +/**************************************************************************/ + +#include "rw_lock.h" + +#include "core/os/memory.h" +#include "core/os/os.h" +#include "core/os/thread.h" +#include "core/typedefs.h" + +int RWLock::threads_number = -1; + +struct RWLock::ThreadMutex { + uint8_t _offset[64]; + BinaryMutex mtx; +}; + +int RWLock::get_thread_pos() { + return Thread::get_caller_id() % threads_number; +} + +void RWLock::init() const { + if (unlikely(threads_number == -1)) { + if (OS::get_singleton() != nullptr) { + threads_number = OS::get_singleton()->get_processor_count(); + } else { + threads_number = THREADING_NAMESPACE::thread::hardware_concurrency(); + } + + if (threads_number < 1) { + threads_number = 1; + } + } + threads_data = (ThreadMutex *)memalloc(sizeof(ThreadMutex) * threads_number); + for (int i = 0; i < threads_number; i++) { + memnew_placement(&threads_data[i], ThreadMutex()); + } +} + +void RWLock::read_lock() const { + if (unlikely(threads_data == nullptr)) { + return; + } + threads_data[get_thread_pos()].mtx.lock(); +} + +void RWLock::read_unlock() const { + if (unlikely(threads_data == nullptr)) { + return; + } + + DEV_ASSERT(threads_data != nullptr); + threads_data[get_thread_pos()].mtx.unlock(); +} + +void RWLock::write_lock() { + if (unlikely(threads_data == nullptr)) { + return; + } + + for (int i = 0; i < threads_number; i++) { + threads_data[i].mtx.lock(); + } +} + +void RWLock::write_unlock() { + if (unlikely(threads_data == nullptr)) { + return; + } + + DEV_ASSERT(threads_data != nullptr); + for (int i = 0; i < threads_number; i++) { + threads_data[i].mtx.unlock(); + } +} + +RWLock::RWLock() { + if (threads_data == nullptr) { + init(); + } +} + +RWLock::~RWLock() { + if (threads_data != nullptr) { + for (int i = 0; i < threads_number; i++) { + threads_data[i].~ThreadMutex(); + } + memfree(threads_data); + threads_data = nullptr; + } +} \ No newline at end of file diff --git a/core/os/rw_lock.h b/core/os/rw_lock.h index b1d061514be..47ddb08768f 100644 --- a/core/os/rw_lock.h +++ b/core/os/rw_lock.h @@ -45,38 +45,32 @@ #endif class RWLock { - mutable THREADING_NAMESPACE::shared_timed_mutex mutex; + struct ThreadMutex; + + static int threads_number; + + mutable ThreadMutex *threads_data = nullptr; + + static int get_thread_pos(); + + void init() const; public: // Lock the RWLock, block if locked by someone else. - _ALWAYS_INLINE_ void read_lock() const { - mutex.lock_shared(); - } + void read_lock() const; // Unlock the RWLock, let other threads continue. - _ALWAYS_INLINE_ void read_unlock() const { - mutex.unlock_shared(); - } - - // Attempt to lock the RWLock for reading. True on success, false means it can't lock. - _ALWAYS_INLINE_ bool read_try_lock() const { - return mutex.try_lock_shared(); - } + void read_unlock() const; // Lock the RWLock, block if locked by someone else. - _ALWAYS_INLINE_ void write_lock() { - mutex.lock(); - } + void write_lock(); // Unlock the RWLock, let other threads continue. - _ALWAYS_INLINE_ void write_unlock() { - mutex.unlock(); - } + void write_unlock(); // Attempt to lock the RWLock for writing. True on success, false means it can't lock. - _ALWAYS_INLINE_ bool write_try_lock() { - return mutex.try_lock(); - } + RWLock(); + ~RWLock(); }; class RWLockRead { diff --git a/core/variant/variant.cpp b/core/variant/variant.cpp index 24159b24842..629d0e1c453 100644 --- a/core/variant/variant.cpp +++ b/core/variant/variant.cpp @@ -1074,7 +1074,7 @@ bool Variant::is_null() const { } } -void Variant::ObjData::ref(const ObjData &p_from) { +void Variant::ObjData::ref(const ObjData &p_from, bool p_is_weak_ref_old, bool p_is_weak_ref) { // Mirrors Ref::ref in refcounted.h if (p_from.id == id) { return; @@ -1083,7 +1083,7 @@ void Variant::ObjData::ref(const ObjData &p_from) { ObjData cleanup_ref = *this; *this = p_from; - if (id.is_ref_counted()) { + if (!p_is_weak_ref && id.is_ref_counted()) { RefCounted *reference = static_cast(obj); // Assuming reference is not null because id.is_ref_counted() was true. if (!reference->reference()) { @@ -1091,10 +1091,10 @@ void Variant::ObjData::ref(const ObjData &p_from) { } } - cleanup_ref.unref(); + cleanup_ref.unref(p_is_weak_ref_old); } -void Variant::ObjData::ref_pointer(Object *p_object) { +void Variant::ObjData::ref_pointer(Object *p_object, bool p_is_weak_ref_old, bool p_is_weak_ref) { // Mirrors Ref::ref_pointer in refcounted.h if (p_object == obj) { return; @@ -1104,7 +1104,7 @@ void Variant::ObjData::ref_pointer(Object *p_object) { if (p_object) { *this = ObjData{ p_object->get_instance_id(), p_object }; - if (p_object->is_ref_counted()) { + if (!p_is_weak_ref && p_object->is_ref_counted()) { RefCounted *reference = static_cast(p_object); if (!reference->init_ref()) { *this = ObjData(); @@ -1114,12 +1114,12 @@ void Variant::ObjData::ref_pointer(Object *p_object) { *this = ObjData(); } - cleanup_ref.unref(); + cleanup_ref.unref(p_is_weak_ref_old); } -void Variant::ObjData::unref() { +void Variant::ObjData::unref(bool p_is_weak_ref) { // Mirrors Ref::unref in refcounted.h - if (id.is_ref_counted()) { + if (!p_is_weak_ref && id.is_ref_counted()) { RefCounted *reference = static_cast(obj); // Assuming reference is not null because id.is_ref_counted() was true. if (reference->unreference()) { @@ -1131,7 +1131,7 @@ void Variant::ObjData::unref() { void Variant::reference(const Variant &p_variant) { if (type == OBJECT && p_variant.type == OBJECT) { - _get_obj().ref(p_variant._get_obj()); + _get_obj().ref(p_variant._get_obj(), is_weak_ref, p_variant.is_weak_ref); return; } @@ -1219,7 +1219,7 @@ void Variant::reference(const Variant &p_variant) { } break; case OBJECT: { memnew_placement(_data._mem, ObjData); - _get_obj().ref(p_variant._get_obj()); + _get_obj().ref(p_variant._get_obj(), is_weak_ref, p_variant.is_weak_ref); } break; case CALLABLE: { memnew_placement(_data._mem, Callable(*reinterpret_cast(p_variant._data._mem))); @@ -1418,7 +1418,7 @@ void Variant::_clear_internal() { reinterpret_cast(_data._mem)->~NodePath(); } break; case OBJECT: { - _get_obj().unref(); + _get_obj().unref(is_weak_ref); } break; case RID: { // Not much need probably. @@ -2625,7 +2625,14 @@ Variant::Variant(const ::RID &p_rid) : Variant::Variant(const Object *p_object) : type(OBJECT) { _get_obj() = ObjData(); - _get_obj().ref_pointer(const_cast(p_object)); + _get_obj().ref_pointer(const_cast(p_object), true, false); +} + +Variant::Variant(const RefCounted *p_object, bool p_is_weak_ref) : + type(OBJECT) { + _get_obj() = ObjData(); + _get_obj().ref_pointer((Object *)p_object, true, p_is_weak_ref); + is_weak_ref = p_is_weak_ref; } Variant::Variant(const Callable &p_callable) : @@ -2847,7 +2854,7 @@ void Variant::operator=(const Variant &p_variant) { *reinterpret_cast<::RID *>(_data._mem) = *reinterpret_cast(p_variant._data._mem); } break; case OBJECT: { - _get_obj().ref(p_variant._get_obj()); + _get_obj().ref(p_variant._get_obj(), is_weak_ref, p_variant.is_weak_ref); } break; case CALLABLE: { *reinterpret_cast(_data._mem) = *reinterpret_cast(p_variant._data._mem); diff --git a/core/variant/variant.h b/core/variant/variant.h index 79d9c51cc2f..f1983864e82 100644 --- a/core/variant/variant.h +++ b/core/variant/variant.h @@ -177,22 +177,23 @@ class Variant { // and PackedArray/Array/Dictionary (platform-dependent). Type type = NIL; + bool is_weak_ref = false; struct ObjData { ObjectID id; Object *obj = nullptr; - void ref(const ObjData &p_from); - void ref_pointer(Object *p_object); - void ref_pointer(RefCounted *p_object); - void unref(); + void ref(const ObjData &p_from, bool p_is_weak_ref_old, bool p_is_weak_ref); + void ref_pointer(Object *p_object, bool p_is_weak_ref_old, bool is_weak_ref); + void ref_pointer(RefCounted *p_object, bool p_is_weak_ref_old, bool p_is_weak_ref); + void unref(bool p_is_weak_ref); template - _ALWAYS_INLINE_ void ref(const Ref &p_from) { + _ALWAYS_INLINE_ void ref(const Ref &p_from, bool p_is_weak_ref_old, bool p_is_weak_ref) { if (p_from.is_valid()) { - ref(ObjData{ p_from->get_instance_id(), p_from.ptr() }); + ref(ObjData{ p_from->get_instance_id(), p_from.ptr() }, p_is_weak_ref_old, p_is_weak_ref); } else { - unref(); + unref(p_is_weak_ref_old); } } }; @@ -328,6 +329,7 @@ class Variant { _clear_internal(); } type = NIL; + is_weak_ref = false; } static void _register_variant_operators(); @@ -486,6 +488,7 @@ class Variant { Variant(const NodePath &p_node_path); Variant(const ::RID &p_rid); Variant(const Object *p_object); + Variant(const RefCounted *p_object, bool p_is_weak_ref = false); Variant(const Callable &p_callable); Variant(const Signal &p_signal); Variant(const Dictionary &p_dictionary); diff --git a/core/variant/variant_internal.h b/core/variant/variant_internal.h index bee7461fa7f..5729c1bf752 100644 --- a/core/variant/variant_internal.h +++ b/core/variant/variant_internal.h @@ -330,20 +330,20 @@ class VariantInternal { } _FORCE_INLINE_ static void object_assign(Variant *v, const Variant *vo) { - v->_get_obj().ref(vo->_get_obj()); + v->_get_obj().ref(vo->_get_obj(), v->is_weak_ref, vo->is_weak_ref); } - _FORCE_INLINE_ static void object_assign(Variant *v, Object *o) { - v->_get_obj().ref_pointer(o); + _FORCE_INLINE_ static void object_assign(Variant *v, Object *o, bool p_is_weak_ref = false) { + v->_get_obj().ref_pointer(o, v->is_weak_ref, p_is_weak_ref); } - _FORCE_INLINE_ static void object_assign(Variant *v, const Object *o) { - v->_get_obj().ref_pointer(const_cast(o)); + _FORCE_INLINE_ static void object_assign(Variant *v, const Object *o, bool p_is_weak_ref = false) { + v->_get_obj().ref_pointer(const_cast(o), v->is_weak_ref, p_is_weak_ref); } template - _FORCE_INLINE_ static void object_assign(Variant *v, const Ref &r) { - v->_get_obj().ref(r); + _FORCE_INLINE_ static void object_assign(Variant *v, const Ref &r, bool p_is_weak_ref = false) { + v->_get_obj().ref(r, v->is_weak_ref, p_is_weak_ref); } _FORCE_INLINE_ static void object_reset_data(Variant *v) { diff --git a/modules/gdscript/gdscript_vm.cpp b/modules/gdscript/gdscript_vm.cpp index 36b1342f123..8d9dcf330d3 100644 --- a/modules/gdscript/gdscript_vm.cpp +++ b/modules/gdscript/gdscript_vm.cpp @@ -594,7 +594,7 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a memnew_placement(&stack[ADDR_STACK_SELF], Variant); script = _script; } - memnew_placement(&stack[ADDR_STACK_CLASS], Variant(script)); + memnew_placement(&stack[ADDR_STACK_CLASS], Variant(script, true)); memnew_placement(&stack[ADDR_STACK_NIL], Variant); String err_text; @@ -818,7 +818,7 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a Variant::Type builtin_type = (Variant::Type)_code_ptr[ip + 4]; int native_type_idx = _code_ptr[ip + 5]; GD_ERR_BREAK(native_type_idx < 0 || native_type_idx >= _global_names_count); - const StringName native_type = _global_names_ptr[native_type_idx]; + const StringName &native_type = _global_names_ptr[native_type_idx]; bool result = false; if (value->get_type() == Variant::ARRAY) { @@ -839,7 +839,7 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a int native_type_idx = _code_ptr[ip + 3]; GD_ERR_BREAK(native_type_idx < 0 || native_type_idx >= _global_names_count); - const StringName native_type = _global_names_ptr[native_type_idx]; + const StringName &native_type = _global_names_ptr[native_type_idx]; bool was_freed = false; Object *object = value->get_validated_object_with_check(was_freed); @@ -1362,7 +1362,7 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a Variant::Type builtin_type = (Variant::Type)_code_ptr[ip + 4]; int native_type_idx = _code_ptr[ip + 5]; GD_ERR_BREAK(native_type_idx < 0 || native_type_idx >= _global_names_count); - const StringName native_type = _global_names_ptr[native_type_idx]; + const StringName &native_type = _global_names_ptr[native_type_idx]; if (src->get_type() != Variant::ARRAY) { #ifdef DEBUG_ENABLED @@ -1673,7 +1673,7 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a Variant::Type builtin_type = (Variant::Type)_code_ptr[ip + 2]; int native_type_idx = _code_ptr[ip + 3]; GD_ERR_BREAK(native_type_idx < 0 || native_type_idx >= _global_names_count); - const StringName native_type = _global_names_ptr[native_type_idx]; + const StringName &native_type = _global_names_ptr[native_type_idx]; Array array; array.resize(argc); @@ -2200,7 +2200,7 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a GD_ERR_BREAK(argc < 0); GD_ERR_BREAK(_code_ptr[ip + 2] < 0 || _code_ptr[ip + 2] >= _global_names_count); - StringName function = _global_names_ptr[_code_ptr[ip + 2]]; + const StringName &function = _global_names_ptr[_code_ptr[ip + 2]]; Variant **argptrs = instruction_args; @@ -2630,7 +2630,7 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a Variant::Type builtin_type = (Variant::Type)_code_ptr[ip + 3]; int native_type_idx = _code_ptr[ip + 4]; GD_ERR_BREAK(native_type_idx < 0 || native_type_idx >= _global_names_count); - const StringName native_type = _global_names_ptr[native_type_idx]; + const StringName &native_type = _global_names_ptr[native_type_idx]; if (r->get_type() != Variant::ARRAY) { #ifdef DEBUG_ENABLED From 9b65d614d6139acfe50c4f048b133aa0ce498c05 Mon Sep 17 00:00:00 2001 From: DAShoe1 <117487773+DAShoe1@users.noreply.github.com> Date: Thu, 28 Aug 2025 18:23:18 -0400 Subject: [PATCH 2/3] Remove rw_lock changes as they don't improve performance. --- core/os/rw_lock.cpp | 120 -------------------------------------------- core/os/rw_lock.h | 36 +++++++------ 2 files changed, 21 insertions(+), 135 deletions(-) delete mode 100644 core/os/rw_lock.cpp diff --git a/core/os/rw_lock.cpp b/core/os/rw_lock.cpp deleted file mode 100644 index a83bb184c88..00000000000 --- a/core/os/rw_lock.cpp +++ /dev/null @@ -1,120 +0,0 @@ -/**************************************************************************/ -/* rw_lock.cpp */ -/**************************************************************************/ -/* This file is part of: */ -/* REDOT ENGINE */ -/* https://redotengine.org */ -/**************************************************************************/ -/* Copyright (c) 2024-present Redot Engine contributors */ -/* (see REDOT_AUTHORS.md) */ -/* Copyright (c) 2014-present Godot Engine contributors (see AUTHORS.md). */ -/* Copyright (c) 2007-2014 Juan Linietsky, Ariel Manzur. */ -/* */ -/* Permission is hereby granted, free of charge, to any person obtaining */ -/* a copy of this software and associated documentation files (the */ -/* "Software"), to deal in the Software without restriction, including */ -/* without limitation the rights to use, copy, modify, merge, publish, */ -/* distribute, sublicense, and/or sell copies of the Software, and to */ -/* permit persons to whom the Software is furnished to do so, subject to */ -/* the following conditions: */ -/* */ -/* The above copyright notice and this permission notice shall be */ -/* included in all copies or substantial portions of the Software. */ -/* */ -/* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, */ -/* EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF */ -/* MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. */ -/* IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY */ -/* CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, */ -/* TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE */ -/* SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */ -/**************************************************************************/ - -#include "rw_lock.h" - -#include "core/os/memory.h" -#include "core/os/os.h" -#include "core/os/thread.h" -#include "core/typedefs.h" - -int RWLock::threads_number = -1; - -struct RWLock::ThreadMutex { - uint8_t _offset[64]; - BinaryMutex mtx; -}; - -int RWLock::get_thread_pos() { - return Thread::get_caller_id() % threads_number; -} - -void RWLock::init() const { - if (unlikely(threads_number == -1)) { - if (OS::get_singleton() != nullptr) { - threads_number = OS::get_singleton()->get_processor_count(); - } else { - threads_number = THREADING_NAMESPACE::thread::hardware_concurrency(); - } - - if (threads_number < 1) { - threads_number = 1; - } - } - threads_data = (ThreadMutex *)memalloc(sizeof(ThreadMutex) * threads_number); - for (int i = 0; i < threads_number; i++) { - memnew_placement(&threads_data[i], ThreadMutex()); - } -} - -void RWLock::read_lock() const { - if (unlikely(threads_data == nullptr)) { - return; - } - threads_data[get_thread_pos()].mtx.lock(); -} - -void RWLock::read_unlock() const { - if (unlikely(threads_data == nullptr)) { - return; - } - - DEV_ASSERT(threads_data != nullptr); - threads_data[get_thread_pos()].mtx.unlock(); -} - -void RWLock::write_lock() { - if (unlikely(threads_data == nullptr)) { - return; - } - - for (int i = 0; i < threads_number; i++) { - threads_data[i].mtx.lock(); - } -} - -void RWLock::write_unlock() { - if (unlikely(threads_data == nullptr)) { - return; - } - - DEV_ASSERT(threads_data != nullptr); - for (int i = 0; i < threads_number; i++) { - threads_data[i].mtx.unlock(); - } -} - -RWLock::RWLock() { - if (threads_data == nullptr) { - init(); - } -} - -RWLock::~RWLock() { - if (threads_data != nullptr) { - for (int i = 0; i < threads_number; i++) { - threads_data[i].~ThreadMutex(); - } - memfree(threads_data); - threads_data = nullptr; - } -} \ No newline at end of file diff --git a/core/os/rw_lock.h b/core/os/rw_lock.h index 47ddb08768f..b1d061514be 100644 --- a/core/os/rw_lock.h +++ b/core/os/rw_lock.h @@ -45,32 +45,38 @@ #endif class RWLock { - struct ThreadMutex; - - static int threads_number; - - mutable ThreadMutex *threads_data = nullptr; - - static int get_thread_pos(); - - void init() const; + mutable THREADING_NAMESPACE::shared_timed_mutex mutex; public: // Lock the RWLock, block if locked by someone else. - void read_lock() const; + _ALWAYS_INLINE_ void read_lock() const { + mutex.lock_shared(); + } // Unlock the RWLock, let other threads continue. - void read_unlock() const; + _ALWAYS_INLINE_ void read_unlock() const { + mutex.unlock_shared(); + } + + // Attempt to lock the RWLock for reading. True on success, false means it can't lock. + _ALWAYS_INLINE_ bool read_try_lock() const { + return mutex.try_lock_shared(); + } // Lock the RWLock, block if locked by someone else. - void write_lock(); + _ALWAYS_INLINE_ void write_lock() { + mutex.lock(); + } // Unlock the RWLock, let other threads continue. - void write_unlock(); + _ALWAYS_INLINE_ void write_unlock() { + mutex.unlock(); + } // Attempt to lock the RWLock for writing. True on success, false means it can't lock. - RWLock(); - ~RWLock(); + _ALWAYS_INLINE_ bool write_try_lock() { + return mutex.try_lock(); + } }; class RWLockRead { From aeb2ca4852e24a219575ddd5577a0a645749b4ad Mon Sep 17 00:00:00 2001 From: DAShoe1 <117487773+DAShoe1@users.noreply.github.com> Date: Thu, 28 Aug 2025 18:23:18 -0400 Subject: [PATCH 3/3] Remove rw_lock changes as they don't improve performance. --- core/object/object.cpp | 2 +- core/os/rw_lock.cpp | 120 ----------------------------------------- core/os/rw_lock.h | 36 +++++++------ 3 files changed, 22 insertions(+), 136 deletions(-) delete mode 100644 core/os/rw_lock.cpp diff --git a/core/object/object.cpp b/core/object/object.cpp index fb486730644..4390cbf893d 100644 --- a/core/object/object.cpp +++ b/core/object/object.cpp @@ -2411,4 +2411,4 @@ Object *ObjectDB::get_instance(ObjectID p_instance_id) { Object *object = blocks[slot.block_number][slot.block_position].object; return object; -} \ No newline at end of file +} diff --git a/core/os/rw_lock.cpp b/core/os/rw_lock.cpp deleted file mode 100644 index a83bb184c88..00000000000 --- a/core/os/rw_lock.cpp +++ /dev/null @@ -1,120 +0,0 @@ -/**************************************************************************/ -/* rw_lock.cpp */ -/**************************************************************************/ -/* This file is part of: */ -/* REDOT ENGINE */ -/* https://redotengine.org */ -/**************************************************************************/ -/* Copyright (c) 2024-present Redot Engine contributors */ -/* (see REDOT_AUTHORS.md) */ -/* Copyright (c) 2014-present Godot Engine contributors (see AUTHORS.md). */ -/* Copyright (c) 2007-2014 Juan Linietsky, Ariel Manzur. */ -/* */ -/* Permission is hereby granted, free of charge, to any person obtaining */ -/* a copy of this software and associated documentation files (the */ -/* "Software"), to deal in the Software without restriction, including */ -/* without limitation the rights to use, copy, modify, merge, publish, */ -/* distribute, sublicense, and/or sell copies of the Software, and to */ -/* permit persons to whom the Software is furnished to do so, subject to */ -/* the following conditions: */ -/* */ -/* The above copyright notice and this permission notice shall be */ -/* included in all copies or substantial portions of the Software. */ -/* */ -/* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, */ -/* EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF */ -/* MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. */ -/* IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY */ -/* CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, */ -/* TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE */ -/* SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */ -/**************************************************************************/ - -#include "rw_lock.h" - -#include "core/os/memory.h" -#include "core/os/os.h" -#include "core/os/thread.h" -#include "core/typedefs.h" - -int RWLock::threads_number = -1; - -struct RWLock::ThreadMutex { - uint8_t _offset[64]; - BinaryMutex mtx; -}; - -int RWLock::get_thread_pos() { - return Thread::get_caller_id() % threads_number; -} - -void RWLock::init() const { - if (unlikely(threads_number == -1)) { - if (OS::get_singleton() != nullptr) { - threads_number = OS::get_singleton()->get_processor_count(); - } else { - threads_number = THREADING_NAMESPACE::thread::hardware_concurrency(); - } - - if (threads_number < 1) { - threads_number = 1; - } - } - threads_data = (ThreadMutex *)memalloc(sizeof(ThreadMutex) * threads_number); - for (int i = 0; i < threads_number; i++) { - memnew_placement(&threads_data[i], ThreadMutex()); - } -} - -void RWLock::read_lock() const { - if (unlikely(threads_data == nullptr)) { - return; - } - threads_data[get_thread_pos()].mtx.lock(); -} - -void RWLock::read_unlock() const { - if (unlikely(threads_data == nullptr)) { - return; - } - - DEV_ASSERT(threads_data != nullptr); - threads_data[get_thread_pos()].mtx.unlock(); -} - -void RWLock::write_lock() { - if (unlikely(threads_data == nullptr)) { - return; - } - - for (int i = 0; i < threads_number; i++) { - threads_data[i].mtx.lock(); - } -} - -void RWLock::write_unlock() { - if (unlikely(threads_data == nullptr)) { - return; - } - - DEV_ASSERT(threads_data != nullptr); - for (int i = 0; i < threads_number; i++) { - threads_data[i].mtx.unlock(); - } -} - -RWLock::RWLock() { - if (threads_data == nullptr) { - init(); - } -} - -RWLock::~RWLock() { - if (threads_data != nullptr) { - for (int i = 0; i < threads_number; i++) { - threads_data[i].~ThreadMutex(); - } - memfree(threads_data); - threads_data = nullptr; - } -} \ No newline at end of file diff --git a/core/os/rw_lock.h b/core/os/rw_lock.h index 47ddb08768f..b1d061514be 100644 --- a/core/os/rw_lock.h +++ b/core/os/rw_lock.h @@ -45,32 +45,38 @@ #endif class RWLock { - struct ThreadMutex; - - static int threads_number; - - mutable ThreadMutex *threads_data = nullptr; - - static int get_thread_pos(); - - void init() const; + mutable THREADING_NAMESPACE::shared_timed_mutex mutex; public: // Lock the RWLock, block if locked by someone else. - void read_lock() const; + _ALWAYS_INLINE_ void read_lock() const { + mutex.lock_shared(); + } // Unlock the RWLock, let other threads continue. - void read_unlock() const; + _ALWAYS_INLINE_ void read_unlock() const { + mutex.unlock_shared(); + } + + // Attempt to lock the RWLock for reading. True on success, false means it can't lock. + _ALWAYS_INLINE_ bool read_try_lock() const { + return mutex.try_lock_shared(); + } // Lock the RWLock, block if locked by someone else. - void write_lock(); + _ALWAYS_INLINE_ void write_lock() { + mutex.lock(); + } // Unlock the RWLock, let other threads continue. - void write_unlock(); + _ALWAYS_INLINE_ void write_unlock() { + mutex.unlock(); + } // Attempt to lock the RWLock for writing. True on success, false means it can't lock. - RWLock(); - ~RWLock(); + _ALWAYS_INLINE_ bool write_try_lock() { + return mutex.try_lock(); + } }; class RWLockRead {