Skip to content

Commit

Permalink
Fix nasa#929, Add message map has implementation
Browse files Browse the repository at this point in the history
- Message map size based on used routes
- Oversized (4x) to limit collisions while retaining
  resonable size related to routing table (still smaller)
- ~10% single collisions seen for full routing table
  with realistic message ID use
- Oversizing means map can never fill, simplifies logic
- Observed approximately 10%-20% performance hit, trade
  against memory use (can now use full 32 bit MsgId space)
- Hash intended for 32 bit, if CFE_SB_MsgId_Atom_t size
  changes may require modification to hash
- Also added full coverage unit tests
  • Loading branch information
skliper committed Oct 21, 2020
1 parent 0aac36a commit 5a795ad
Show file tree
Hide file tree
Showing 2 changed files with 281 additions and 0 deletions.
163 changes: 163 additions & 0 deletions modules/sbr/src/cfe_sbr_map_hash.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,163 @@
/*
** GSC-18128-1, "Core Flight Executive Version 6.7"
**
** Copyright (c) 2006-2019 United States Government as represented by
** the Administrator of the National Aeronautics and Space Administration.
** All Rights Reserved.
**
** Licensed under the Apache License, Version 2.0 (the "License");
** you may not use this file except in compliance with the License.
** You may obtain a copy of the License at
**
** http://www.apache.org/licenses/LICENSE-2.0
**
** Unless required by applicable law or agreed to in writing, software
** distributed under the License is distributed on an "AS IS" BASIS,
** WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
** See the License for the specific language governing permissions and
** limitations under the License.
*/

/******************************************************************************
* Hash routing map implementation
*
* Notes:
* These functions manipulate/access global variables and need
* to be protected by the SB Shared data lock.
*
*/

/*
* Include Files
*/

#include "common_types.h"
#include "private/cfe_sbr.h"
#include "cfe_sbr_priv.h"
#include <string.h>
#include <limits.h>

/*
* Macro Definitions
*/

/**
* \brief Message map size
*
* For hash mapping, map size is a multiple of maximum number of routes.
* The multiple impacts the number of collisions when the routes fill up.
* 4 was initially chosen to provide for plenty of holes in the map, while
* still remaining much smaller than the routing table. Note the
* multiple must be a factor of 2 to use the efficient shift logic, and
* can't be bigger than what can be indexed by CFE_SB_MsgId_Atom_t
*/
#define CFE_SBR_MSG_MAP_SIZE (4 * CFE_PLATFORM_SB_MAX_MSG_IDS)

/* Verify power of two */
#if ((CFE_SBR_MSG_MAP_SIZE & (CFE_SBR_MSG_MAP_SIZE - 1)) != 0)
#error CFE_SBR_MSG_MAP_SIZE must be a power of 2 for hash algorithm to work
#endif

/** \brief Hash algorithm magic number
*
* Ref:
* https://stackoverflow.com/questions/664014/what-integer-hash-function-are-good-that-accepts-an-integer-hash-key/12996028#12996028
*/
#define CFE_SBR_HASH_MAGIC (0x45d9f3b)

/******************************************************************************
* Shared data
*/

/** \brief Message map shared data */
CFE_SBR_RouteId_t CFE_SBR_MSGMAP[CFE_SBR_MSG_MAP_SIZE];

/******************************************************************************
* Internal helper function to hash the message id
*
* Note: algorithm designed for a 32 bit int, changing the size of
* CFE_SB_MsgId_Atom_t may require an update to this impelementation
*/
CFE_SB_MsgId_Atom_t CFE_SBR_MsgIdHash(CFE_SB_MsgId_t MsgId)
{
CFE_SB_MsgId_Atom_t hash;

hash = CFE_SB_MsgIdToValue(MsgId);

hash = ((hash >> 16) ^ hash) * CFE_SBR_HASH_MAGIC;
hash = ((hash >> 16) ^ hash) * CFE_SBR_HASH_MAGIC;
hash = (hash >> 16) ^ hash;

/* Reduce to fit in map */
hash &= CFE_SBR_MSG_MAP_SIZE - 1;

return hash;
}

/******************************************************************************
* Interface function - see header for description
*/
void CFE_SBR_Init_Map(void)
{
/* Clear the shared data */
memset(&CFE_SBR_MSGMAP, 0, sizeof(CFE_SBR_MSGMAP));
}

/******************************************************************************
* Interface function - see header for description
*/
uint32 CFE_SBR_SetRouteId(CFE_SB_MsgId_t MsgId, CFE_SBR_RouteId_t RouteId)
{
CFE_SB_MsgId_Atom_t hash;
uint32 collisions = 0;

if (CFE_SB_IsValidMsgId(MsgId))
{
hash = CFE_SBR_MsgIdHash(MsgId);

/*
* Increment from original hash to find the next open slot.
* Since map is larger than possible routes this will
* never deadlock
*/
while (CFE_SBR_IsValidRouteId(CFE_SBR_MSGMAP[hash]))
{
/* Increment or loop to start of array */
hash = (hash + 1) & (CFE_SBR_MSG_MAP_SIZE - 1);
collisions++;
}

CFE_SBR_MSGMAP[hash] = RouteId;
}

return collisions;
}

