Forums
It’s hard to do any non-trivial programming in C without having to use a struct. Structs are great! A single variable holds access to multiple pieces of data, and all the nasty details of how they’re laid out in memory are handled by the compiler.
In more modern OO languages, we take that kind of thing for granted. We’re so abstracted from the details of how memory is laid out it’s easy to forget how tricky and difficult actually managing that kind of memory layout is.
Of course, if you’re Jean-Yves R’s co-worker, letting structs manage your memory layout is beginner mode stuff.
Jean-Yves was trying to understand why a bunch of structs were taking up huge amounts of memory, relative to how much they should take. Every bit of memory mattered, as this was an embedded application. Already, these structs weren’t actually stored in RAM, but in the flash memory on the device. They served as a database- when a request came in over Modbus or CAN or I2C, the ID on the request would be used to look up the struct containing metadata for handling that request. It was complex software, so there were a lot of these structs taking up flash memory.
It didn’t take long to see that the structs were defined with padding to ensure every field fell on a multiple of 32-bits, which meant there were huge gaps in every struct. Why? Well, this is an example of how they’d search the database:
/* These lines are actually in an included header */
#define DAT_calcOffset(address,offset) (address += offset)
#define DAT_MODBUS_ID_OFFSET 0x00000002 /* (32 bits pointer) */
/*
* Database search code
*/
/*
* Setting up start adress at beginning of flash zone + an
* offset corresponding to the member of struct
*/
DAT_calcOffset(pu32SearchBaseAddress,DAT_MODBUS_ID_OFFSET);
/* Return : Status */
if(u16ModBusID >= 0x8000)
{
u16ModBusID -= 0x8000;
bReturnStatus = TRUE;
}
/* Increment until we find the correct ID */
while((*pu32SearchBaseAddress != u16ModBusID) && (u16Index < u16NbOfData))
{
pu32SearchBaseAddress += (sizeof(DAT_typDataArray)/4);
u16Index++;
}
if(u16Index == u16NbOfData)
{
if(penFlagReturn)
*penFlagReturn = (DAT_typFlag)DAT_enCFlagErrOF;
xSemaphoreGive(DAT_tMutexDatabase);
return DAT_tNullStr;
}pu32SearchBaseAddress is a pointer to a struct in the flash memory, at least until the DAT_calcOffset macro does a little pointer arithmetic to point it at a field within the struct- specifically the modbus message ID. Then, in the while loop, we keep incrementing that pointer based on sizeof(DAT_typDataArray)/4- which is the size of our struct.
You might be wondering, why not do something sane and readable, like pu32SearchBaseAddress->modbus_id? Well, obviously this is “optimized”.
Like all “optimizations”, this one is a tradeoff. In this case, the memory layout of the structs is now fixed, and the structs cannot ever evolve in the future without completely breaking this code. It’s also not portable, due to memory sizes.
On the flip side, this also offers many benefits. The code is cryptic and unreadable, which helps ensure job security. The fact that each type of message- modbus, CANbus, I2C, etc.- has its own database means that this code has to be written for each one of those protocols, ensuring that the developer always has lots of code to copy/paste between those datasets with minor changes to constants and variable names. This keeps their lines-of-code count up in the source control metrics.
It probably didn’t do anything for performance of course, as pretty much any C compiler is going to compile the -> operator into a static memory offset anyway, thus making this “optimization” pretty useless for performance.