Skip to content

Commit

Permalink
Move PeerConnections to be template based (project-chip#1255)
Browse files Browse the repository at this point in the history 
* Move PeerConnections to be template based, to asses the impact in code size

* Fix test

* Trivial comment change to kick the restyler again

Co-authored-by: Andrei Litvin <[email protected]>
  • Loading branch information
@andy31415 @andreilitvin
andy31415 and andreilitvin authored Jun 25, 2020
1 parent 4d762b6 commit c567d83
Show file tree
Hide file tree
Showing 5 changed files with 108 additions and 176 deletions.
114 changes: 0 additions & 114 deletions src/transport/PeerConnections.cpp
View file

This file was deleted.

140 changes: 101 additions & 39 deletions src/transport/PeerConnections.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@
#ifndef PEER_CONNECTIONS_H_
#define PEER_CONNECTIONS_H_

#include <core/CHIPConfig.h>
#include <core/CHIPError.h>
#include <support/CodeUtils.h>
#include <system/TimeSource.h>
#include <transport/PeerConnectionState.h>

Expand All @@ -32,17 +32,10 @@ namespace Transport {
* - handle connection active time and expiration
* - allocate and free space for connection states.
*/
class PeerConnectionsBase
template <size_t kMaxConnectionCount, Time::Source kTimeSource = Time::Source::kSystem>
class PeerConnections
{
public:
/**
* Construct a PeerConnectionsBase object using a preallocated array used for connection state storage.
*/
PeerConnectionsBase(PeerConnectionState * storageArray, size_t arraySize) :
mConnectionStateArray(storageArray), mConnectionStateArraySize(arraySize)
{}
virtual ~PeerConnectionsBase() {}

/**
* Allocates a new peer connection state state object out of the internal resource pool.
*
Expand All @@ -54,17 +47,58 @@ class PeerConnectionsBase
* @returns CHIP_NO_ERROR if state could be initialized. May fail if maximum connection count
* has been reached (with CHIP_ERROR_NO_MEMORY).
*/
CHIP_ERROR CreateNewPeerConnectionState(const PeerAddress & address, PeerConnectionState ** state);
CHECK_RETURN_VALUE
CHIP_ERROR CreateNewPeerConnectionState(const PeerAddress & address, PeerConnectionState ** state)
{
CHIP_ERROR err = CHIP_ERROR_NO_MEMORY;

if (state)
{
*state = nullptr;
}

for (size_t i = 0; i < kMaxConnectionCount; i++)
{
if (!mStates[i].GetPeerAddress().IsInitialized())
{
mStates[i] = PeerConnectionState(address);
mStates[i].SetLastActivityTimeMs(mTimeSource.GetCurrentMonotonicTimeMs());

if (state)
{
*state = &mStates[i];
}

err = CHIP_NO_ERROR;
break;
}
}

return err;
}

/**
* Get a peer connection state given a peer address.
* Get a peer connection state given a Peer address.
*
* @param address is the connection to find (based on address)
* @param state [out] the connection if found, null otherwise. MUST not be null.
*
* @return true if a corresponding state was found.
*/
bool FindPeerConnectionState(const PeerAddress & address, PeerConnectionState ** state);
CHECK_RETURN_VALUE
bool FindPeerConnectionState(const PeerAddress & address, PeerConnectionState ** state)
{
*state = nullptr;
for (size_t i = 0; i < kMaxConnectionCount; i++)
{
if (mStates[i].GetPeerAddress() == address)
{
*state = &mStates[i];
break;
}
}
return *state != nullptr;
}

/**
* Get a peer connection state given a Node Id.
Expand All @@ -75,18 +109,66 @@ class PeerConnectionsBase
*
* @return true if a corresponding state was found.
*/
bool FindPeerConnectionState(NodeId nodeId, PeerConnectionState ** state);
CHECK_RETURN_VALUE
bool FindPeerConnectionState(NodeId nodeId, PeerConnectionState ** state)
{
*state = nullptr;
for (size_t i = 0; i < kMaxConnectionCount; i++)
{
if (!mStates[i].GetPeerAddress().IsInitialized())
{
continue;
}
if (mStates[i].GetPeerNodeId() == nodeId)
{
*state = &mStates[i];
break;
}
}
return *state != nullptr;
}

/// Convenience method to mark a peer connection state as active
void MarkConnectionActive(PeerConnectionState * state) { state->SetLastActivityTimeMs(GetCurrentMonotonicTimeMs()); }
void MarkConnectionActive(PeerConnectionState * state)
{
state->SetLastActivityTimeMs(mTimeSource.GetCurrentMonotonicTimeMs());
}

/**
* Iterates through all active connections and expires any connection with an idle time
* larger than the given amount.
*
* Expiring a connection involves callback execution and then clearing the internal state.
*/
void ExpireInactiveConnections(uint64_t maxIdleTimeMs);
void ExpireInactiveConnections(uint64_t maxIdleTimeMs)
{
const uint64_t currentTime = mTimeSource.GetCurrentMonotonicTimeMs();

for (size_t i = 0; i < kMaxConnectionCount; i++)
{
if (!mStates[i].GetPeerAddress().IsInitialized())
{
continue; // not an active connection
}

uint64_t connectionActiveTime = mStates[i].GetLastActivityTimeMs();
if (connectionActiveTime + maxIdleTimeMs >= currentTime)
{
continue; // not expired
}

if (OnConnectionExpired)
{
OnConnectionExpired(mStates[i], mConnectionExpiredArgument);
}

// Connection is assumed expired, marking it as invalid
mStates[i] = PeerConnectionState(PeerAddress::Uninitialized());
}
}

/// Allows access to the underlying time source used for keeping track of connection active time
Time::TimeSource<kTimeSource> & GetTimeSource() { return mTimeSource; }

/**
* Sets the handler for expired connections
Expand All @@ -102,36 +184,16 @@ class PeerConnectionsBase
OnConnectionExpired = reinterpret_cast<ConnectionExpiredHandler>(handler);
}

protected:
/// Get the current time from a Time::TimeSource or equivalent
virtual uint64_t GetCurrentMonotonicTimeMs() = 0;

private:
PeerConnectionState * mConnectionStateArray;
const size_t mConnectionStateArraySize;
Time::TimeSource<kTimeSource> mTimeSource;
PeerConnectionState mStates[kMaxConnectionCount];

typedef void (*ConnectionExpiredHandler)(const PeerConnectionState & state, void * param);

ConnectionExpiredHandler OnConnectionExpired = nullptr; ///< Callback for when a connection expires
ConnectionExpiredHandler OnConnectionExpired = nullptr; ///< Callback for connection expiry
void * mConnectionExpiredArgument = nullptr; ///< Argument for callback
};

/**
* Concrete peer connections implementation based on system sizes and timers.
*/
class PeerConnections : public PeerConnectionsBase
{
public:
PeerConnections() : PeerConnectionsBase(mState, sizeof(mState) / sizeof((mState)[0])) {}

protected:
uint64_t GetCurrentMonotonicTimeMs() override { return mTimeSource.GetCurrentMonotonicTimeMs(); }

private:
PeerConnectionState mState[CHIP_CONFIG_PEER_CONNECTION_POOL_SIZE];
Time::TimeSource<Time::Source::kSystem> mTimeSource;
};

} // namespace Transport
} // namespace chip

Expand Down
6 changes: 3 additions & 3 deletions src/transport/SecureSessionMgr.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,9 +130,9 @@ class DLL_EXPORT SecureSessionMgr : public ReferenceCounted<SecureSessionMgr>
// TODO: add support for multiple transports (TCP, BLE to be added)
Transport::UDP mTransport;

NodeId mLocalNodeId; //< Id of the current node
Transport::PeerConnections mPeerConnections; //< Active connections to other peers
State mState; //< Initialization state of the object
NodeId mLocalNodeId; //< Id of the current node
Transport::PeerConnections<CHIP_CONFIG_PEER_CONNECTION_POOL_SIZE> mPeerConnections; //< Active connections to other peers
State mState; //< Initialization state of the object

/**
* This function is the application callback that is invoked when a message is received over a
Expand Down
1 change: 0 additions & 1 deletion src/transport/TransportLayer.am
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
CHIP_BUILD_TRANSPORT_LAYER_SOURCE_FILES = \
@top_builddir@/src/transport/SecureSession.cpp \
@top_builddir@/src/transport/MessageHeader.cpp \
@top_builddir@/src/transport/PeerConnections.cpp \
@top_builddir@/src/transport/SecureSessionMgr.cpp \
@top_builddir@/src/transport/UDP.cpp \
$(NULL)
Expand Down
23 changes: 4 additions & 19 deletions src/transport/tests/TestPeerConnections.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,26 +53,11 @@ const NodeId kPeer1NodeId = 123;
const NodeId kPeer2NodeId = 6;
const NodeId kPeer3NodeId = 81;

/// A Peer connections that supports exactly 2 connections and a test time source.
class TestPeerConnections : public PeerConnectionsBase
{
public:
TestPeerConnections() : PeerConnectionsBase(mState, ArraySize(mState)) {}
Time::TimeSource<Time::Source::kTest> & GetTimeSource() { return mTimeSource; }

protected:
uint64_t GetCurrentMonotonicTimeMs() override { return mTimeSource.GetCurrentMonotonicTimeMs(); }

private:
PeerConnectionState mState[2];
Time::TimeSource<Time::Source::kTest> mTimeSource;
};

void TestBasicFunctionality(nlTestSuite * inSuite, void * inContext)
{
CHIP_ERROR err;
PeerConnectionState * statePtr;
TestPeerConnections connections;
PeerConnections<2, Time::Source::kTest> connections;
connections.GetTimeSource().SetCurrentMonotonicTimeMs(100);

err = connections.CreateNewPeerConnectionState(kPeer1Addr, nullptr);
Expand All @@ -93,7 +78,7 @@ void TestFindByAddress(nlTestSuite * inSuite, void * inContext)
{
CHIP_ERROR err;
PeerConnectionState * statePtr;
TestPeerConnections connections;
PeerConnections<2, Time::Source::kTest> connections;

err = connections.CreateNewPeerConnectionState(kPeer1Addr, nullptr);
NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);
Expand All @@ -114,7 +99,7 @@ void TestFindByNodeId(nlTestSuite * inSuite, void * inContext)
{
CHIP_ERROR err;
PeerConnectionState * statePtr;
TestPeerConnections connections;
PeerConnections<2, Time::Source::kTest> connections;

err = connections.CreateNewPeerConnectionState(kPeer1Addr, &statePtr);
NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);
Expand Down Expand Up @@ -155,7 +140,7 @@ void TestExpireConnections(nlTestSuite * inSuite, void * inContext)
CHIP_ERROR err;
ExpiredCallInfo callInfo;
PeerConnectionState * statePtr;
TestPeerConnections connections;
PeerConnections<2, Time::Source::kTest> connections;

connections.SetConnectionExpiredHandler(OnConnectionExpired, &callInfo);

Expand Down

0 comments on commit c567d83

Please sign in to comment.