/******************************************************************************
* Interface function - see API for description
*/
CFE_SBR_RouteId_t CFE_SBR_GetRouteId(CFE_SB_MsgId_t MsgId)
{
CFE_SB_MsgId_Atom_t hash;
CFE_SBR_RouteId_t routeid = CFE_SBR_INVALID_ROUTE_ID;

if (CFE_SB_IsValidMsgId(MsgId))
{
hash = CFE_SBR_MsgIdHash(MsgId);
routeid = CFE_SBR_MSGMAP[hash];

/*
* Increment from original hash to find matching route.
* Since map is larger than possible routes this will
* never deadlock
*/
while (CFE_SBR_IsValidRouteId(routeid) && !CFE_SB_MsgId_Equal(CFE_SBR_GetMsgId(routeid), MsgId))
{
/* Increment or loop to start of array */
hash = (hash + 1) & (CFE_SBR_MSG_MAP_SIZE - 1);
routeid = CFE_SBR_MSGMAP[hash];
}
}

return routeid;
}
118 changes: 118 additions & 0 deletions modules/sbr/unit-test-coverage/test_cfe_sbr_map_hash.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
/*
** GSC-18128-1, "Core Flight Executive Version 6.7"
**
** Copyright (c) 2006-2019 United States Government as represented by
** the Administrator of the National Aeronautics and Space Administration.
** All Rights Reserved.
**
** Licensed under the Apache License, Version 2.0 (the "License");
** you may not use this file except in compliance with the License.
** You may obtain a copy of the License at
**
** http://www.apache.org/licenses/LICENSE-2.0
**
** Unless required by applicable law or agreed to in writing, software
** distributed under the License is distributed on an "AS IS" BASIS,
** WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
** See the License for the specific language governing permissions and
** limitations under the License.
*/

/*
* Test SBR direct message map implementation
*/

/*
* Includes
*/
#include "utassert.h"
#include "ut_support.h"
#include "private/cfe_sbr.h"
#include "cfe_sbr_priv.h"

/*
* Defines
*/

/* Unhash magic number */
#define CFE_SBR_UNHASH_MAGIC (0x119de1f3)

/******************************************************************************
* Local helper to unhash
*/
CFE_SB_MsgId_t Test_SBR_Unhash(CFE_SB_MsgId_Atom_t Hash)
{

Hash = ((Hash >> 16) ^ Hash) * CFE_SBR_UNHASH_MAGIC;
Hash = ((Hash >> 16) ^ Hash) * CFE_SBR_UNHASH_MAGIC;
Hash = (Hash >> 16) ^ Hash;

return CFE_SB_ValueToMsgId(Hash);
}

void Test_SBR_Map_Hash(void)
{

CFE_SB_MsgId_Atom_t msgidx;
CFE_SBR_RouteId_t routeid[3];
CFE_SB_MsgId_t msgid[3];
uint32 count;
uint32 collisions;

UtPrintf("Invalid msg checks");
ASSERT_EQ(CFE_SBR_SetRouteId(CFE_SB_ValueToMsgId(0), CFE_SBR_ValueToRouteId(0)), 0);
ASSERT_EQ(CFE_SBR_IsValidRouteId(CFE_SBR_GetRouteId(CFE_SB_ValueToMsgId(0))), false);

UtPrintf("Initialize routing and map");
CFE_SBR_Init();

/* Force valid msgid responses */
UT_SetForceFail(UT_KEY(CFE_SB_IsValidMsgId), true);

UtPrintf("Check that all entries are set invalid");
count = 0;
for (msgidx = 0; msgidx <= CFE_PLATFORM_SB_HIGHEST_VALID_MSGID; msgidx++)
{
if (!CFE_SBR_IsValidRouteId(CFE_SBR_GetRouteId(CFE_SB_ValueToMsgId(msgidx))))
{
count++;
}
}
ASSERT_EQ(count, CFE_PLATFORM_SB_HIGHEST_VALID_MSGID + 1);

/* Note AddRoute required for hash logic to work since it depends on MsgId in routing table */
UtPrintf("Add routes and check with a rollover and a skip");
msgid[0] = CFE_SB_ValueToMsgId(0);
msgid[1] = Test_SBR_Unhash(0xFFFFFFFF);
msgid[2] = Test_SBR_Unhash(0x7FFFFFFF);
routeid[0] = CFE_SBR_AddRoute(msgid[0], &collisions);
ASSERT_EQ(collisions, 0);
routeid[1] = CFE_SBR_AddRoute(msgid[1], &collisions);
ASSERT_EQ(collisions, 0);
routeid[2] = CFE_SBR_AddRoute(msgid[2], &collisions);
ASSERT_EQ(collisions, 2);

ASSERT_EQ(CFE_SBR_RouteIdToValue(CFE_SBR_GetRouteId(msgid[0])), CFE_SBR_RouteIdToValue(routeid[0]));
ASSERT_EQ(CFE_SBR_RouteIdToValue(CFE_SBR_GetRouteId(msgid[1])), CFE_SBR_RouteIdToValue(routeid[1]));
ASSERT_EQ(CFE_SBR_RouteIdToValue(CFE_SBR_GetRouteId(msgid[2])), CFE_SBR_RouteIdToValue(routeid[2]));

/* Performance check, 0xFFFFFF on 3.2GHz linux box is around 8-9 seconds */
count = 0;
for (msgidx = 0; msgidx <= 0xFFFF; msgidx++)
{
if (CFE_SBR_IsValidRouteId(CFE_SBR_GetRouteId(CFE_SB_ValueToMsgId(msgidx))))
{
count++;
}
}
UtPrintf("Valid route id's encountered in performance loop: %u", count);
}

/* Main unit test routine */
void UtTest_Setup(void)
{
UT_Init("map_hash");
UtPrintf("Software Bus Routing hash map coverage test...");

UT_ADD_TEST(Test_SBR_Map_Hash);
}

0 comments on commit 5a795ad

Please sign in to comment.