From 061f3c1d90acc415f9b61b4b43d11b827846a0ca Mon Sep 17 00:00:00 2001 From: AnyOldName3 Date: Mon, 16 Mar 2026 18:41:04 +0000 Subject: [PATCH 1/2] Implement hybrid ref_ptr --- include/vsg/core/Auxiliary.h | 18 +- include/vsg/core/Inherit.h | 4 +- include/vsg/core/Object.h | 35 +- include/vsg/core/observer_ptr.h | 96 +++-- include/vsg/core/ref_ptr.h | 463 +++++++++++++++++----- include/vsg/platform/ios/iOS_Window.h | 1 + include/vsg/platform/macos/MacOS_Window.h | 3 +- include/vsg/platform/win32/Win32_Window.h | 4 +- include/vsg/vk/DeviceMemory.h | 4 +- include/vsg/vk/Instance.h | 4 +- src/vsg/core/Auxiliary.cpp | 43 +- src/vsg/core/Object.cpp | 26 +- src/vsg/io/Options.cpp | 1 + src/vsg/platform/ios/iOS_Window.mm | 4 +- src/vsg/platform/macos/MacOS_Window.mm | 4 +- src/vsg/platform/win32/Win32_Window.cpp | 4 +- src/vsg/vk/DeviceMemory.cpp | 4 +- src/vsg/vk/Instance.cpp | 4 +- 18 files changed, 537 insertions(+), 185 deletions(-) diff --git a/include/vsg/core/Auxiliary.h b/include/vsg/core/Auxiliary.h index e57cdbbfbf..6aa29c5a4c 100644 --- a/include/vsg/core/Auxiliary.h +++ b/include/vsg/core/Auxiliary.h @@ -32,10 +32,10 @@ namespace vsg virtual std::size_t getSizeOf() const { return sizeof(Auxiliary); } - void ref() const; - void unref() const; - void unref_nodelete() const; - inline unsigned int referenceCount() const { return _referenceCount.load(); } + //void ref() const; + //void unref() const; + //void unref_nodelete() const; + inline unsigned int referenceCount() const { return _referenceCount->useCount(); } virtual int compare(const Auxiliary& rhs) const; @@ -97,10 +97,18 @@ namespace vsg void resetConnectedObject(); + void callDeleteOperator() { delete this; } + friend class Object; friend class Allocator; - mutable std::atomic_uint _referenceCount; + template + friend class ref_ptr; + + friend class RefCountBase; + friend class detail::RefCountPointer; + + RefCountBase* _referenceCount; mutable std::mutex _mutex; Object* _connectedObject; diff --git a/include/vsg/core/Inherit.h b/include/vsg/core/Inherit.h index 1278a5f17e..e57390383f 100644 --- a/include/vsg/core/Inherit.h +++ b/include/vsg/core/Inherit.h @@ -34,13 +34,13 @@ namespace vsg template static ref_ptr create(Args&&... args) { - return ref_ptr(new Subclass(std::forward(args)...)); + return make_referenced(std::forward(args)...); } template static ref_ptr create_if(bool flag, Args&&... args) { - if (flag) return ref_ptr(new Subclass(std::forward(args)...)); + if (flag) return make_referenced(std::forward(args)...); return {}; } diff --git a/include/vsg/core/Object.h b/include/vsg/core/Object.h index b29a643b36..cdb1de92f8 100644 --- a/include/vsg/core/Object.h +++ b/include/vsg/core/Object.h @@ -64,12 +64,14 @@ namespace vsg Object(const Object& object, const CopyOp& copyop = {}); Object& operator=(const Object&); - static ref_ptr create() { return ref_ptr(new Object); } + static ref_ptr create() { + return make_referenced(); + } static ref_ptr create_if(bool flag) { if (flag) - return ref_ptr(new Object); + return make_referenced(); else return {}; } @@ -118,14 +120,14 @@ namespace vsg virtual void read(Input& input); virtual void write(Output& output) const; - // ref counting methods - inline void ref() const noexcept { _referenceCount.fetch_add(1, std::memory_order_relaxed); } - inline void unref() const noexcept - { - if (_referenceCount.fetch_sub(1, std::memory_order_seq_cst) <= 1) _attemptDelete(); - } - inline void unref_nodelete() const noexcept { _referenceCount.fetch_sub(1, std::memory_order_seq_cst); } - inline unsigned int referenceCount() const noexcept { return _referenceCount.load(); } + // ref counting methods - maybe kill these? + //inline void ref() const noexcept { _referenceCount->increment(); } + //inline void unref() const noexcept + //{ + // _referenceCount->decrement(); + //} + //inline void unref_nodelete() const noexcept { _referenceCount.fetch_sub(1, std::memory_order_seq_cst); } + inline unsigned int referenceCount() const noexcept { return _referenceCount->useCount(); } /// meta data access methods /// wraps the value with a vsg::Value object and then assigns via setObject(key, vsg::Value) @@ -182,12 +184,23 @@ namespace vsg virtual ~Object(); virtual void _attemptDelete() const; + void callDestructor() { this->~Object(); } + void callDeleteOperator() const { delete this; } + void assignRefCount(RefCountBase* refCount); + void setAuxiliary(Auxiliary* auxiliary); private: friend class Auxiliary; - mutable std::atomic_uint _referenceCount; + template + friend class ref_ptr; + template + friend class observer_ptr; + + friend class RefCountBase; + + mutable RefCountBase* _referenceCount; Auxiliary* _auxiliary; }; diff --git a/include/vsg/core/observer_ptr.h b/include/vsg/core/observer_ptr.h index c37633d46d..73f92caeef 100644 --- a/include/vsg/core/observer_ptr.h +++ b/include/vsg/core/observer_ptr.h @@ -12,7 +12,7 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI */ -#include +#include namespace vsg { @@ -26,84 +26,120 @@ namespace vsg using element_type = T; observer_ptr() : - _ptr(nullptr) {} + _ptr(nullptr), + _refCount(nullptr) + {} observer_ptr(const observer_ptr& rhs) : - _ptr(rhs._ptr), - _auxiliary(rhs._auxiliary) + _ptr(nullptr), + _refCount(rhs._refCount) { + if (_refCount) + { + _ptr = rhs._ptr; + _refCount->incrementObservers(); + } } template explicit observer_ptr(R* ptr) : _ptr(ptr), - _auxiliary(ptr ? ptr->getOrCreateAuxiliary() : nullptr) + _refCount(ptr ? ptr->_referenceCount : nullptr) { + if (_ptr) + _refCount->incrementObservers(); } template explicit observer_ptr(const observer_ptr& ptr) : - _ptr(ptr._ptr), - _auxiliary(ptr._auxiliary) + _ptr(nullptr), + _refCount(ptr._refCount) { + if (_refCount) + { + _ptr = ptr._ptr; + _refCount->incrementObservers(); + } } template explicit observer_ptr(const vsg::ref_ptr& ptr) : _ptr(ptr.get()), - _auxiliary(ptr.valid() ? ptr->getOrCreateAuxiliary() : nullptr) + _refCount(ptr.valid() ? ptr->_referenceCount : nullptr) { + if (_refCount) + _refCount->incrementObservers(); } ~observer_ptr() { + if (_refCount) + _refCount->decrementObservers(); } void reset() { _ptr = nullptr; - _auxiliary.reset(); + if (_refCount) + _refCount->decrementObservers(); + _refCount = nullptr; } template observer_ptr& operator=(R* ptr) { + if (_refCount) + _refCount->decrementObservers(); _ptr = ptr; - _auxiliary = ptr ? ptr->getOrCreateAuxiliary() : nullptr; + _refCount = ptr ? ptr->_referenceCount : nullptr; + if (_refCount) + _refCount->incrementObservers(); return *this; } observer_ptr& operator=(const observer_ptr& rhs) { + if (_refCount) + _refCount->decrementObservers(); _ptr = rhs._ptr; - _auxiliary = rhs._auxiliary; + _refCount = rhs._refCount; + if (_refCount) + _refCount->incrementObservers(); return *this; } template observer_ptr& operator=(const observer_ptr& rhs) { + if (_refCount) + _refCount->decrementObservers(); _ptr = rhs._ptr; - _auxiliary = rhs._auxiliary; + _refCount = rhs._refCount; + if (_refCount) + _refCount->incrementObservers(); return *this; } template observer_ptr& operator=(const vsg::ref_ptr& rhs) { + if (_refCount) + _refCount->decrementObservers(); _ptr = rhs.get(); - _auxiliary = rhs.valid() ? rhs->getOrCreateAuxiliary() : nullptr; + _refCount = rhs.valid() ? rhs->_referenceCount : nullptr; + if (_refCount) + _refCount->incrementObservers(); return *this; } template - bool operator<(const observer_ptr& rhs) const { return (rhs._ptr < _ptr) || (rhs._ptr == _ptr && rhs._auxiliary < _auxiliary); } + bool operator<(const observer_ptr& rhs) const { return (rhs._ptr < _ptr) || (rhs._ptr == _ptr && rhs._refCount < _refCount); } template - bool operator==(const observer_ptr& rhs) const { return (rhs._auxiliary == _auxiliary); } + bool operator==(const observer_ptr& rhs) const { return (rhs._refCount == _refCount); } template - bool operator!=(const observer_ptr& rhs) const { return (rhs._auxiliary != _auxiliary); } + bool operator!=(const observer_ptr& rhs) const { return (rhs._refCount != _refCount); } template bool operator<(const R* rhs) const @@ -112,7 +148,7 @@ namespace vsg return true; if (_ptr == nullptr) return false; - return rhs->getAuxiliary() < _auxiliary; + return rhs->_referenceCount < _refCount; } template @@ -122,7 +158,7 @@ namespace vsg return false; if (rhs == nullptr) return true; - return rhs->getAuxiliary() == _auxiliary; + return rhs->_referenceCount == _refCount; } template @@ -132,7 +168,7 @@ namespace vsg return true; if (rhs == nullptr) return false; - return rhs->getAuxiliary() != _auxiliary; + return rhs->_referenceCount != _refCount; } template @@ -142,7 +178,7 @@ namespace vsg return true; if (_ptr == nullptr) return false; - return rhs->getAuxiliary() < _auxiliary; + return rhs->_referenceCount < _refCount; } template @@ -152,7 +188,7 @@ namespace vsg return false; if (rhs == nullptr) return true; - return rhs->getAuxiliary() == _auxiliary; + return rhs->_referenceCount == _refCount; } template @@ -162,10 +198,10 @@ namespace vsg return true; if (rhs == nullptr) return false; - return rhs->getAuxiliary() != _auxiliary; + return rhs->_referenceCount != _refCount; } - bool valid() const noexcept { return _auxiliary.valid() && _auxiliary->getConnectedObject() != nullptr; } + bool valid() const noexcept { return _refCount && _refCount->useCount() > 0; } explicit operator bool() const noexcept { return valid(); } @@ -176,13 +212,15 @@ namespace vsg template operator vsg::ref_ptr() const { - if (!_auxiliary) return vsg::ref_ptr(); - - std::scoped_lock guard(_auxiliary->getMutex()); - if (_auxiliary->getConnectedObject() != nullptr) - return vsg::ref_ptr(_ptr); + if (_refCount && _refCount->incrementIfNonZero()) + { + detail::TemporaryOwner temp{_ptr}; + return vsg::ref_ptr(temp); + } else + { return {}; + } } protected: @@ -190,7 +228,7 @@ namespace vsg friend class observer_ptr; T* _ptr; - vsg::ref_ptr _auxiliary; + RefCountBase* _refCount; }; } // namespace vsg diff --git a/include/vsg/core/ref_ptr.h b/include/vsg/core/ref_ptr.h index 88af99f8f7..1d1023b453 100644 --- a/include/vsg/core/ref_ptr.h +++ b/include/vsg/core/ref_ptr.h @@ -12,191 +12,448 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI */ +#include +#include +#include + namespace vsg { + // smart pointer implementation aiming to get the feature richness of std::shared_ptr with the small pointer size (i.e. the same as a C pointer) of intrusive reference-counting + template + class ref_ptr; - /// smart pointer that works with objects that have intrusive reference counting, such as all vsg::Object - /// broadly similar in role to std::shared_ptr<> but half size and faster thanks to lower memory footprint and better cache coherency template - class ref_ptr + class observer_ptr; + + class RefCountBase { public: - using element_type = T; + using RefCountType = std::atomic_uint; - ref_ptr() noexcept : - _ptr(nullptr) {} + RefCountBase(const RefCountBase&) = delete; + RefCountBase& operator=(const RefCountBase&) = delete; - ref_ptr(const ref_ptr& rhs) noexcept : - _ptr(rhs._ptr) + virtual ~RefCountBase() noexcept = default; + + void increment() noexcept { - if (_ptr) _ptr->ref(); +#ifdef __GNUC__ +#pragma GCC diagnostic push +// GCC bug 107694 +#pragma GCC diagnostic ignored "-Wstringop-overflow" +#endif + _referenceCount.fetch_add(1, std::memory_order_relaxed); +#ifdef __GNUC__ +#pragma GCC diagnostic pop +#endif } - /// move constructor - template - ref_ptr(ref_ptr&& rhs) noexcept : - _ptr(rhs._ptr) + bool incrementIfNonZero() noexcept { - rhs._ptr = nullptr; + RefCountType::value_type prevCount = _referenceCount.load(); + while (prevCount != 0) + { + if (_referenceCount.compare_exchange_weak(prevCount, prevCount + 1)) + return true; + } + return false; } - template - ref_ptr(const ref_ptr& ptr) noexcept : - _ptr(ptr._ptr) + void incrementObservers() noexcept { - if (_ptr) _ptr->ref(); + _observerReferenceCount.fetch_add(1, std::memory_order_relaxed); } - explicit ref_ptr(T* ptr) noexcept : - _ptr(ptr) + void decrement() noexcept { - if (_ptr) _ptr->ref(); + if (--_referenceCount == 0) + { + destroyResource(); + decrementObservers(); + } } - template - explicit ref_ptr(R* ptr) noexcept : - _ptr(ptr) + void decrementObservers() noexcept { - if (_ptr) _ptr->ref(); + if (--_observerReferenceCount == 0) + { + destroyRefCount(); + } } - // std::nullptr_t requires extra header - ref_ptr(decltype(nullptr)) noexcept : - ref_ptr() {} - - ~ref_ptr() + RefCountType::value_type useCount() const noexcept { - if (_ptr) _ptr->unref(); + return _referenceCount; } - void reset() + protected: + constexpr RefCountBase() noexcept = default; + + template + void callDestructor(T& instance) { - if (_ptr) _ptr->unref(); - _ptr = nullptr; + instance.callDestructor(); } - ref_ptr& operator=(T* ptr) + template + void callDeleteOperator(T& instance) { - if (ptr == _ptr) return *this; + instance.callDeleteOperator(); + } - T* temp_ptr = _ptr; + template + void assignTo(T& instance) + { + // assert(instance._referenceCount == nullptr || instance._referenceCount == this); + instance._referenceCount = this; + } - _ptr = ptr; + private: + virtual void destroyResource() noexcept = 0; + virtual void destroyRefCount() noexcept = 0; - if (_ptr) _ptr->ref(); + RefCountType _referenceCount = 1; + RefCountType _observerReferenceCount = 1; + }; - // unref the original pointer after ref in case the old pointer object is a parent of the new pointer's object - if (temp_ptr) temp_ptr->unref(); + // we've got lots of template magic to make this work, but don't want to pollute the main namespace + namespace detail + { + template + class RefCountPointer : public RefCountBase + { + public: + explicit RefCountPointer(T* ptr) : + RefCountBase(), + _ptr(ptr) + { + assignTo(*_ptr); + } + + private: + void destroyResource() noexcept override + { + callDeleteOperator(*_ptr); + } + + void destroyRefCount() noexcept override + { + delete this; + } + + T* _ptr; + }; + + template + struct NeedsRefCountEarly : std::false_type + { + }; - return *this; - } + template + struct NeedsRefCountEarly> + : T::NeedsRefCountInConstructor + { + }; - ref_ptr& operator=(const ref_ptr& rhs) + template + class RefCountResource : public RefCountBase + { + public: + RefCountResource(Res resource, Deleter deleter) : + RefCountBase(), + _resource(resource), + _deleter(deleter) + {} + + ~RefCountResource() noexcept override = default; + + private: + void destroyResource() noexcept override + { + _deleter(_resource); + } + + void destroyRefCount() noexcept override + { + delete this; + } + + // todo: compressed pair/no_unique_address + Res _resource; + Deleter _deleter; + }; + + // RefCountResourceWithAllocator + +#ifdef _MSC_VER +# pragma warning(push) +# pragma warning(disable : 4624) // '%s': destructor was implicitly defined as deleted +#endif + template + class RefCountWithObject : public RefCountBase { - if (rhs._ptr == _ptr) return *this; + public: + static void* operator new(std::size_t count) + { + return T::operator new(count); + } + + static void operator delete(void* ptr) + { + T::operator delete(ptr); + } + + template + explicit RefCountWithObject(Args&&... args) : + RefCountBase() + { + if constexpr (NeedsRefCountEarly::value) + { + ::new ((void*)&storage) T(this, std::forward(args)...); + } + else + { + ::new ((void*)&storage) T(std::forward(args)...); + } + assignTo(storage); + } + + ~RefCountWithObject() noexcept override + { + // can't use default keyword due to the union, but we don't want to do anything here + } + + // trick to prevent double-destruction + union + { + std::remove_cv_t storage; + }; + + private: + void destroyResource() noexcept override + { + callDestructor(storage); + } + + void destroyRefCount() noexcept override + { + delete this; + } + }; + + template + class RefCountWithObjectAndAllocator : public RefCountBase + { + private: + static_assert(std::is_same_v>, "allocate_shared didn't remove_cv_t"); + + public: + using Rebound = typename std::allocator_traits::template rebind_alloc; + + template + explicit RefCountWithObjectAndAllocator(const Alloc& alloc, Args&&... args) : + RefCountBase(), + allocator(alloc) + { + // roughly equivalent to + // alloc.construct(&storage, std::forward(args)...); + // we can't use that, though, as vsg::Object destructor is protected, so std::is_constructible_v is false + if constexpr (!std::uses_allocator_v) + { + ::new ((void*)&storage) T(std::forward(args)...); + } + else + { + ::new ((void*)&storage) T(std::forward(args)..., alloc); + } + assignTo(storage); + } + + ~RefCountWithObjectAndAllocator() noexcept + { + // can't use default keyword due to the union, but we don't want to do anything here + } + + // trick to prevent double-destruction + union + { + T storage; + }; + + // todo: empty base optimisation/no_unique_address + Rebound allocator; + + private: + void destroyResource() noexcept override + { + callDestructor(storage); + } + + void destroyRefCount() noexcept override + { + this->~RefCountWithObjectAndAllocator(); + std::allocator_traits::deallocate(allocator, this, 1); + } + }; +#ifdef _MSC_VER +# pragma warning(pop) +#endif + + template + struct TemporaryOwner + { + T* ptr = nullptr; + }; + } // namespace detail - T* temp_ptr = _ptr; + template + class ref_ptr + { + public: + using element_type = T; + using observer_type = observer_ptr; - _ptr = rhs._ptr; + constexpr ref_ptr() noexcept = default; - if (_ptr) _ptr->ref(); + constexpr ref_ptr(std::nullptr_t) noexcept {} - // unref the original pointer after ref in case the old pointer object is a parent of the new pointer's object - if (temp_ptr) temp_ptr->unref(); + ref_ptr(detail::TemporaryOwner& ptr) noexcept : + _ptr(ptr.ptr) + { + ptr.ptr = nullptr; + } - return *this; + explicit ref_ptr(T* ptr) noexcept : + _ptr(ptr) + { + if (_ptr) + { + if (_ptr->_referenceCount) + _ptr->_referenceCount->increment(); + else + _ptr->_referenceCount = new detail::RefCountPointer(_ptr); + } } - template - ref_ptr& operator=(const ref_ptr& rhs) + ref_ptr& operator=(T* ptr) { - if (rhs._ptr == _ptr) return *this; + if (ptr == _ptr) return *this; T* temp_ptr = _ptr; - _ptr = rhs._ptr; + _ptr = ptr; - if (_ptr) _ptr->ref(); + if (_ptr) + { + if (_ptr->_referenceCount) + _ptr->_referenceCount->increment(); + else + _ptr->_referenceCount = new detail::RefCountPointer(_ptr); + } // unref the original pointer after ref in case the old pointer object is a parent of the new pointer's object - if (temp_ptr) temp_ptr->unref(); + if (temp_ptr) temp_ptr->_referenceCount->decrement(); return *this; } - /// move assignment - template - ref_ptr& operator=(ref_ptr&& rhs) + ref_ptr(const ref_ptr& rhs) noexcept: + _ptr(rhs._ptr) { - if (rhs._ptr == _ptr) return *this; - - if (_ptr) _ptr->unref(); - - _ptr = rhs._ptr; - - rhs._ptr = nullptr; + if (_ptr) + _ptr->_referenceCount->increment(); + } + ref_ptr& operator=(const ref_ptr& rhs) noexcept + { + ref_ptr(rhs).swap(*this); return *this; } template - bool operator<(const ref_ptr& rhs) const { return (_ptr < rhs._ptr); } - - template - bool operator==(const ref_ptr& rhs) const { return (rhs._ptr == _ptr); } - - template - bool operator!=(const ref_ptr& rhs) const { return (rhs._ptr != _ptr); } + ref_ptr(const ref_ptr& rhs) noexcept : + _ptr(rhs._ptr) + { + if (_ptr) + _ptr->_referenceCount->increment(); + } template - bool operator<(const R* rhs) const { return (_ptr < rhs); } + ref_ptr& operator=(const ref_ptr& rhs) noexcept + { + ref_ptr(rhs).swap(*this); + return *this; + } - template - bool operator==(const R* rhs) const { return (rhs == _ptr); } + ~ref_ptr() noexcept + { + if (_ptr) + _ptr->_referenceCount->decrement(); + } - template - bool operator!=(const R* rhs) const { return (rhs != _ptr); } + void reset() noexcept + { + ref_ptr().swap(*this); + } - bool valid() const noexcept { return _ptr != nullptr; } + void swap(ref_ptr& other) noexcept + { + std::swap(_ptr, other._ptr); + } - explicit operator bool() const noexcept { return valid(); } + [[nodiscard]] element_type* get() const noexcept + { + return _ptr; + } - // potentially dangerous automatic type conversion, could cause dangling pointer if ref_ptr<> assigned to C pointer and ref_ptr<> destruction causes an object delete. - operator T*() const noexcept { return _ptr; } + bool valid() const noexcept + { + return get() != nullptr; + } - void operator[](int) const = delete; + explicit operator bool() const noexcept + { + return valid(); + } T& operator*() const noexcept { return *_ptr; } T* operator->() const noexcept { return _ptr; } - T* get() const noexcept { return _ptr; } - - T* release_nodelete() noexcept - { - T* temp_ptr = _ptr; - - if (_ptr) _ptr->unref_nodelete(); - _ptr = nullptr; - - return temp_ptr; - } + // potentially dangerous automatic type conversion, could cause dangling pointer if ref_ptr<> assigned to C pointer and ref_ptr<> destruction causes an object delete. + operator T*() const noexcept { return _ptr; } - void swap(ref_ptr& rhs) noexcept - { - T* temp_ptr = _ptr; - _ptr = rhs._ptr; - rhs._ptr = temp_ptr; - } + void operator[](int) const = delete; template ref_ptr cast() const { return ref_ptr(_ptr ? _ptr->template cast() : nullptr); } - protected: + private: template friend class ref_ptr; - T* _ptr; + element_type* _ptr = nullptr; }; + // like std::make_shared + template + [[nodiscard]] ref_ptr make_referenced(Args&&... args) + { + auto* controlBlock = new detail::RefCountWithObject(std::forward(args)...); + detail::TemporaryOwner temp{std::addressof(controlBlock->storage)}; + return ref_ptr(temp); + } + + // like std::allocate_shared + template + [[nodiscard]] ref_ptr allocate_referenced(const Alloc& allocator, Args&&... args) + { + using ControlBlock = detail::RefCountWithObjectAndAllocator, Alloc>; + auto reboundAlloc = typename ControlBlock::Rebound(allocator); + ControlBlock* controlBlock = std::allocator_traits::allocate(reboundAlloc, 1); + ::new ((void*)controlBlock) ControlBlock(reboundAlloc, std::forward(args)...); + detail::TemporaryOwner temp{std::addressof(controlBlock->storage)}; + return ref_ptr(temp); + } + } // namespace vsg diff --git a/include/vsg/platform/ios/iOS_Window.h b/include/vsg/platform/ios/iOS_Window.h index 973cd7093a..a8fc00fb58 100644 --- a/include/vsg/platform/ios/iOS_Window.h +++ b/include/vsg/platform/ios/iOS_Window.h @@ -46,6 +46,7 @@ namespace vsgiOS class iOS_Window : public vsg::Inherit { public: + using NeedsRefCountInConstructor = std::true_type; iOS_Window(vsg::ref_ptr traits); iOS_Window() = delete; diff --git a/include/vsg/platform/macos/MacOS_Window.h b/include/vsg/platform/macos/MacOS_Window.h index 21b9331680..18137b6088 100644 --- a/include/vsg/platform/macos/MacOS_Window.h +++ b/include/vsg/platform/macos/MacOS_Window.h @@ -49,8 +49,9 @@ namespace vsgMacOS class MacOS_Window : public vsg::Inherit { public: + using NeedsRefCountInConstructor = std::true_type; - MacOS_Window(vsg::ref_ptr traits); + MacOS_Window(vsg::RefCountBase* refCount, ::ref_ptr traits); MacOS_Window() = delete; MacOS_Window(const MacOS_Window&) = delete; MacOS_Window operator = (const MacOS_Window&) = delete; diff --git a/include/vsg/platform/win32/Win32_Window.h b/include/vsg/platform/win32/Win32_Window.h index 54755cf159..226588a4fa 100644 --- a/include/vsg/platform/win32/Win32_Window.h +++ b/include/vsg/platform/win32/Win32_Window.h @@ -183,7 +183,9 @@ namespace vsgWin32 class VSG_DECLSPEC Win32_Window : public vsg::Inherit { public: - Win32_Window(vsg::ref_ptr traits); + using NeedsRefCountInConstructor = std::true_type; + + Win32_Window(vsg::RefCountBase* refCount, vsg::ref_ptr traits); Win32_Window() = delete; Win32_Window(const Win32_Window&) = delete; Win32_Window operator=(const Win32_Window&) = delete; diff --git a/include/vsg/vk/DeviceMemory.h b/include/vsg/vk/DeviceMemory.h index ebd86590c3..ef9a0b5eaa 100644 --- a/include/vsg/vk/DeviceMemory.h +++ b/include/vsg/vk/DeviceMemory.h @@ -28,7 +28,9 @@ namespace vsg class VSG_DECLSPEC DeviceMemory : public Inherit { public: - DeviceMemory(Device* device, const VkMemoryRequirements& memRequirements, VkMemoryPropertyFlags properties, void* pNextAllocInfo = nullptr); + using NeedsRefCountInConstructor = std::true_type; + + DeviceMemory(RefCountBase* refCount, Device* device, const VkMemoryRequirements& memRequirements, VkMemoryPropertyFlags properties, void* pNextAllocInfo = nullptr); operator VkDeviceMemory() const { return _deviceMemory; } VkDeviceMemory vk() const { return _deviceMemory; } diff --git a/include/vsg/vk/Instance.h b/include/vsg/vk/Instance.h index c863a99923..bb81920af0 100644 --- a/include/vsg/vk/Instance.h +++ b/include/vsg/vk/Instance.h @@ -48,7 +48,9 @@ namespace vsg class VSG_DECLSPEC Instance : public Inherit { public: - Instance(Names instanceExtensions, Names layers, uint32_t vulkanApiVersion = VK_API_VERSION_1_0, AllocationCallbacks* allocator = nullptr); + using NeedsRefCountInConstructor = std::true_type; + + Instance(RefCountBase* refCount, Names instanceExtensions, Names layers, uint32_t vulkanApiVersion = VK_API_VERSION_1_0, AllocationCallbacks* allocator = nullptr); /// Vulkan apiVersion used when creating the VkInstance const uint32_t apiVersion = VK_API_VERSION_1_0; diff --git a/src/vsg/core/Auxiliary.cpp b/src/vsg/core/Auxiliary.cpp index 04abafc36f..0d44184479 100644 --- a/src/vsg/core/Auxiliary.cpp +++ b/src/vsg/core/Auxiliary.cpp @@ -19,9 +19,10 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI using namespace vsg; Auxiliary::Auxiliary(Object* object) : - _referenceCount(0), + _referenceCount(nullptr), _connectedObject(object) { + new detail::RefCountPointer(this); //vsg::debug("Auxiliary::Auxiliary(Object = ", object, ") ", this); } @@ -30,26 +31,26 @@ Auxiliary::~Auxiliary() //vsg::debug("Auxiliary::~Auxiliary() ", this); } -void Auxiliary::ref() const -{ - ++_referenceCount; - //debug("Auxiliary::ref() ", this, " ", _referenceCount.load()); -} - -void Auxiliary::unref() const -{ - //debug("Auxiliary::unref() ", this, " ", _referenceCount.load()); - if (_referenceCount.fetch_sub(1) <= 1) - { - delete this; - } -} - -void Auxiliary::unref_nodelete() const -{ - //debug("Auxiliary::unref_nodelete() ", this, " ", _referenceCount.load()); - --_referenceCount; -} +//void Auxiliary::ref() const +//{ +// ++_referenceCount; +// //debug("Auxiliary::ref() ", this, " ", _referenceCount.load()); +//} +// +//void Auxiliary::unref() const +//{ +// //debug("Auxiliary::unref() ", this, " ", _referenceCount.load()); +// if (_referenceCount.fetch_sub(1) <= 1) +// { +// delete this; +// } +//} +// +//void Auxiliary::unref_nodelete() const +//{ +// //debug("Auxiliary::unref_nodelete() ", this, " ", _referenceCount.load()); +// --_referenceCount; +//} bool Auxiliary::signalConnectedObjectToBeDeleted() { diff --git a/src/vsg/core/Object.cpp b/src/vsg/core/Object.cpp index 8b39dedfb0..165ae68a70 100644 --- a/src/vsg/core/Object.cpp +++ b/src/vsg/core/Object.cpp @@ -31,14 +31,24 @@ Object::Object() : Object::Object(const Object& rhs, const CopyOp& copyop) : Object() { + /* I'm not sure this block can ever do anything productive - if it's called from CopyOp, and itr->second is null, + then CopyOp will assign this itself, and if it's not null, then it'll use the existing value instead of constructing + something fresh. + I think that's the only sane callsite. + However, this was added a couple of weeks after the CopyOp stuff that I think makes it seem redundant, so maybe I'm + missing something. + + While it's detonative, I'll just comment it out. + // assign this copy constructed object to copyop.duplicate so that it can be lated referenced. if (copyop.duplicate) { if (auto itr = copyop.duplicate->find(&rhs); itr != copyop.duplicate->end()) { + // this will blow up as the refcount hasn't been set yet itr->second = this; } - } + } */ if (rhs._auxiliary && rhs._auxiliary->getConnectedObject() == &rhs) { @@ -77,7 +87,8 @@ Object::~Object() if (_auxiliary) { - _auxiliary->unref(); + _auxiliary->resetConnectedObject(); + _auxiliary->_referenceCount->decrement(); } } @@ -206,19 +217,25 @@ void Object::removeObject(const std::string& key) } } +void Object::assignRefCount(RefCountBase* refCount) +{ + //assert(_referenceCount == nullptr || _referenceCount == refCount); + _referenceCount = refCount; +} + void Object::setAuxiliary(Auxiliary* auxiliary) { if (_auxiliary) { _auxiliary->resetConnectedObject(); - _auxiliary->unref(); + _auxiliary->_referenceCount->decrement(); } _auxiliary = auxiliary; if (auxiliary) { - auxiliary->ref(); + auxiliary->_referenceCount->increment(); } } @@ -228,7 +245,6 @@ Auxiliary* Object::getOrCreateAuxiliary() if (!_auxiliary) { _auxiliary = new Auxiliary(this); - _auxiliary->ref(); } return _auxiliary; } diff --git a/src/vsg/io/Options.cpp b/src/vsg/io/Options.cpp index c7ebf30bb8..17159455c9 100644 --- a/src/vsg/io/Options.cpp +++ b/src/vsg/io/Options.cpp @@ -10,6 +10,7 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI */ +#include #include #include #include diff --git a/src/vsg/platform/ios/iOS_Window.mm b/src/vsg/platform/ios/iOS_Window.mm index e58fd7ac08..65e9e2d0ed 100644 --- a/src/vsg/platform/ios/iOS_Window.mm +++ b/src/vsg/platform/ios/iOS_Window.mm @@ -256,9 +256,11 @@ - (void)mouseMoved:(UIEvent *)event }; } -vsgiOS::iOS_Window::iOS_Window(vsg::ref_ptr traits) +vsgiOS::iOS_Window::iOS_Window(vsg::RefCountBase* refCount, vsg::ref_ptr traits) : Inherit(traits) { + assignRefCount(refCount); + auto devicePixelScale = _traits->hdpi ? UIScreen.mainScreen.nativeScale : 1.0f; _window = std::any_cast(traits->nativeWindow); _view = (vsg_iOS_View*)( _window.rootViewController.view ); diff --git a/src/vsg/platform/macos/MacOS_Window.mm b/src/vsg/platform/macos/MacOS_Window.mm index 8b0da4bea9..1b36612ff4 100644 --- a/src/vsg/platform/macos/MacOS_Window.mm +++ b/src/vsg/platform/macos/MacOS_Window.mm @@ -707,9 +707,11 @@ void createApplicationMenus(void) return true; } -MacOS_Window::MacOS_Window(vsg::ref_ptr traits) : +MacOS_Window::MacOS_Window(vsg::RefCountBase* refCount, vsg::ref_ptr traits) : Inherit(traits) { + assignRefCount(refCount); + _keyboard = new KeyboardMap; NSRect contentRect = NSMakeRect(0, 0, traits->width, traits->height); diff --git a/src/vsg/platform/win32/Win32_Window.cpp b/src/vsg/platform/win32/Win32_Window.cpp index ef7c77ccab..7b4f1c62f6 100644 --- a/src/vsg/platform/win32/Win32_Window.cpp +++ b/src/vsg/platform/win32/Win32_Window.cpp @@ -328,10 +328,12 @@ KeyboardMap::KeyboardMap() // clang-format on } -Win32_Window::Win32_Window(vsg::ref_ptr traits) : +Win32_Window::Win32_Window(RefCountBase* refCount, vsg::ref_ptr traits) : Inherit(traits), _window(nullptr) { + assignRefCount(refCount); + _keyboard = new KeyboardMap; #ifdef UNICODE diff --git a/src/vsg/vk/DeviceMemory.cpp b/src/vsg/vk/DeviceMemory.cpp index 086637fcc1..82322d8da9 100644 --- a/src/vsg/vk/DeviceMemory.cpp +++ b/src/vsg/vk/DeviceMemory.cpp @@ -42,12 +42,14 @@ DeviceMemoryList vsg::getActiveDeviceMemoryList(VkMemoryPropertyFlagBits propert // // DeviceMemory // -DeviceMemory::DeviceMemory(Device* device, const VkMemoryRequirements& memRequirements, VkMemoryPropertyFlags properties, void* pNextAllocInfo) : +DeviceMemory::DeviceMemory(RefCountBase* refCount, Device* device, const VkMemoryRequirements& memRequirements, VkMemoryPropertyFlags properties, void* pNextAllocInfo) : _memoryRequirements(memRequirements), _properties(properties), _device(device), _memorySlots(memRequirements.size) { + assignRefCount(refCount); + uint32_t typeFilter = memRequirements.memoryTypeBits; // find the memory type to use diff --git a/src/vsg/vk/Instance.cpp b/src/vsg/vk/Instance.cpp index d34e3b9297..ef857d2248 100644 --- a/src/vsg/vk/Instance.cpp +++ b/src/vsg/vk/Instance.cpp @@ -122,9 +122,11 @@ static VKAPI_ATTR VkBool32 VKAPI_CALL debugUtilsMessengerCallback( return VK_FALSE; } -Instance::Instance(Names instanceExtensions, Names layers, uint32_t vulkanApiVersion, AllocationCallbacks* allocator) : +Instance::Instance(RefCountBase* refCount, Names instanceExtensions, Names layers, uint32_t vulkanApiVersion, AllocationCallbacks* allocator) : apiVersion(vulkanApiVersion) { + assignRefCount(refCount); + // application info VkApplicationInfo appInfo = {}; appInfo.sType = VK_STRUCTURE_TYPE_APPLICATION_INFO; From 217996243d64141cd042eee48e3a76d0e49db649 Mon Sep 17 00:00:00 2001 From: AnyOldName3 Date: Tue, 24 Mar 2026 19:10:23 +0000 Subject: [PATCH 2/2] Don't co-allocate ref count by default --- include/vsg/core/ref_ptr.h | 35 +++++++++++++++++++++++++++++++---- 1 file changed, 31 insertions(+), 4 deletions(-) diff --git a/include/vsg/core/ref_ptr.h b/include/vsg/core/ref_ptr.h index 1d1023b453..a105762f2d 100644 --- a/include/vsg/core/ref_ptr.h +++ b/include/vsg/core/ref_ptr.h @@ -130,6 +130,16 @@ namespace vsg assignTo(*_ptr); } + template + explicit RefCountPointer(Args&&... args) : + RefCountBase(), + _ptr(new T(this, std::forward(args)...)) + { + assignTo(*_ptr); + } + + T* _ptr; + private: void destroyResource() noexcept override { @@ -140,8 +150,6 @@ namespace vsg { delete this; } - - T* _ptr; }; template @@ -435,9 +443,28 @@ namespace vsg element_type* _ptr = nullptr; }; - // like std::make_shared + // like std::make_shared, but allocates the ref count separately template [[nodiscard]] ref_ptr make_referenced(Args&&... args) + { + if constexpr (detail::NeedsRefCountEarly::value) + { + auto* controlBlock = new detail::RefCountPointer(std::forward(args)...); + detail::TemporaryOwner temp{controlBlock->_ptr}; + return ref_ptr(temp); + } + else + { + auto* instance = new T(std::forward(args)...); + new detail::RefCountPointer(instance); + detail::TemporaryOwner temp{instance}; + return ref_ptr(temp); + } + } + + // like std::make_shared + template + [[nodiscard]] ref_ptr make_referenced_adjacent_ref_count(Args&&... args) { auto* controlBlock = new detail::RefCountWithObject(std::forward(args)...); detail::TemporaryOwner temp{std::addressof(controlBlock->storage)}; @@ -446,7 +473,7 @@ namespace vsg // like std::allocate_shared template - [[nodiscard]] ref_ptr allocate_referenced(const Alloc& allocator, Args&&... args) + [[nodiscard]] ref_ptr allocate_referenced_adjacent_ref_count(const Alloc& allocator, Args&&... args) { using ControlBlock = detail::RefCountWithObjectAndAllocator, Alloc>; auto reboundAlloc = typename ControlBlock::Rebound(allocator);