From e118ed5e38c3e909f575c5618449556db93400a7 Mon Sep 17 00:00:00 2001 From: Kevin Ring Date: Thu, 25 Jan 2024 17:26:57 +1100 Subject: [PATCH 1/5] Add ReferenceCountedThreadSafe. --- .../ReferenceCountedThreadSafe.h | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 CesiumUtility/include/CesiumUtility/ReferenceCountedThreadSafe.h diff --git a/CesiumUtility/include/CesiumUtility/ReferenceCountedThreadSafe.h b/CesiumUtility/include/CesiumUtility/ReferenceCountedThreadSafe.h new file mode 100644 index 000000000..c56ee41ba --- /dev/null +++ b/CesiumUtility/include/CesiumUtility/ReferenceCountedThreadSafe.h @@ -0,0 +1,52 @@ +#pragma once + +#include +#include + +namespace CesiumUtility { + +/** + * @brief A reference-counted base class, meant to be used with + * {@link IntrusivePointer}. The reference count is thread-safe, so references + * may be added and removed from any thread at any time. The object will be + * destroyed in the thread that releases the last reference. + * + * @tparam T The type that is _deriving_ from this class. For example, you + * should declare your class as + * `class MyClass : public ReferenceCountedThreadSafe { ... };` + */ +template class ReferenceCountedThreadSafe { +public: + /** + * @brief Adds a counted reference to this object. Use + * {@link CesiumUtility::IntrusivePointer} instead of calling this method + * directly. + */ + void addReference() const noexcept { ++this->_referenceCount; } + + /** + * @brief Removes a counted reference from this object. When the last + * reference is removed, this method will delete this instance. Use + * {@link CesiumUtility::IntrusivePointer} instead of calling this method + * directly. + */ + void releaseReference() const noexcept { + assert(this->_referenceCount > 0); + const int32_t references = --this->_referenceCount; + if (references == 0) { + delete static_cast(this); + } + } + + /** + * @brief Returns the current reference count of this instance. + */ + std::int32_t getReferenceCount() const noexcept { + return this->_referenceCount; + } + +private: + mutable std::atomic _referenceCount{0}; +}; + +} // namespace CesiumUtility From e2d79d1a020c63bb09f2e286e5117a8a2cbbe97c Mon Sep 17 00:00:00 2001 From: Kevin Ring Date: Wed, 7 Feb 2024 19:12:22 +1100 Subject: [PATCH 2/5] Update CHANGES.md. --- CHANGES.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGES.md b/CHANGES.md index 9369bc6ac..efe27ccb2 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,5 +1,11 @@ # Change Log +### ? - ? + +##### Additions :tada: + +- Added `ReferenceCountedThreadSafe` class. + ### v0.32.0 - 20234-02-01 ##### Breaking Changes :mega: From 9f8b011a6edd9f5bf4e2d442a7b1b45c03b8312b Mon Sep 17 00:00:00 2001 From: Kevin Ring Date: Sun, 25 Feb 2024 22:44:05 +1100 Subject: [PATCH 3/5] Just one templatized ReferenceCounted class. --- .../RasterOverlayCollection.h | 2 +- .../src/TilesetContentManager.h | 2 +- .../CesiumRasterOverlays/RasterOverlay.h | 2 +- .../CesiumRasterOverlays/RasterOverlayTile.h | 2 +- .../RasterOverlayTileProvider.h | 2 +- .../include/CesiumUtility/ReferenceCounted.h | 121 ++++++++++++++++++ .../ReferenceCountedNonThreadSafe.h | 83 ------------ .../ReferenceCountedThreadSafe.h | 52 -------- 8 files changed, 126 insertions(+), 140 deletions(-) create mode 100644 CesiumUtility/include/CesiumUtility/ReferenceCounted.h delete mode 100644 CesiumUtility/include/CesiumUtility/ReferenceCountedNonThreadSafe.h delete mode 100644 CesiumUtility/include/CesiumUtility/ReferenceCountedThreadSafe.h diff --git a/Cesium3DTilesSelection/include/Cesium3DTilesSelection/RasterOverlayCollection.h b/Cesium3DTilesSelection/include/Cesium3DTilesSelection/RasterOverlayCollection.h index 8f49ad6de..d4754517c 100644 --- a/Cesium3DTilesSelection/include/Cesium3DTilesSelection/RasterOverlayCollection.h +++ b/Cesium3DTilesSelection/include/Cesium3DTilesSelection/RasterOverlayCollection.h @@ -6,7 +6,7 @@ #include #include #include -#include +#include #include #include diff --git a/Cesium3DTilesSelection/src/TilesetContentManager.h b/Cesium3DTilesSelection/src/TilesetContentManager.h index 611e218f2..d750b2207 100644 --- a/Cesium3DTilesSelection/src/TilesetContentManager.h +++ b/Cesium3DTilesSelection/src/TilesetContentManager.h @@ -12,7 +12,7 @@ #include #include #include -#include +#include #include diff --git a/CesiumRasterOverlays/include/CesiumRasterOverlays/RasterOverlay.h b/CesiumRasterOverlays/include/CesiumRasterOverlays/RasterOverlay.h index 2176a804a..4afeb6f57 100644 --- a/CesiumRasterOverlays/include/CesiumRasterOverlays/RasterOverlay.h +++ b/CesiumRasterOverlays/include/CesiumRasterOverlays/RasterOverlay.h @@ -6,7 +6,7 @@ #include #include #include -#include +#include #include #include diff --git a/CesiumRasterOverlays/include/CesiumRasterOverlays/RasterOverlayTile.h b/CesiumRasterOverlays/include/CesiumRasterOverlays/RasterOverlayTile.h index 917ab902d..f74023e16 100644 --- a/CesiumRasterOverlays/include/CesiumRasterOverlays/RasterOverlayTile.h +++ b/CesiumRasterOverlays/include/CesiumRasterOverlays/RasterOverlayTile.h @@ -5,7 +5,7 @@ #include #include #include -#include +#include #include diff --git a/CesiumRasterOverlays/include/CesiumRasterOverlays/RasterOverlayTileProvider.h b/CesiumRasterOverlays/include/CesiumRasterOverlays/RasterOverlayTileProvider.h index 9f74e5b39..1f5c1d8f1 100644 --- a/CesiumRasterOverlays/include/CesiumRasterOverlays/RasterOverlayTileProvider.h +++ b/CesiumRasterOverlays/include/CesiumRasterOverlays/RasterOverlayTileProvider.h @@ -7,7 +7,7 @@ #include #include #include -#include +#include #include #include diff --git a/CesiumUtility/include/CesiumUtility/ReferenceCounted.h b/CesiumUtility/include/CesiumUtility/ReferenceCounted.h new file mode 100644 index 000000000..e245a7678 --- /dev/null +++ b/CesiumUtility/include/CesiumUtility/ReferenceCounted.h @@ -0,0 +1,121 @@ +#pragma once + +#include + +#ifndef NDEBUG +#include +#endif + +namespace CesiumUtility { + +/** + * @brief A reference-counted base class, meant to be used with + * {@link IntrusivePointer}. + * + * Consider using {@link ReferenceCountedThreadSafe} or + * {@link ReferenceCountedNoThreadSafe} instead of using this class directly. + * + * @tparam T The type that is _deriving_ from this class. For example, you + * should declare your class as + * `class MyClass : public ReferenceCounted { ... };` + * @tparam isThreadSafe If `true`, the reference count will be thread-safe by + * using `std::atomic`, allowing references to safely be added and removed from + * any thread at any time. The object will be destroyed in the thread that + * releases the last reference. If false, it uses a simple integer for the + * reference count, which is not thread safe. In this case, references must be + * added and removed (including automatically via `IntrusivePointer`) from only + * one thread at a time. However, this mode has a bit less overhead for objects + * that are only ever accessed from a single thread. + */ +template class ReferenceCounted { +public: + ReferenceCounted() noexcept +#ifndef NDEBUG + : _threadID(std::this_thread::get_id()) +#endif + { + } + + ~ReferenceCounted() noexcept { assert(this->_referenceCount == 0); } + + /** + * @brief Adds a counted reference to this object. Use + * {@link CesiumUtility::IntrusivePointer} instead of calling this method + * directly. + */ + void addReference() const /*noexcept*/ { +#ifndef NDEBUG + if constexpr (!isThreadSafe) { + assert(std::this_thread::get_id() == this->_threadID); + } +#endif + + ++this->_referenceCount; + } + + /** + * @brief Removes a counted reference from this object. When the last + * reference is removed, this method will delete this instance. Use + * {@link CesiumUtility::IntrusivePointer} instead of calling this method + * directly. + */ + void releaseReference() const /*noexcept*/ { +#ifndef NDEBUG + if constexpr (!isThreadSafe) { + assert(std::this_thread::get_id() == this->_threadID); + } +#endif + assert(this->_referenceCount > 0); + const int32_t references = --this->_referenceCount; + if (references == 0) { + delete static_cast(this); + } + } + + /** + * @brief Returns the current reference count of this instance. + */ + std::int32_t getReferenceCount() const noexcept { + return this->_referenceCount; + } + +private: + using ThreadSafeCounter = std::atomic; + using NonThreadSafeCounter = std::int32_t; + using CounterType = + std::conditional_t; + + mutable CounterType _referenceCount{0}; + +#ifndef NDEBUG + std::thread::id _threadID; +#endif +}; + +/** + * @brief A reference-counted base class, meant to be used with + * {@link IntrusivePointer}. The reference count is thread-safe, so references + * may be added and removed from any thread at any time. The object will be + * destroyed in the thread that releases the last reference. + * + * @tparam T The type that is _deriving_ from this class. For example, you + * should declare your class as + * `class MyClass : public ReferenceCountedThreadSafe { ... };` + */ +template +using ReferenceCountedThreadSafe = typename ReferenceCounted; + +/** + * @brief A reference-counted base class, meant to be used with + * {@link IntrusivePointer}. The reference count is not thread-safe, so + * references must be added and removed (including automatically via + * `IntrusivePointer`) from only one thread at a time. + * + * @tparam T The type that is _deriving_ from this class. For example, you + * should declare your class as + * `class MyClass : public ReferenceCountedNonThreadSafe { ... };` + */ +template +using ReferenceCountedNonThreadSafe = typename ReferenceCounted; + +} // namespace CesiumUtility diff --git a/CesiumUtility/include/CesiumUtility/ReferenceCountedNonThreadSafe.h b/CesiumUtility/include/CesiumUtility/ReferenceCountedNonThreadSafe.h deleted file mode 100644 index edc9014bc..000000000 --- a/CesiumUtility/include/CesiumUtility/ReferenceCountedNonThreadSafe.h +++ /dev/null @@ -1,83 +0,0 @@ -#pragma once - -#include - -#ifndef NDEBUG -#include -#endif - -namespace CesiumUtility { - -/** - * @brief A reference-counted base class, meant to be used with - * {@link IntrusivePointer}. The reference count is not thread-safe, so - * references must be added and removed (including automatically via - * `IntrusivePointer`) from only one thread at a time. - * - * @tparam T The type that is _deriving_ from this class. For example, you - * should declare your class as - * `class MyClass : public ReferenceCountedNonThreadSafe { ... };` - */ -template class ReferenceCountedNonThreadSafe { -public: - ReferenceCountedNonThreadSafe() noexcept -#ifndef NDEBUG - : _threadID(std::this_thread::get_id()) -#endif - { - } - - ~ReferenceCountedNonThreadSafe() noexcept { - assert(this->_referenceCount == 0); - } - - /** - * @brief Adds a counted reference to this object. Use - * {@link CesiumUtility::IntrusivePointer} instead of calling this method - * directly. - * - * This method is _not_ thread safe. Do not call it or use an - * `IntrusivePointer` from multiple threads simultaneously. - */ - void addReference() const /*noexcept*/ { -#ifndef NDEBUG - assert(std::this_thread::get_id() == this->_threadID); -#endif - ++this->_referenceCount; - } - - /** - * @brief Removes a counted reference from this object. When the last - * reference is removed, this method will delete this instance. Use - * {@link CesiumUtility::IntrusivePointer} instead of calling this method - * directly. - * - * This method is _not_ thread safe. Do not call it or use an - * `IntrusivePointer` from multiple threads simultaneously. - */ - void releaseReference() const /*noexcept*/ { -#ifndef NDEBUG - assert(std::this_thread::get_id() == this->_threadID); -#endif - assert(this->_referenceCount > 0); - const int32_t references = --this->_referenceCount; - if (references == 0) { - delete static_cast(this); - } - } - - /** - * @brief Returns the current reference count of this instance. - */ - std::int32_t getReferenceCount() const noexcept { - return this->_referenceCount; - } - -private: - mutable std::int32_t _referenceCount{0}; -#ifndef NDEBUG - std::thread::id _threadID; -#endif -}; - -} // namespace CesiumUtility diff --git a/CesiumUtility/include/CesiumUtility/ReferenceCountedThreadSafe.h b/CesiumUtility/include/CesiumUtility/ReferenceCountedThreadSafe.h deleted file mode 100644 index c56ee41ba..000000000 --- a/CesiumUtility/include/CesiumUtility/ReferenceCountedThreadSafe.h +++ /dev/null @@ -1,52 +0,0 @@ -#pragma once - -#include -#include - -namespace CesiumUtility { - -/** - * @brief A reference-counted base class, meant to be used with - * {@link IntrusivePointer}. The reference count is thread-safe, so references - * may be added and removed from any thread at any time. The object will be - * destroyed in the thread that releases the last reference. - * - * @tparam T The type that is _deriving_ from this class. For example, you - * should declare your class as - * `class MyClass : public ReferenceCountedThreadSafe { ... };` - */ -template class ReferenceCountedThreadSafe { -public: - /** - * @brief Adds a counted reference to this object. Use - * {@link CesiumUtility::IntrusivePointer} instead of calling this method - * directly. - */ - void addReference() const noexcept { ++this->_referenceCount; } - - /** - * @brief Removes a counted reference from this object. When the last - * reference is removed, this method will delete this instance. Use - * {@link CesiumUtility::IntrusivePointer} instead of calling this method - * directly. - */ - void releaseReference() const noexcept { - assert(this->_referenceCount > 0); - const int32_t references = --this->_referenceCount; - if (references == 0) { - delete static_cast(this); - } - } - - /** - * @brief Returns the current reference count of this instance. - */ - std::int32_t getReferenceCount() const noexcept { - return this->_referenceCount; - } - -private: - mutable std::atomic _referenceCount{0}; -}; - -} // namespace CesiumUtility From 579ec6fad7614963824e2b1c7e9b24710832dbac Mon Sep 17 00:00:00 2001 From: Kevin Ring Date: Sun, 25 Feb 2024 22:48:34 +1100 Subject: [PATCH 4/5] Update CHANGES.md. --- CHANGES.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index 79f1698c9..ba1fd570b 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -5,12 +5,13 @@ ##### Breaking Changes :mega: - Removed support for `EXT_feature_metadata` in `CesiumGltf`, `CesiumGltfReader`, and `CesiumGltfWriter`. This extension was replaced by `EXT_mesh_features`, `EXT_instance_features`, and `EXT_structural_metadata`. +- Moved `ReferenceCountedNonThreadSafe` to `ReferenceCounted.h`. It also now a type alias for `ReferenceCounted` rather than an actual class. ##### Additions :tada: - Added `contains` method to `BoundingSphere`. - Added `GlobeRectangle::MAXIMUM` static field. -- Added `ReferenceCountedThreadSafe` class. +- Added `ReferenceCountedThreadSafe` type alias. ##### Fixes :wrench: From 1e7af45c4819f36286b87e293164f0c95b2405c5 Mon Sep 17 00:00:00 2001 From: Kevin Ring Date: Sun, 25 Feb 2024 23:10:18 +1100 Subject: [PATCH 5/5] Don't include threadID in thread safe implementation. --- .../include/CesiumUtility/ReferenceCounted.h | 36 ++++++++++++------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/CesiumUtility/include/CesiumUtility/ReferenceCounted.h b/CesiumUtility/include/CesiumUtility/ReferenceCounted.h index e245a7678..f11c40ae2 100644 --- a/CesiumUtility/include/CesiumUtility/ReferenceCounted.h +++ b/CesiumUtility/include/CesiumUtility/ReferenceCounted.h @@ -8,6 +8,20 @@ namespace CesiumUtility { +#ifndef NDEBUG +template class ThreadIdHolder; + +template <> class ThreadIdHolder { + ThreadIdHolder() : _threadID(std::this_thread::get_id()) {} + + std::thread::id _threadID; + + template friend class ReferenceCounted; +}; + +template <> class ThreadIdHolder {}; +#endif + /** * @brief A reference-counted base class, meant to be used with * {@link IntrusivePointer}. @@ -27,15 +41,14 @@ namespace CesiumUtility { * one thread at a time. However, this mode has a bit less overhead for objects * that are only ever accessed from a single thread. */ -template class ReferenceCounted { -public: - ReferenceCounted() noexcept +template +class ReferenceCounted #ifndef NDEBUG - : _threadID(std::this_thread::get_id()) + : public ThreadIdHolder #endif - { - } - +{ +public: + ReferenceCounted() noexcept {} ~ReferenceCounted() noexcept { assert(this->_referenceCount == 0); } /** @@ -65,6 +78,7 @@ template class ReferenceCounted { assert(std::this_thread::get_id() == this->_threadID); } #endif + assert(this->_referenceCount > 0); const int32_t references = --this->_referenceCount; if (references == 0) { @@ -86,10 +100,6 @@ template class ReferenceCounted { std::conditional_t; mutable CounterType _referenceCount{0}; - -#ifndef NDEBUG - std::thread::id _threadID; -#endif }; /** @@ -103,7 +113,7 @@ template class ReferenceCounted { * `class MyClass : public ReferenceCountedThreadSafe { ... };` */ template -using ReferenceCountedThreadSafe = typename ReferenceCounted; +using ReferenceCountedThreadSafe = ReferenceCounted; /** * @brief A reference-counted base class, meant to be used with @@ -116,6 +126,6 @@ using ReferenceCountedThreadSafe = typename ReferenceCounted; * `class MyClass : public ReferenceCountedNonThreadSafe { ... };` */ template -using ReferenceCountedNonThreadSafe = typename ReferenceCounted; +using ReferenceCountedNonThreadSafe = ReferenceCounted; } // namespace CesiumUtility