From 5b2a4d84562f33e54f5bf354d509bfd414851938 Mon Sep 17 00:00:00 2001 From: C Freeman Date: Fri, 5 Nov 2021 19:59:02 -0400 Subject: [PATCH] Separate ObjectPool into static and dynamic parts (#11371) * Separate ObjectPool into static and dynamic parts The general goal of this refactor is to separate the static and dynamic parts of the ObjectPool so that developers that support heap allocation can also opt to use static allocation for portions of their code. This refactor maps ObjectPool to the same default types as current (dynamic if heap allocation is allowed, static otherwise). Upcoming refactor PRs: - adding unlocked iterator access so we can use range-based for loops (works more easily with the design of certain part of the SDK stack) - Demonstrating the use of the unsized abstract base type pointer for passing pools of unknown size * Apply suggestions from code review Co-authored-by: Michael Sandstedt * Fix size_t and remove base class. Previous size_t changes broke the build - just using size_t now for all stats. Also remove the base class. The idea with that was to allow this pool to be used with the unsigned base classes in the mdns code. However, it looks like it's causing memory bloat despite the fact that nothing actually uses that base class. So it looks like maybe we just need to refactor to mdns. Co-authored-by: Michael Sandstedt --- src/system/SystemObject.h | 367 +++++++++++++++++++++----------------- 1 file changed, 199 insertions(+), 168 deletions(-) diff --git a/src/system/SystemObject.h b/src/system/SystemObject.h index 6dfcb557009781..f6b26025ae9980 100644 --- a/src/system/SystemObject.h +++ b/src/system/SystemObject.h @@ -60,8 +60,10 @@ namespace System { class Layer; class LayerSockets; class LayerLwIP; -template -class ObjectPool; +template +class ObjectPoolStatic; +template +class ObjectPoolDynamic; /** * @class Object @@ -81,8 +83,10 @@ class ObjectPool; */ class DLL_EXPORT Object { - template - friend class ObjectPool; + template + friend class ObjectPoolStatic; + template + friend class ObjectPoolDynamic; public: Object() : mRefCount(0) @@ -194,21 +198,115 @@ union ObjectArena }; /** + * @class ObjectPoolStatic + * * @brief - * A class template used for allocating Object subclass objects from an ObjectArena<> template union. + * This is the data class for static object pools. * - * @tparam T a subclass of Object to be allocated from the arena. + * @tparam T a subclass of Object to be allocated. * @tparam N a positive integer number of objects of class T to allocate from the arena. */ -template -class ObjectPool +template +class ObjectPoolStatic { public: - void Reset(); + T * TryCreate() + { + T * lReturn = nullptr; + (void) static_cast(lReturn); /* In C++-11, this would be a static_assert that T inherits Object. */ + + size_t lIndex = 0; + for (lIndex = 0; lIndex < N; ++lIndex) + { + T & lObject = reinterpret_cast(mArena.uMemory)[lIndex]; + + if (lObject.TryCreate(sizeof(T))) + { + lReturn = &lObject; + break; + } + } +#if CHIP_SYSTEM_CONFIG_PROVIDE_STATISTICS + size_t lNumInUse = 0; + + if (lReturn != nullptr) + { + lIndex++; + lNumInUse = lIndex; + GetNumObjectsInUse(lIndex, lNumInUse); + } + else + { + lNumInUse = N; + } + + UpdateHighWatermark(lNumInUse); +#endif + return lReturn; + } +#if CHIP_SYSTEM_CONFIG_PROVIDE_STATISTICS + /** + * Return the number of objects in use starting at a given index + * + * @param[in] aStartIndex The index to start counting from; pass 0 to count over + * the whole pool. + * @param[in/out] aNumInUse The number of objects in use. If aStartIndex is not 0, + * the function adds to the counter without resetting it first. + */ + void GetNumObjectsInUse(size_t aStartIndex, size_t & aNumInUse) + { + size_t count = 0; + + for (size_t lIndex = aStartIndex; lIndex < N; ++lIndex) + { + T & lObject = reinterpret_cast(mArena.uMemory)[lIndex]; + + if (lObject.IsRetained()) + { + count++; + } + } + + if (aStartIndex == 0) + { + aNumInUse = 0; + } + + aNumInUse += count; + } + void UpdateHighWatermark(const size_t & aCandidate) + { + size_t lTmp; + while (aCandidate > (lTmp = mHighWatermark)) + { + SYSTEM_OBJECT_HWM_TEST_HOOK(); + (void) __sync_bool_compare_and_swap(&mHighWatermark, lTmp, aCandidate); + } + } + volatile size_t mHighWatermark; +#endif + + void GetStatistics(chip::System::Stats::count_t & aNumInUse, chip::System::Stats::count_t & aHighWatermark) + { +#if CHIP_SYSTEM_CONFIG_PROVIDE_STATISTICS + size_t lNumInUse; + size_t lHighWatermark; - T * TryCreate(); - void GetStatistics(chip::System::Stats::count_t & aNumInUse, chip::System::Stats::count_t & aHighWatermark); + GetNumObjectsInUse(0, lNumInUse); + lHighWatermark = mHighWatermark; + if (lNumInUse > CHIP_SYS_STATS_COUNT_MAX) + { + lNumInUse = CHIP_SYS_STATS_COUNT_MAX; + } + if (lHighWatermark > CHIP_SYS_STATS_COUNT_MAX) + { + lHighWatermark = CHIP_SYS_STATS_COUNT_MAX; + } + aNumInUse = static_cast(lNumInUse); + aHighWatermark = static_cast(lHighWatermark); +#endif + } /** * @brief * Run a functor for each active object in the pool @@ -219,19 +317,7 @@ class ObjectPool template bool ForEachActiveObject(Function && function) { -#if CHIP_SYSTEM_CONFIG_POOL_USE_HEAP - std::lock_guard lock(mMutex); - Object * p = mDummyHead.mNext; - while (p) - { - if (!function(static_cast(p))) - { - return false; - } - p = p->mNext; - } -#else - for (unsigned int i = 0; i < N; ++i) + for (size_t i = 0; i < N; ++i) { T & lObject = reinterpret_cast(mArena.uMemory)[i]; @@ -241,185 +327,130 @@ class ObjectPool return false; } } -#endif return true; } - -private: - friend class TestObject; - -#if CHIP_SYSTEM_CONFIG_POOL_USE_HEAP - std::mutex mMutex; - Object mDummyHead; -#else - ObjectArena mArena; - + void Reset() + { + memset(mArena.uMemory, 0, N * sizeof(T)); #if CHIP_SYSTEM_CONFIG_PROVIDE_STATISTICS - void GetNumObjectsInUse(unsigned int aStartIndex, unsigned int & aNumInUse); - void UpdateHighWatermark(const unsigned int & aCandidate); - volatile unsigned int mHighWatermark; -#endif + mHighWatermark = 0; #endif + } + ObjectArena mArena; }; -template -inline void ObjectPool::Reset() -{ #if CHIP_SYSTEM_CONFIG_POOL_USE_HEAP - std::lock_guard lock(mMutex); - Object * p = mDummyHead.mNext; - - while (p) - { - Object * del = p; - p = p->mNext; - delete del; - } - - mDummyHead.mNext = nullptr; -#else - memset(mArena.uMemory, 0, N * sizeof(T)); - -#if CHIP_SYSTEM_CONFIG_PROVIDE_STATISTICS - mHighWatermark = 0; -#endif -#endif -} - /** + * @class ObjectPoolDynamic + * * @brief - * Tries to initially retain the first object in the pool that is not retained. + * This is the data class for dynamic object pools. + * + * @tparam T a subclass of Object to be allocated. */ -template -inline T * ObjectPool::TryCreate() +template +class ObjectPoolDynamic { - T * lReturn = nullptr; +public: + T * TryCreate() + { - (void) static_cast(lReturn); /* In C++-11, this would be a static_assert that T inherits Object. */ + T * newNode = new T(); -#if CHIP_SYSTEM_CONFIG_POOL_USE_HEAP - T * newNode = new T(); + if (newNode->TryCreate(sizeof(T))) + { + std::lock_guard lock(mMutex); + Object * p = &mDummyHead; + if (p->mNext) + { + p->mNext->mPrev = newNode; + } + newNode->mNext = p->mNext; + p->mNext = newNode; + newNode->mPrev = p; + newNode->mMutexRef = &mMutex; + return newNode; + } + else + { + delete newNode; + return nullptr; + } + } - if (newNode->TryCreate(sizeof(T))) + /** + * @brief + * Run a functor for each active object in the pool + * + * @param function The functor of type `bool (*)(T*)`, return false to break the iteration + * @return bool Returns false if broke during iteration + */ + template + bool ForEachActiveObject(Function && function) { std::lock_guard lock(mMutex); - Object * p = &mDummyHead; - if (p->mNext) + Object * p = mDummyHead.mNext; + while (p) { - p->mNext->mPrev = newNode; + if (!function(static_cast(p))) + { + return false; + } + p = p->mNext; } - newNode->mNext = p->mNext; - p->mNext = newNode; - newNode->mPrev = p; - newNode->mMutexRef = &mMutex; - lReturn = newNode; - } - else - { - delete newNode; + return true; } -#else // CHIP_SYSTEM_CONFIG_POOL_USE_HEAP - unsigned int lIndex = 0; - - for (lIndex = 0; lIndex < N; ++lIndex) + void Reset() { - T & lObject = reinterpret_cast(mArena.uMemory)[lIndex]; + std::lock_guard lock(mMutex); + Object * p = mDummyHead.mNext; - if (lObject.TryCreate(sizeof(T))) + while (p) { - lReturn = &lObject; - break; + Object * del = p; + p = p->mNext; + delete del; } - } -#if CHIP_SYSTEM_CONFIG_PROVIDE_STATISTICS - unsigned int lNumInUse = 0; - - if (lReturn != nullptr) - { - lIndex++; - lNumInUse = lIndex; - GetNumObjectsInUse(lIndex, lNumInUse); + mDummyHead.mNext = nullptr; } - else - { - lNumInUse = N; - } - - UpdateHighWatermark(lNumInUse); -#endif + void GetStatistics(chip::System::Stats::count_t & aNumInUse, chip::System::Stats::count_t & aHighWatermark) {} + std::mutex mMutex; + Object mDummyHead; + friend class TestObject; +}; #endif // CHIP_SYSTEM_CONFIG_POOL_USE_HEAP - return lReturn; -} +#if CHIP_SYSTEM_CONFIG_POOL_USE_HEAP +template +using ObjectPool = ObjectPoolDynamic; +#else +template +using ObjectPool = ObjectPoolStatic; +#endif -#if CHIP_SYSTEM_CONFIG_PROVIDE_STATISTICS && !CHIP_SYSTEM_CONFIG_POOL_USE_HEAP -template -inline void ObjectPool::UpdateHighWatermark(const unsigned int & aCandidate) +enum class ObjectPoolMem { - unsigned int lTmp; + kStatic, +#if CHIP_SYSTEM_CONFIG_POOL_USE_HEAP + kDynamic +#endif +}; - while (aCandidate > (lTmp = mHighWatermark)) - { - SYSTEM_OBJECT_HWM_TEST_HOOK(); - (void) __sync_bool_compare_and_swap(&mHighWatermark, lTmp, aCandidate); - } -} +template +class MemTypeObjectPool; -/** - * Return the number of objects in use starting at a given index - * - * @param[in] aStartIndex The index to start counting from; pass 0 to count over - * the whole pool. - * @param[in/out] aNumInUse The number of objects in use. If aStartIndex is not 0, - * the function adds to the counter without resetting it first. - */ -template -inline void ObjectPool::GetNumObjectsInUse(unsigned int aStartIndex, unsigned int & aNumInUse) +template +class MemTypeObjectPool : public ObjectPoolStatic { - unsigned int count = 0; - - for (unsigned int lIndex = aStartIndex; lIndex < N; ++lIndex) - { - T & lObject = reinterpret_cast(mArena.uMemory)[lIndex]; - - if (lObject.IsRetained()) - { - count++; - } - } - - if (aStartIndex == 0) - { - aNumInUse = 0; - } - - aNumInUse += count; -} -#endif // CHIP_SYSTEM_CONFIG_PROVIDE_STATISTICS && !CHIP_SYSTEM_CONFIG_POOL_USE_HEAP +}; -template -inline void ObjectPool::GetStatistics(chip::System::Stats::count_t & aNumInUse, chip::System::Stats::count_t & aHighWatermark) +#if CHIP_SYSTEM_CONFIG_POOL_USE_HEAP +template +class MemTypeObjectPool : public ObjectPoolDynamic { -#if CHIP_SYSTEM_CONFIG_PROVIDE_STATISTICS && !CHIP_SYSTEM_CONFIG_POOL_USE_HEAP - unsigned int lNumInUse; - unsigned int lHighWatermark; - - GetNumObjectsInUse(0, lNumInUse); - lHighWatermark = mHighWatermark; - - if (lNumInUse > CHIP_SYS_STATS_COUNT_MAX) - { - lNumInUse = CHIP_SYS_STATS_COUNT_MAX; - } - if (lHighWatermark > CHIP_SYS_STATS_COUNT_MAX) - { - lHighWatermark = CHIP_SYS_STATS_COUNT_MAX; - } - aNumInUse = static_cast(lNumInUse); - aHighWatermark = static_cast(lHighWatermark); +}; #endif -} } // namespace System } // namespace chip