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.

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

Streamline ObjectPool in System::Timer #12628

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 3 additions & 2 deletions src/credentials/CertificationDeclaration.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,9 @@
* working with Certification Declaration elements.
*/

#include <inttypes.h>
#include <stddef.h>
#include <algorithm>
#include <cinttypes>
#include <cstddef>

#include <credentials/CertificationDeclaration.h>
#include <crypto/CHIPCryptoPAL.h>
Expand Down
3 changes: 3 additions & 0 deletions src/inet/InetLayer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,10 @@ CHIP_ERROR InetLayer::Shutdown()
{
VerifyOrReturnError(mLayerState.SetShuttingDown(), CHIP_ERROR_INCORRECT_STATE);
VerifyOrReturnError(mTCPEndPointManager == nullptr, CHIP_ERROR_INCORRECT_STATE);
VerifyOrReturnError(mUDPEndPointManager != nullptr, CHIP_ERROR_INCORRECT_STATE);
ReturnErrorOnFailure(mUDPEndPointManager->Shutdown());
mUDPEndPointManager = nullptr;
mSystemLayer = nullptr;
mLayerState.ResetFromShuttingDown(); // Return to uninitialized state to permit re-initialization.
return CHIP_NO_ERROR;
}
Expand Down
1 change: 1 addition & 0 deletions src/inet/InetLayer.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#pragma once

#include <lib/support/ObjectLifeCycle.h>
#include <lib/support/Pool.h>
#include <platform/LockTracker.h>
#include <system/SystemLayer.h>
#include <system/SystemStats.h>
Expand Down
28 changes: 15 additions & 13 deletions src/inet/tests/TestInetEndPoint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -319,35 +319,34 @@ static void TestInetEndPointLimit(nlTestSuite * inSuite, void * inContext)

CHIP_ERROR err = CHIP_NO_ERROR;

// TODO: err is not validated EXCEPT the last call
for (int i = 0; i < INET_CONFIG_NUM_UDP_ENDPOINTS + 1; i++)
for (int i = INET_CONFIG_NUM_UDP_ENDPOINTS; i >= 0; --i)
{
err = gInet.GetUDPEndPointManager()->NewEndPoint(&testUDPEP[i]);
NL_TEST_ASSERT(inSuite, err == CHIP_ERROR_ENDPOINT_POOL_FULL);
NL_TEST_ASSERT(inSuite, err == (i ? CHIP_NO_ERROR : CHIP_ERROR_ENDPOINT_POOL_FULL));
}

// TODO: err is not validated EXCEPT the last call
for (int i = 0; i < INET_CONFIG_NUM_TCP_ENDPOINTS + 1; i++)
for (int i = INET_CONFIG_NUM_TCP_ENDPOINTS; i >= 0; --i)
{
err = gInet.GetTCPEndPointManager()->NewEndPoint(&testTCPEP[i]);
NL_TEST_ASSERT(inSuite, err == CHIP_ERROR_ENDPOINT_POOL_FULL);
NL_TEST_ASSERT(inSuite, err == (i ? CHIP_NO_ERROR : CHIP_ERROR_ENDPOINT_POOL_FULL));
}

#if CHIP_SYSTEM_CONFIG_NUM_TIMERS
// Verify same aComplete and aAppState args do not exhaust timer pool
for (int i = 0; i < CHIP_SYSTEM_CONFIG_NUM_TIMERS + 1; i++)
{
err = gSystemLayer.StartTimer(10_ms32, HandleTimer, nullptr);
NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);
}

#if CHIP_SYSTEM_CONFIG_USE_TIMER_POOL
char numTimersTest[CHIP_SYSTEM_CONFIG_NUM_TIMERS + 1];
for (int i = 0; i < CHIP_SYSTEM_CONFIG_NUM_TIMERS + 1; i++)
err = gSystemLayer.StartTimer(10_ms32, HandleTimer, &numTimersTest[i]);
NL_TEST_ASSERT(inSuite, err == CHIP_ERROR_NO_MEMORY);
#endif // CHIP_SYSTEM_CONFIG_USE_TIMER_POOL

ShutdownNetwork();
ShutdownSystemLayer();
#endif // CHIP_SYSTEM_CONFIG_NUM_TIMERS

