Add heap allocation to lib/support/Pool.h

#### Problem We have too many pool allocators. Previous PRs (project-chip#11428, project-chip#11487) transitionally use `BitMapObjectPool` where previously `System::ObjectPool` had been used, but this lost the ability to configure heap allocation. #### Change overview - Add a heap allocator (from project-chip#9590) - Add allocation selection (from project-chip#11371) - Use this for `System::Timer` (complementing project-chip#11487) Co-authored-by: Zang MingJie <[email protected]> Co-authored-by: C Freeman <[email protected]> A future PR will use this for Inet pools (complementing project-chip#11428); that is not done here because it would conflict with other Inet changes under way. #### Testing Added heap versions of unit tests in TestPool. (A future PR will add `System::Object`-style statistics and re-unify most of these tests.) CI should show `.bss` decreases corresponding to increases in project-chip#11487.
kpschoedel · Nov 16, 2021 · 6536d11 · 6536d11
1 parent 42df058
commit 6536d11
Show file tree

Hide file tree

Showing 4 changed files with 190 additions and 4 deletions.
diff --git a/src/lib/support/Pool.h b/src/lib/support/Pool.h
@@ -23,13 +23,20 @@
 
 #pragma once
 
+#include <lib/support/CodeUtils.h>
+
 #include <array>
 #include <assert.h>
 #include <atomic>
 #include <limits>
 #include <new>
 #include <stddef.h>
 
+#if CHIP_SYSTEM_CONFIG_POOL_USE_HEAP
+#include <mutex>
+#include <set>
+#endif // CHIP_SYSTEM_CONFIG_POOL_USE_HEAP
+
 namespace chip {
 
 class StaticAllocatorBase
@@ -160,4 +167,107 @@ class BitMapObjectPool : public StaticAllocatorBitmap
     } mData;
 };
 
