Skip to content

Commit

Permalink
ObjectPool: Use Platform::New and rename modes (#13088)
Browse files Browse the repository at this point in the history
#### Problem

- Heap allocations should use `Platform::New` rather than raw `new`.
- `ObjectPoolMem::kStatic` could be misleading, since the pool memory is
  part of the enclosing context, not static unless also declared `static`.

#### Change overview

- Use `Platform::New` and `Platform::Delete`.
- Rename `ObjectPoolMem::kStatic` → `kInline` and `kDynamic` → `kHeap`.

#### Testing

CI
  • Loading branch information
kpschoedel authored Dec 16, 2021
1 parent 59dd330 commit 7cca950
Show file tree
Hide file tree
Showing 7 changed files with 45 additions and 26 deletions.
2 changes: 1 addition & 1 deletion src/lib/dnssd/minimal_mdns/Server.h
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ class ServerBase

// The PoolImpl impl is used as a base class because its destructor must be called after ServerBase's destructor.
template <size_t kCount>
class Server : private chip::PoolImpl<ServerBase::EndpointInfo, kCount, chip::ObjectPoolMem::kStatic,
class Server : private chip::PoolImpl<ServerBase::EndpointInfo, kCount, chip::ObjectPoolMem::kInline,
ServerBase::EndpointInfoPoolType::Interface>,
public ServerBase
{
Expand Down
2 changes: 1 addition & 1 deletion src/lib/dnssd/minimal_mdns/tests/CheckOnlyServer.h
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ void MakePrintableName(char (&location)[N], FullQName name)

} // namespace

class CheckOnlyServer : private chip::PoolImpl<ServerBase::EndpointInfo, 0, chip::ObjectPoolMem::kStatic,
class CheckOnlyServer : private chip::PoolImpl<ServerBase::EndpointInfo, 0, chip::ObjectPoolMem::kInline,
ServerBase::EndpointInfoPoolType::Interface>,
public ServerBase,
public ParserDelegate,
Expand Down
2 changes: 1 addition & 1 deletion src/lib/support/Pool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ Loop HeapObjectList::ForEachNode(void * context, Lambda lambda)
if (p->mObject == nullptr)
{
p->Remove();
delete p;
Platform::Delete(p);
}
p = next;
}
Expand Down
30 changes: 21 additions & 9 deletions src/lib/support/Pool.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

#pragma once

#include <lib/support/CHIPMem.h>
#include <lib/support/CodeUtils.h>
#include <system/SystemConfig.h>