// Release UDP endpoints
for (int i = 0; i < INET_CONFIG_NUM_UDP_ENDPOINTS; i++)
for (int i = 0; i <= INET_CONFIG_NUM_UDP_ENDPOINTS; i++)
{
if (testUDPEP[i] != nullptr)
{
Expand All @@ -356,13 +355,16 @@ static void TestInetEndPointLimit(nlTestSuite * inSuite, void * inContext)
}

// Release TCP endpoints
for (int i = 0; i < INET_CONFIG_NUM_TCP_ENDPOINTS; i++)
for (int i = 0; i <= INET_CONFIG_NUM_TCP_ENDPOINTS; i++)
{
if (testTCPEP[i] != nullptr)
{
testTCPEP[i]->Free();
}
}

ShutdownNetwork();
ShutdownSystemLayer();
}
#endif

Expand Down
1 change: 1 addition & 0 deletions src/lib/support/TestPersistentStorageDelegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include <lib/core/CHIPPersistentStorageDelegate.h>
#include <lib/support/DLLUtil.h>
#include <map>
#include <string>
#include <vector>

namespace chip {
Expand Down
1 change: 1 addition & 0 deletions src/lib/support/Variant.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

#include <algorithm>
#include <new>
#include <tuple>
#include <type_traits>
#include <typeinfo>
#include <utility>
Expand Down
1 change: 1 addition & 0 deletions src/messaging/ExchangeMessageDispatch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#define __STDC_LIMIT_MACROS
#endif

#include <errno.h>
#include <inttypes.h>
#include <memory>

Expand Down
2 changes: 2 additions & 0 deletions src/platform/Linux/ConnectivityManagerImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@
#include <platform/Linux/dbus/wpa/DBusWpaBss.h>
#include <platform/Linux/dbus/wpa/DBusWpaInterface.h>
#include <platform/Linux/dbus/wpa/DBusWpaNetwork.h>

#include <mutex>
#endif

namespace chip {
Expand Down
14 changes: 0 additions & 14 deletions src/system/SystemConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -439,20 +439,6 @@ struct LwIPEvent;
#define CHIP_SYSTEM_CONFIG_NUM_TIMERS 32
#endif /* CHIP_SYSTEM_CONFIG_NUM_TIMERS */

/**
* @def CHIP_SYSTEM_CONFIG_USE_TIMER_POOL
*
* @brief
* This defines whether (1) or not (0) the implementation uses the System::Timer pool.
*/
#ifndef CHIP_SYSTEM_CONFIG_USE_TIMER_POOL
#if CHIP_SYSTEM_CONFIG_NUM_TIMERS > 0
#define CHIP_SYSTEM_CONFIG_USE_TIMER_POOL 1
#else
#define CHIP_SYSTEM_CONFIG_USE_TIMER_POOL 0
#endif
#endif /* CHIP_SYSTEM_CONFIG_USE_TIMER_POOL */

/**
* @def CHIP_SYSTEM_CONFIG_PROVIDE_STATISTICS
*
Expand Down
3 changes: 2 additions & 1 deletion src/system/SystemLayer.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
#include <system/SystemClock.h>
#include <system/SystemError.h>
#include <system/SystemEvent.h>
#include <system/SystemTimer.h>

#if CHIP_SYSTEM_CONFIG_USE_SOCKETS
#include <system/SocketEvents.h>
Expand All @@ -47,6 +46,8 @@
#include <dispatch/dispatch.h>
#endif // CHIP_SYSTEM_CONFIG_USE_DISPATCH

#include <utility>

namespace chip {
namespace System {

Expand Down
38 changes: 13 additions & 25 deletions src/system/SystemLayerImplLwIP.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,6 @@ CHIP_ERROR LayerImplLwIP::Init()

RegisterLwIPErrorFormatter();

ReturnErrorOnFailure(mTimerList.Init());

VerifyOrReturnError(mLayerState.SetInitialized(), CHIP_ERROR_INCORRECT_STATE);
return CHIP_NO_ERROR;
}
Expand All @@ -58,7 +56,7 @@ CHIP_ERROR LayerImplLwIP::StartTimer(Clock::Timeout delay, TimerCompleteCallback

CancelTimer(onComplete, appState);

Timer * timer = Timer::New(*this, delay, onComplete, appState);
TimerList::Node * timer = mTimerPool.Create(*this, delay, onComplete, appState);
VerifyOrReturnError(timer != nullptr, CHIP_ERROR_NO_MEMORY);

if (mTimerList.Add(timer) == timer)
Expand All @@ -78,34 +76,25 @@ void LayerImplLwIP::CancelTimer(TimerCompleteCallback onComplete, void * appStat
{
VerifyOrReturn(mLayerState.IsInitialized());

Timer * timer = mTimerList.Remove(onComplete, appState);
VerifyOrReturn(timer != nullptr);

timer->Clear();
timer->Release();
TimerList::Node * timer = mTimerList.Remove(onComplete, appState);
if (timer != nullptr)
{
mTimerPool.Release(timer);
}
}

CHIP_ERROR LayerImplLwIP::ScheduleWork(TimerCompleteCallback onComplete, void * appState)
{
VerifyOrReturnError(mLayerState.IsInitialized(), CHIP_ERROR_INCORRECT_STATE);

Timer * timer = Timer::New(*this, System::Clock::kZero, onComplete, appState);
TimerList::Node * timer = mTimerPool.Create(*this, System::Clock::kZero, onComplete, appState);
VerifyOrReturnError(timer != nullptr, CHIP_ERROR_NO_MEMORY);

return ScheduleLambda([timer] { timer->HandleComplete(); });
return ScheduleLambda([this, timer] { this->mTimerPool.Invoke(timer); });
}

/**
* Start the platform timer with specified millsecond duration.
*
* @brief
* Calls the Platform specific API to start a platform timer. This API is called by the chip::System::Timer class when
* one or more timers are active and require deferred execution.
*
* @param[in] aDelay The timer duration in milliseconds.
*
* @return CHIP_NO_ERROR on success, error code otherwise.
*
*/
CHIP_ERROR LayerImplLwIP::StartPlatformTimer(System::Clock::Timeout aDelay)
{
Expand Down Expand Up @@ -142,14 +131,13 @@ CHIP_ERROR LayerImplLwIP::HandlePlatformTimer()
// limit the number of timers handled before the control is returned to the event queue. The bound is similar to
// (though not exactly same) as that on the sockets-based systems.

size_t timersHandled = 0;
Timer * timer = nullptr;
size_t timersHandled = 0;
TimerList::Node * timer = nullptr;
while ((timersHandled < CHIP_SYSTEM_CONFIG_NUM_TIMERS) && ((timer = mTimerList.PopIfEarlier(expirationTime)) != nullptr))
{
mHandlingTimerComplete = true;
timer->HandleComplete();
mTimerPool.Invoke(timer);
mHandlingTimerComplete = false;

timersHandled++;
}

Expand All @@ -160,10 +148,10 @@ CHIP_ERROR LayerImplLwIP::HandlePlatformTimer()

Clock::Timestamp currentTime = SystemClock().GetMonotonicTimestamp();

if (currentTime < mTimerList.Earliest()->mAwakenTime)
if (currentTime < mTimerList.Earliest()->AwakenTime())
{
// the next timer expires in the future, so set the delay to a non-zero value
delay = mTimerList.Earliest()->mAwakenTime - currentTime;
delay = mTimerList.Earliest()->AwakenTime() - currentTime;
}

StartPlatformTimer(delay);
Expand Down
4 changes: 3 additions & 1 deletion src/system/SystemLayerImplLwIP.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

#include <lib/support/ObjectLifeCycle.h>
#include <system/SystemLayer.h>
#include <system/SystemTimer.h>

namespace chip {
namespace System {
Expand Down Expand Up @@ -51,7 +52,8 @@ class LayerImplLwIP : public LayerLwIP

CHIP_ERROR StartPlatformTimer(System::Clock::Timeout aDelay);

Timer::MutexedList mTimerList;
TimerPool<TimerList::Node> mTimerPool;
TimerList mTimerList;
bool mHandlingTimerComplete; // true while handling any timer completion
ObjectLifeCycle mLayerState;
};
Expand Down
39 changes: 19 additions & 20 deletions src/system/SystemLayerImplSelect.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,6 @@ CHIP_ERROR LayerImplSelect::Init()

RegisterPOSIXErrorFormatter();

ReturnErrorOnFailure(mTimerList.Init());

for (auto & w : mSocketWatchPool)
{
w.Clear();
Expand All @@ -68,21 +66,23 @@ CHIP_ERROR LayerImplSelect::Shutdown()
{
VerifyOrReturnError(mLayerState.SetShuttingDown(), CHIP_ERROR_INCORRECT_STATE);

Timer * timer;
#if CHIP_SYSTEM_CONFIG_USE_DISPATCH
TimerList::Node * timer;
while ((timer = mTimerList.PopEarliest()) != nullptr)
{
timer->Clear();

#if CHIP_SYSTEM_CONFIG_USE_DISPATCH
if (timer->mTimerSource != nullptr)
{
dispatch_source_cancel(timer->mTimerSource);
dispatch_release(timer->mTimerSource);
}
#endif // CHIP_SYSTEM_CONFIG_USE_DISPATCH

timer->Release();
mTimerPool.Release(timer);
}
#else // CHIP_SYSTEM_CONFIG_USE_DISPATCH
mTimerList.Clear();
mTimerPool.ReleaseAll();
#endif // CHIP_SYSTEM_CONFIG_USE_DISPATCH

mWakeEvent.Close(*this);

mLayerState.ResetFromShuttingDown(); // Return to uninitialized state to permit re-initialization.
Expand Down Expand Up @@ -111,6 +111,7 @@ void LayerImplSelect::Signal()
CHIP_ERROR status = mWakeEvent.Notify();
if (status != CHIP_NO_ERROR)
{

ChipLogError(chipSystemLayer, "System wake event notify failed: %" CHIP_ERROR_FORMAT, status.Format());
}
}
Expand All @@ -123,7 +124,7 @@ CHIP_ERROR LayerImplSelect::StartTimer(Clock::Timeout delay, TimerCompleteCallba

CancelTimer(onComplete, appState);

Timer * timer = Timer::New(*this, delay, onComplete, appState);
TimerList::Node * timer = mTimerPool.Create(*this, delay, onComplete, appState);
VerifyOrReturnError(timer != nullptr, CHIP_ERROR_NO_MEMORY);

#if CHIP_SYSTEM_CONFIG_USE_DISPATCH
Expand Down Expand Up @@ -164,11 +165,9 @@ void LayerImplSelect::CancelTimer(TimerCompleteCallback onComplete, void * appSt
{
VerifyOrReturn(mLayerState.IsInitialized());

Timer * timer = mTimerList.Remove(onComplete, appState);
TimerList::Node * timer = mTimerList.Remove(onComplete, appState);
VerifyOrReturn(timer != nullptr);

timer->Clear();

#if CHIP_SYSTEM_CONFIG_USE_DISPATCH
if (timer->mTimerSource != nullptr)
{
Expand All @@ -177,7 +176,7 @@ void LayerImplSelect::CancelTimer(TimerCompleteCallback onComplete, void * appSt
}
#endif

timer->Release();
mTimerPool.Release(timer);
Signal();
}

Expand All @@ -187,7 +186,7 @@ CHIP_ERROR LayerImplSelect::ScheduleWork(TimerCompleteCallback onComplete, void

CancelTimer(onComplete, appState);

Timer * timer = Timer::New(*this, Clock::kZero, onComplete, appState);
TimerList::Node * timer = mTimerPool.Create(*this, Clock::kZero, onComplete, appState);
VerifyOrReturnError(timer != nullptr, CHIP_ERROR_NO_MEMORY);

#if CHIP_SYSTEM_CONFIG_USE_DISPATCH
Expand Down Expand Up @@ -332,7 +331,7 @@ void LayerImplSelect::PrepareEvents()
const Clock::Timestamp currentTime = SystemClock().GetMonotonicTimestamp();
Clock::Timestamp awakenTime = currentTime + kDefaultMinSleepPeriod;

Timer * timer = mTimerList.Earliest();
TimerList::Node * timer = mTimerList.Earliest();
if (timer && timer->AwakenTime() < awakenTime)
{
awakenTime = timer->AwakenTime();
Expand Down Expand Up @@ -386,11 +385,11 @@ void LayerImplSelect::HandleEvents()

// Obtain the list of currently expired timers. Any new timers added by timer callback are NOT handled on this pass,
// since that could result in infinite handling of new timers blocking any other progress.
Timer::List expiredTimers(mTimerList.ExtractEarlier(Clock::Timeout(1) + SystemClock().GetMonotonicTimestamp()));
Timer * timer = nullptr;
TimerList expiredTimers = mTimerList.ExtractEarlier(Clock::Timeout(1) + SystemClock().GetMonotonicTimestamp());
TimerList::Node * timer = nullptr;
while ((timer = expiredTimers.PopEarliest()) != nullptr)
{
timer->HandleComplete();
mTimerPool.Invoke(timer);
}

for (auto & w : mSocketWatchPool)
Expand All @@ -411,10 +410,10 @@ void LayerImplSelect::HandleEvents()
}

#if CHIP_SYSTEM_CONFIG_USE_DISPATCH
void LayerImplSelect::HandleTimerComplete(Timer * timer)
void LayerImplSelect::HandleTimerComplete(TimerList::Node * timer)
{
mTimerList.Remove(timer);
timer->HandleComplete();
mTimerPool.Invoke(timer);
}
#endif // CHIP_SYSTEM_CONFIG_USE_DISPATCH

Expand Down
Loading