+#if CHIP_SYSTEM_CONFIG_POOL_USE_HEAP
+
+/**
+ *  @brief
+ *      A class template used for allocating Object subclass objects from an ObjectArena<> template union.
+ *
+ *  @tparam     T   a class to be allocated from the arena.
+ */
+template <class T>
+class HeapObjectPool
+{
+public:
+    template <typename... Args>
+    T * CreateObject(Args &&... args)
+    {
+        std::lock_guard<std::mutex> lock(mutex);
+        T * object = new T(std::forward<Args>(args)...);
+        if (object == nullptr)
+        {
+            return nullptr;
+        }
+
+        mObjects.insert(object);
+        return object;
+    }
+
+    void ReleaseObject(T * object)
+    {
+        if (object == nullptr)
+            return;
+
+        std::lock_guard<std::mutex> lock(mutex);
+        auto iter = mObjects.find(object);
+        VerifyOrDie(iter != mObjects.end());
+        mObjects.erase(iter);
+        delete object;
+    }
+
+    template <typename... Args>
+    void ResetObject(T * element, Args &&... args)
+    {
+        element->~T();
+        new (element) T(std::forward<Args>(args)...);
+    }
+
+    /**
+     * @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 <typename Function>
+    bool ForEachActiveObject(Function && function)
+    {
+        std::lock_guard<std::mutex> lock(mutex);
+        // Create a new copy of original set, allowing add/remove elements while iterating in the same thread.
+        for (auto object : std::set<T *>(mObjects))
+        {
+            if (!function(object))
+                return false;
+        }
+        return true;
+    }
+
+private:
+    std::mutex mutex;
+    std::set<T *> mObjects;
+};
+
+#endif // CHIP_SYSTEM_CONFIG_POOL_USE_HEAP
+
+#if CHIP_SYSTEM_CONFIG_POOL_USE_HEAP
+template <typename T, unsigned int N>
+using ObjectPool = HeapObjectPool<T>;
+#else  // CHIP_SYSTEM_CONFIG_POOL_USE_HEAP
+template <typename T, unsigned int N>
+using ObjectPool = BitMapObjectPool<T, N>;
+#endif // CHIP_SYSTEM_CONFIG_POOL_USE_HEAP
+
+enum class ObjectPoolMem
+{
+    kStatic,
+#if CHIP_SYSTEM_CONFIG_POOL_USE_HEAP
+    kDynamic
+#endif // CHIP_SYSTEM_CONFIG_POOL_USE_HEAP
+};
+
+template <typename T, size_t N, ObjectPoolMem P>
+class MemTypeObjectPool;
+
+template <typename T, size_t N>
+class MemTypeObjectPool<T, N, ObjectPoolMem::kStatic> : public BitMapObjectPool<T, N>
+{
+};
+
+#if CHIP_SYSTEM_CONFIG_POOL_USE_HEAP
+template <typename T, size_t N>
+class MemTypeObjectPool<T, N, ObjectPoolMem::kDynamic> : public HeapObjectPool<T>
+{
+};
+#endif // CHIP_SYSTEM_CONFIG_POOL_USE_HEAP
+
 } // namespace chip
diff --git a/src/lib/support/tests/TestPool.cpp b/src/lib/support/tests/TestPool.cpp
@@ -129,6 +129,71 @@ void TestCreateReleaseStruct(nlTestSuite * inSuite, void * inContext)
     }
 }
 
+#if CHIP_SYSTEM_CONFIG_POOL_USE_HEAP
+
+void TestReleaseNullDynamic(nlTestSuite * inSuite, void * inContext)
+{
+    HeapObjectPool<uint32_t> pool;
+    pool.ReleaseObject(nullptr);
+    // Not crashing is good.
+}
+
+void TestCreateReleaseObjectDynamic(nlTestSuite * inSuite, void * inContext)
+{
+    constexpr const size_t size = 100;
+    HeapObjectPool<uint32_t> pool;
+    uint32_t * obj[size];
+    for (size_t i = 0; i < size; ++i)
+    {
+        obj[i] = pool.CreateObject();
+        NL_TEST_ASSERT(inSuite, obj[i] != nullptr);
+        for (size_t j = 0; j < i; ++j)
+        {
+            NL_TEST_ASSERT(inSuite, obj[i] != obj[j]);
+        }
+    }
+
+    pool.ReleaseObject(obj[55]);
+    obj[55] = pool.CreateObject();
+    NL_TEST_ASSERT(inSuite, obj[55] != nullptr);
+
+    for (size_t i = 0; i < size; ++i)
+    {
+        pool.ReleaseObject(obj[i]);
+    }
+}
+
+void TestCreateReleaseStructDynamic(nlTestSuite * inSuite, void * inContext)
+{
+    struct S
+    {
+        S(std::set<S *> & set) : mSet(set) { mSet.insert(this); }
+        ~S() { mSet.erase(this); }
+        std::set<S *> & mSet;
+    };
+
+    std::set<S *> objs1;
+
+    constexpr const size_t size = 100;
+    HeapObjectPool<S> pool;
+    S * objs2[size];
+    for (size_t i = 0; i < size; ++i)
+    {
+        objs2[i] = pool.CreateObject(objs1);
+        NL_TEST_ASSERT(inSuite, objs2[i] != nullptr);
+        for (size_t j = 0; j < i; ++j)
+        {
+            NL_TEST_ASSERT(inSuite, objs2[i] != objs2[j]);
+        }
+    }
+    for (size_t i = 0; i < size; ++i)
+    {
+        pool.ReleaseObject(objs2[i]);
+    }
+}
+
+#endif // CHIP_SYSTEM_CONFIG_POOL_USE_HEAP
+
 int Setup(void * inContext)
 {
     return SUCCESS;
@@ -145,8 +210,19 @@ int Teardown(void * inContext)
 /**
  *   Test Suite. It lists all the test functions.
  */
-static const nlTest sTests[] = { NL_TEST_DEF_FN(TestReleaseNull), NL_TEST_DEF_FN(TestCreateReleaseObject),
-                                 NL_TEST_DEF_FN(TestCreateReleaseStruct), NL_TEST_SENTINEL() };
+static const nlTest sTests[] = {
+    // clang-format off
+    NL_TEST_DEF_FN(TestReleaseNull),
+    NL_TEST_DEF_FN(TestCreateReleaseObject),
+    NL_TEST_DEF_FN(TestCreateReleaseStruct),
+#if CHIP_SYSTEM_CONFIG_POOL_USE_HEAP
+    NL_TEST_DEF_FN(TestReleaseNullDynamic),
+    NL_TEST_DEF_FN(TestCreateReleaseObjectDynamic),
+    NL_TEST_DEF_FN(TestCreateReleaseStructDynamic),
+#endif // CHIP_SYSTEM_CONFIG_POOL_USE_HEAP
+    NL_TEST_SENTINEL()
+    // clang-format on
+};
 
 int TestPool()
 {

diff --git a/src/system/SystemTimer.cpp b/src/system/SystemTimer.cpp
@@ -74,7 +74,7 @@ namespace System {
  *******************************************************************************
  */
 
-BitMapObjectPool<Timer, CHIP_SYSTEM_CONFIG_NUM_TIMERS> Timer::sPool;
+chip::ObjectPool<Timer, CHIP_SYSTEM_CONFIG_NUM_TIMERS> Timer::sPool;
 Stats::count_t Timer::mNumInUse      = 0;
 Stats::count_t Timer::mHighWatermark = 0;
 

diff --git a/src/system/SystemTimer.h b/src/system/SystemTimer.h
@@ -229,7 +229,7 @@ class DLL_EXPORT Timer
 
 private:
     friend class LayerImplLwIP;
-    static BitMapObjectPool<Timer, CHIP_SYSTEM_CONFIG_NUM_TIMERS> sPool;
+    static chip::ObjectPool<Timer, CHIP_SYSTEM_CONFIG_NUM_TIMERS> sPool;
     static Stats::count_t mNumInUse;
     static Stats::count_t mHighWatermark;