Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support to use heap for pool for large systems #7871

Closed
wants to merge 1 commit into from
+122 −18
Closed

Support to use heap for pool for large systems #7871

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 25 additions & 7 deletions src/inet/InetLayer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -281,14 +281,26 @@ INET_ERROR InetLayer::Init(chip::System::Layer & aSystemLayer, void * aContext)

State = kState_Initialized;

#if CHIP_SYSTEM_CONFIG_USE_SOCKETS
#if INET_CONFIG_ENABLE_DNS_RESOLVER && INET_CONFIG_ENABLE_ASYNC_DNS_SOCKETS
#if INET_CONFIG_ENABLE_RAW_ENDPOINT
RawEndPoint::sPool.Init();
#endif

#if INET_CONFIG_ENABLE_UDP_ENDPOINT
UDPEndPoint::sPool.Init();
#endif

#if INET_CONFIG_ENABLE_TCP_ENDPOINT
TCPEndPoint::sPool.Init();
#endif

#if INET_CONFIG_ENABLE_DNS_RESOLVER
DNSResolver::sPool.Init();

#if CHIP_SYSTEM_CONFIG_USE_SOCKETS && INET_CONFIG_ENABLE_ASYNC_DNS_SOCKETS
err = mAsyncDNSResolver.Init(this);
SuccessOrExit(err);

#endif // INET_CONFIG_ENABLE_DNS_RESOLVER && INET_CONFIG_ENABLE_ASYNC_DNS_SOCKETS
#endif // CHIP_SYSTEM_CONFIG_USE_SOCKETS
#endif // CHIP_SYSTEM_CONFIG_USE_SOCKETS && INET_CONFIG_ENABLE_ASYNC_DNS_SOCKETS
#endif // INET_CONFIG_ENABLE_DNS_RESOLVER

exit:
Platform::InetLayer::DidInit(this, mContext, err);
Expand Down Expand Up @@ -331,10 +343,10 @@ INET_ERROR InetLayer::Shutdown()
}

#if CHIP_SYSTEM_CONFIG_USE_SOCKETS && INET_CONFIG_ENABLE_ASYNC_DNS_SOCKETS

err = mAsyncDNSResolver.Shutdown();

#endif // CHIP_SYSTEM_CONFIG_USE_SOCKETS && INET_CONFIG_ENABLE_ASYNC_DNS_SOCKETS

DNSResolver::sPool.Shutdown();
#endif // INET_CONFIG_ENABLE_DNS_RESOLVER

#if INET_CONFIG_ENABLE_RAW_ENDPOINT
Expand All @@ -347,6 +359,8 @@ INET_ERROR InetLayer::Shutdown()
lEndPoint->Close();
}
}

RawEndPoint::sPool.Shutdown();
#endif // INET_CONFIG_ENABLE_RAW_ENDPOINT

#if INET_CONFIG_ENABLE_TCP_ENDPOINT
Expand All @@ -359,6 +373,8 @@ INET_ERROR InetLayer::Shutdown()
lEndPoint->Abort();
}
}

TCPEndPoint::sPool.Shutdown();
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT

#if INET_CONFIG_ENABLE_UDP_ENDPOINT
Expand All @@ -371,6 +387,8 @@ INET_ERROR InetLayer::Shutdown()
lEndPoint->Close();
}
}

UDPEndPoint::sPool.Shutdown();
#endif // INET_CONFIG_ENABLE_UDP_ENDPOINT
}

Expand Down
2 changes: 1 addition & 1 deletion src/platform/Linux/SystemPlatformConfig.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ struct ChipDeviceEvent;
#define CHIP_SYSTEM_CONFIG_NO_LOCKING 0
#define CHIP_SYSTEM_CONFIG_PLATFORM_PROVIDES_EVENT_FUNCTIONS 1
#define CHIP_SYSTEM_CONFIG_PLATFORM_PROVIDES_TIME 0

#define CHIP_SYSTEM_CONFIG_USE_POSIX_TIME_FUNCTS 1
#define CHIP_SYSTEM_CONFIG_POOL_USE_HEAP 1