Expand Down Expand Up @@ -276,10 +277,10 @@ class HeapObjectPool : public internal::Statistics, public internal::PoolCommon<
template <typename... Args>
T * CreateObject(Args &&... args)
{
T * object = new T(std::forward<Args>(args)...);
T * object = Platform::New<T>(std::forward<Args>(args)...);
if (object != nullptr)
{
auto node = new internal::HeapObjectListNode();
auto node = Platform::New<internal::HeapObjectListNode>();
if (node != nullptr)
{
node->mObject = object;
Expand All @@ -300,7 +301,7 @@ class HeapObjectPool : public internal::Statistics, public internal::PoolCommon<
{
// Note that the node is not removed here; that is deferred until the end of the next pool iteration.
node->mObject = nullptr;
delete object;
Platform::Delete(object);
DecreaseUsage();
}
}
Expand Down Expand Up @@ -336,28 +337,39 @@ class HeapObjectPool : public internal::Statistics, public internal::PoolCommon<

#endif // CHIP_SYSTEM_CONFIG_POOL_USE_HEAP

/**
* Specify ObjectPool storage allocation.
*/
enum class ObjectPoolMem
{
kStatic,
/**
* Use storage inside the containing scope for both objects and pool management state.
*/
kInline,
#if CHIP_SYSTEM_CONFIG_POOL_USE_HEAP
kDynamic,
kDefault = kDynamic
/**
* Allocate objects from the heap, with only pool management state in the containing scope.
*
* For this case, the ObjectPool size parameter is ignored.
*/
kHeap,
kDefault = kHeap
#else // CHIP_SYSTEM_CONFIG_POOL_USE_HEAP
kDefault = kStatic
kDefault = kInline
#endif // CHIP_SYSTEM_CONFIG_POOL_USE_HEAP
};

template <typename T, size_t N, ObjectPoolMem P = ObjectPoolMem::kDefault>
class ObjectPool;

template <typename T, size_t N>
class ObjectPool<T, N, ObjectPoolMem::kStatic> : public BitMapObjectPool<T, N>
class ObjectPool<T, N, ObjectPoolMem::kInline> : public BitMapObjectPool<T, N>
{
};

#if CHIP_SYSTEM_CONFIG_POOL_USE_HEAP
template <typename T, size_t N>
class ObjectPool<T, N, ObjectPoolMem::kDynamic> : public HeapObjectPool<T>
class ObjectPool<T, N, ObjectPoolMem::kHeap> : public HeapObjectPool<T>
{
};
#endif // CHIP_SYSTEM_CONFIG_POOL_USE_HEAP
Expand Down
27 changes: 14 additions & 13 deletions src/lib/support/tests/TestPool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -62,20 +62,20 @@ void TestReleaseNull(nlTestSuite * inSuite, void * inContext)

void TestReleaseNullStatic(nlTestSuite * inSuite, void * inContext)
{
TestReleaseNull<uint32_t, 10, ObjectPoolMem::kStatic>(inSuite, inContext);
TestReleaseNull<uint32_t, 10, ObjectPoolMem::kInline>(inSuite, inContext);
}

#if CHIP_SYSTEM_CONFIG_POOL_USE_HEAP
void TestReleaseNullDynamic(nlTestSuite * inSuite, void * inContext)
{
TestReleaseNull<uint32_t, 10, ObjectPoolMem::kDynamic>(inSuite, inContext);
TestReleaseNull<uint32_t, 10, ObjectPoolMem::kHeap>(inSuite, inContext);
}
#endif // CHIP_SYSTEM_CONFIG_POOL_USE_HEAP

template <typename T, size_t N, ObjectPoolMem P>
void TestCreateReleaseObject(nlTestSuite * inSuite, void * inContext)
{
ObjectPool<uint32_t, N, ObjectPoolMem::kStatic> pool;
ObjectPool<uint32_t, N, ObjectPoolMem::kInline> pool;
uint32_t * obj[N];

NL_TEST_ASSERT(inSuite, pool.Allocated() == 0);
Expand Down Expand Up @@ -104,9 +104,9 @@ void TestCreateReleaseObject(nlTestSuite * inSuite, void * inContext)
void TestCreateReleaseObjectStatic(nlTestSuite * inSuite, void * inContext)
{
constexpr const size_t kSize = 100;
TestCreateReleaseObject<uint32_t, kSize, ObjectPoolMem::kStatic>(inSuite, inContext);
TestCreateReleaseObject<uint32_t, kSize, ObjectPoolMem::kInline>(inSuite, inContext);

ObjectPool<uint32_t, kSize, ObjectPoolMem::kStatic> pool;
ObjectPool<uint32_t, kSize, ObjectPoolMem::kInline> pool;
uint32_t * obj[kSize];

for (size_t i = 0; i < kSize; ++i)
Expand Down Expand Up @@ -144,7 +144,7 @@ void TestCreateReleaseObjectStatic(nlTestSuite * inSuite, void * inContext)
#if CHIP_SYSTEM_CONFIG_POOL_USE_HEAP
void TestCreateReleaseObjectDynamic(nlTestSuite * inSuite, void * inContext)
{
TestCreateReleaseObject<uint32_t, 100, ObjectPoolMem::kDynamic>(inSuite, inContext);
TestCreateReleaseObject<uint32_t, 100, ObjectPoolMem::kHeap>(inSuite, inContext);
}
#endif // CHIP_SYSTEM_CONFIG_POOL_USE_HEAP

Expand Down Expand Up @@ -201,13 +201,13 @@ void TestCreateReleaseStruct(nlTestSuite * inSuite, void * inContext)

void TestCreateReleaseStructStatic(nlTestSuite * inSuite, void * inContext)
{
TestCreateReleaseStruct<ObjectPoolMem::kStatic>(inSuite, inContext);
TestCreateReleaseStruct<ObjectPoolMem::kInline>(inSuite, inContext);
}

#if CHIP_SYSTEM_CONFIG_POOL_USE_HEAP
void TestCreateReleaseStructDynamic(nlTestSuite * inSuite, void * inContext)
{
TestCreateReleaseStruct<ObjectPoolMem::kDynamic>(inSuite, inContext);
TestCreateReleaseStruct<ObjectPoolMem::kHeap>(inSuite, inContext);
}
#endif // CHIP_SYSTEM_CONFIG_POOL_USE_HEAP

Expand Down Expand Up @@ -334,13 +334,13 @@ void TestForEachActiveObject(nlTestSuite * inSuite, void * inContext)

void TestForEachActiveObjectStatic(nlTestSuite * inSuite, void * inContext)
{
TestForEachActiveObject<ObjectPoolMem::kStatic>(inSuite, inContext);
TestForEachActiveObject<ObjectPoolMem::kInline>(inSuite, inContext);
}

#if CHIP_SYSTEM_CONFIG_POOL_USE_HEAP
void TestForEachActiveObjectDynamic(nlTestSuite * inSuite, void * inContext)
{
TestForEachActiveObject<ObjectPoolMem::kDynamic>(inSuite, inContext);
TestForEachActiveObject<ObjectPoolMem::kHeap>(inSuite, inContext);
}
#endif // CHIP_SYSTEM_CONFIG_POOL_USE_HEAP

Expand Down Expand Up @@ -397,23 +397,24 @@ void TestPoolInterface(nlTestSuite * inSuite, void * inContext)

void TestPoolInterfaceStatic(nlTestSuite * inSuite, void * inContext)
{
TestPoolInterface<ObjectPoolMem::kStatic>(inSuite, inContext);
TestPoolInterface<ObjectPoolMem::kInline>(inSuite, inContext);
}

#if CHIP_SYSTEM_CONFIG_POOL_USE_HEAP
void TestPoolInterfaceDynamic(nlTestSuite * inSuite, void * inContext)
{
TestPoolInterface<ObjectPoolMem::kDynamic>(inSuite, inContext);
TestPoolInterface<ObjectPoolMem::kHeap>(inSuite, inContext);
}
#endif // CHIP_SYSTEM_CONFIG_POOL_USE_HEAP

int Setup(void * inContext)
{
return SUCCESS;
return ::chip::Platform::MemoryInit() == CHIP_NO_ERROR ? SUCCESS : FAILURE;
}

int Teardown(void * inContext)
{
::chip::Platform::MemoryShutdown();
return SUCCESS;
}

Expand Down
6 changes: 6 additions & 0 deletions src/system/tests/TestSystemTimer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -442,6 +442,11 @@ static int TestSetup(void * aContext)
{
TestContext & lContext = *reinterpret_cast<TestContext *>(aContext);

if (::chip::Platform::MemoryInit() != CHIP_NO_ERROR)
{
return FAILURE;
}

#if CHIP_SYSTEM_CONFIG_USE_LWIP && LWIP_VERSION_MAJOR == 2 && LWIP_VERSION_MINOR == 0
static sys_mbox_t * sLwIPEventQueue = NULL;

Expand Down Expand Up @@ -471,6 +476,7 @@ static int TestTeardown(void * aContext)
tcpip_finish(NULL, NULL);
#endif // CHIP_SYSTEM_CONFIG_USE_LWIP && (LWIP_VERSION_MAJOR == 2) && (LWIP_VERSION_MINOR == 0)

::chip::Platform::MemoryShutdown();
return (SUCCESS);
}

Expand Down
2 changes: 1 addition & 1 deletion src/transport/raw/TCP.h
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ class TCP : public TCPBase
private:
friend class TCPTest;
TCPBase::ActiveConnectionState mConnectionsBuffer[kActiveConnectionsSize];
PoolImpl<PendingPacket, kPendingPacketSize, ObjectPoolMem::kStatic, PendingPacketPoolType::Interface> mPendingPackets;
PoolImpl<PendingPacket, kPendingPacketSize, ObjectPoolMem::kInline, PendingPacketPoolType::Interface> mPendingPackets;
};

} // namespace Transport
Expand Down

0 comments on commit 7cca950

Please sign in to comment.