// ========== Platform-specific Configuration Overrides =========

Expand Down
10 changes: 10 additions & 0 deletions src/system/SystemConfig.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,16 @@
#define CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING 0
#endif /* CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING */

/**
* @def CHIP_SYSTEM_CONFIG_POOL_USE_HEAP
*
* @brief
* Allocate Pool from Heap for large systems (e.g. Linux).
*/
#ifndef CHIP_SYSTEM_CONFIG_POOL_USE_HEAP
#define CHIP_SYSTEM_CONFIG_POOL_USE_HEAP 0
#endif /* CHIP_SYSTEM_CONFIG_POOL_USE_HEAP */

/**
* @def CHIP_SYSTEM_CONFIG_NO_LOCKING
*
Expand Down
3 changes: 3 additions & 0 deletions src/system/SystemLayer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,8 @@ Error Layer::Init(void * aContext)
{
Error lReturn;

Timer::sPool.Init();

RegisterLayerErrorFormatter();
#if CHIP_SYSTEM_CONFIG_USE_SOCKETS
RegisterPOSIXErrorFormatter();
Expand Down Expand Up @@ -198,6 +200,7 @@ Error Layer::Shutdown()
this->mContext = nullptr;
this->mLayerState = kLayerState_NotInitialized;

Timer::sPool.Shutdown();
exit:
Platform::Layer::DidShutdown(*this, lContext, lReturn);
return lReturn;
Expand Down
1 change: 0 additions & 1 deletion src/system/SystemObject.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
// Include local headers
#include <stddef.h>
#include <stdlib.h>
#include <string.h>

namespace chip {
namespace System {
Expand Down
65 changes: 65 additions & 0 deletions src/system/SystemObject.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,10 @@
// Include dependent headers
#include <stddef.h>
#include <stdint.h>
#include <string.h>
#include <unistd.h>

#include <support/CHIPMem.h>
#include <support/DLLUtil.h>

#include <system/SystemError.h>
Expand Down Expand Up @@ -160,7 +162,11 @@ inline Object::~Object() {}
template <typename ALIGN, size_t SIZE>
union ObjectArena
{
#if CHIP_SYSTEM_CONFIG_POOL_USE_HEAP
uint8_t * uMemory;
#else
uint8_t uMemory[SIZE];
#endif
ALIGN uAlign;
};

Expand All @@ -175,6 +181,9 @@ template <class T, unsigned int N>
class ObjectPool
{
public:
void Init();
void Reset();
void Shutdown();
static size_t Size();

T * Get(const Layer & aLayer, size_t aIndex);
Expand All @@ -185,6 +194,9 @@ class ObjectPool
friend class TestObject;

ObjectArena<void *, N * sizeof(T)> mArena;
#if CHIP_SYSTEM_CONFIG_POOL_USE_HEAP
unsigned int mRefCount = 0;
#endif

#if CHIP_SYSTEM_CONFIG_PROVIDE_STATISTICS
void GetNumObjectsInUse(unsigned int aStartIndex, unsigned int & aNumInUse);
Expand All @@ -193,6 +205,59 @@ class ObjectPool
#endif
};

template <class T, unsigned int N>
inline void ObjectPool<T, N>::Init()
{
#if CHIP_SYSTEM_CONFIG_POOL_USE_HEAP
unsigned int oldCount = __sync_fetch_and_add(&this->mRefCount, 1);

if (oldCount == 0)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see how the atomic operations ensure correctness for mRefCount. However, how do we ensure that the pool is actually initialized after the second call to Init returns? I don't think we actually can.

It seems like this is unsafe unless all of the pools are carefully initialized at the start of the program before anything actually uses them. That seems to somewhat defeat the purpose of using atomics for mRefCount, or having the reference counting scheme at all.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that we're moving to purely external synchronization, I would just leave it out unless there's an actual race (and I don't see how there can be; I think we'll have bigger problems if somebody tries to initialize the sdk twice), but if necessary, prefer std::atomic to gcc instrinsics.

{
mArena.uMemory = reinterpret_cast<uint8_t *>(chip::Platform::MemoryAlloc(N * sizeof(T)));
memset(mArena.uMemory, 0, N * sizeof(T));
#if CHIP_SYSTEM_CONFIG_PROVIDE_STATISTICS
mHighWatermark = 0;
#endif
}
#endif
}

template <class T, unsigned int N>
inline void ObjectPool<T, N>::Shutdown()
{
#if CHIP_SYSTEM_CONFIG_POOL_USE_HEAP
unsigned int oldCount = __sync_fetch_and_sub(&this->mRefCount, 1);

if (oldCount == 1)
{
if (mArena.uMemory)
{
chip::Platform::MemoryFree(mArena.uMemory);
mArena.uMemory = nullptr;
}

__sync_synchronize();
}
else if (oldCount == 0)
{
abort();
}
#endif

#if CHIP_SYSTEM_CONFIG_PROVIDE_STATISTICS
mHighWatermark = 0;
#endif
}

template <class T, unsigned int N>
inline void ObjectPool<T, N>::Reset()
{
memset(mArena.uMemory, 0, N * sizeof(T));
#if CHIP_SYSTEM_CONFIG_PROVIDE_STATISTICS
mHighWatermark = 0;
#endif
}

/**
* @brief
* Returns the number of objects that can be simultaneously retained from a pool.
Expand Down
27 changes: 18 additions & 9 deletions src/system/tests/TestSystemObject.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,13 @@ static int Finalize(void * aContext);
class TestObject : public Object
{
public:
enum
{
kPoolSize = 122 // a multiple of kNumThreads, less than CHIP_SYS_STATS_COUNT_MAX
};

static ObjectPool<TestObject, kPoolSize> sPool;

Error Init();

static void CheckRetention(nlTestSuite * inSuite, void * aContext);
Expand All @@ -78,12 +85,6 @@ class TestObject : public Object
static void CheckHighWatermarkConcurrency(nlTestSuite * inSuite, void * aContext);

private:
enum
{
kPoolSize = 122 // a multiple of kNumThreads, less than CHIP_SYS_STATS_COUNT_MAX
};
static ObjectPool<TestObject, kPoolSize> sPool;

#if CHIP_SYSTEM_CONFIG_POSIX_LOCKING
unsigned int mDelay;

Expand Down Expand Up @@ -137,7 +138,7 @@ void TestObject::CheckRetention(nlTestSuite * inSuite, void * aContext)
unsigned int i, j;

lLayer.Init(lContext.mLayerContext);
memset(&sPool, 0, sizeof(sPool));
sPool.Reset();

for (i = 0; i < kPoolSize; ++i)
{
Expand Down Expand Up @@ -332,7 +333,7 @@ void TestObject::MultithreadedTest(nlTestSuite * inSuite, void * aContext, void
TestContext & lContext = *static_cast<TestContext *>(aContext);
pthread_t lThread[kNumThreads];

memset(&sPool, 0, sizeof(sPool));
sPool.Reset();

for (unsigned int i = 0; i < kNumThreads; ++i)
{
Expand Down Expand Up @@ -369,7 +370,7 @@ void TestObject::CheckHighWatermarkConcurrency(nlTestSuite * inSuite, void * aCo

void TestObject::CheckHighWatermark(nlTestSuite * inSuite, void * aContext)
{
memset(&sPool, 0, sizeof(sPool));
sPool.Reset();

const int kNumObjects = kPoolSize;
TestObject * lObject = nullptr;
Expand Down Expand Up @@ -488,6 +489,11 @@ static int Initialize(void * aContext)
TestContext & lContext = *reinterpret_cast<TestContext *>(aContext);
void * lLayerContext = nullptr;

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

TestObject::sPool.Init();

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

Expand Down Expand Up @@ -515,6 +521,9 @@ static int Finalize(void * aContext)

lContext.mTestSuite = nullptr;

TestObject::sPool.Shutdown();
chip::Platform::MemoryShutdown();

return SUCCESS;
}

Expand Down