patterncMinor
Memory management for small allocations
Viewed 0 times
smallforallocationsmemorymanagement
Problem
This code is for a program that needs to add thousands of strings to memory while it's running and these strings will be used until the end of execution. So the memory for these strings will be freed only in the end.
First I tested calling
This is my first attempt at doing this, so I know this code is probably bad. I would like to know exactly what is wrong with it, so I won't make the same mistakes again.
```
//The actual numbers will be smaller after DEBUG
#define BIG_BLOCK 1073741824 // 1GiB ..........
#define MEDIUM_BLOCK 524288000 //500MiB .....
#define SMALL_BLOCK 104857600 //100MiB .
//Linked-list containing the required data for each malloc'ed block
typedef struct {
void *mem_pool; //starting address for this block
void *next; //next node
size_t *position; //current position
size_t *available; //how much memory was allocated?
} memory_t;
//First link
static memory_t *root = 0;
//handles malloc and returns a new memory_t node, it allocates
//more memory on the first call than on subsequent calls
static void *increase_memory(void)
{
void *result = 0;
memory_t *new_block = 0;
static int firstCall = 0;
static const size_t default_sizes[] = {BIG_BLOCK, MEDIUM_BLOCK, SMALL_BLOCK, 0};
for(size_t i = firstCall; default_sizes[i] != 0; ++i)
{
if((result = malloc(default_sizes[i])))
{
new_block = result;
new_block->mem_pool = new_block; //starting address for this block
new_block->next = 0; //link to next node
new_block->positi
First I tested calling
malloc for every new string and have used Valgrind to benchmark total execution cost for 200 hundred thousand strings. Then I wrote these memory handling functions that call malloc once and hand out small chunks of memory for every string. The overall execution cost for the entire program went down over 20%.This is my first attempt at doing this, so I know this code is probably bad. I would like to know exactly what is wrong with it, so I won't make the same mistakes again.
```
//The actual numbers will be smaller after DEBUG
#define BIG_BLOCK 1073741824 // 1GiB ..........
#define MEDIUM_BLOCK 524288000 //500MiB .....
#define SMALL_BLOCK 104857600 //100MiB .
//Linked-list containing the required data for each malloc'ed block
typedef struct {
void *mem_pool; //starting address for this block
void *next; //next node
size_t *position; //current position
size_t *available; //how much memory was allocated?
} memory_t;
//First link
static memory_t *root = 0;
//handles malloc and returns a new memory_t node, it allocates
//more memory on the first call than on subsequent calls
static void *increase_memory(void)
{
void *result = 0;
memory_t *new_block = 0;
static int firstCall = 0;
static const size_t default_sizes[] = {BIG_BLOCK, MEDIUM_BLOCK, SMALL_BLOCK, 0};
for(size_t i = firstCall; default_sizes[i] != 0; ++i)
{
if((result = malloc(default_sizes[i])))
{
new_block = result;
new_block->mem_pool = new_block; //starting address for this block
new_block->next = 0; //link to next node
new_block->positi
Solution
-
I know it's kind of standard to do this in C land but you should spell things like this
Yes it's one line of code more but the intent is much more obvious at a first glance. As suggested in the comments you might want to invert the condition and continue. Reduces the level of nesting:
Technically it's better to write
-
It's perfectly legal for a struct to contain a pointer to itself like this:
-
In
I know it's kind of standard to do this in C land but you should spell things like this
if((result = malloc(default_sizes[i]))) out as:result = malloc(default_sizes[i]);
if (result != NULL)
{
}Yes it's one line of code more but the intent is much more obvious at a first glance. As suggested in the comments you might want to invert the condition and continue. Reduces the level of nesting:
result = malloc(default_sizes[i]);
if (result == NULL) { continue; }Technically it's better to write
if (NULL == result) because if you accidentally write = instead of == then the compiler will throw an error instead of compiling buggy code - I just can't seem to get myself into that habit for some reason. Probably because it reads a bit clunkier that way (imho).-
It's perfectly legal for a struct to contain a pointer to itself like this:
typedef struct memory_ {
void *mem_pool; //starting address for this block
struct memory_ *next; //next node
size_t *position; //current position
size_t *available; //how much memory was allocated?
} memory_t;- The return type of
increase_memoryshould bememory_t *because that's what it returns. You can also get rid ofresultin there and usenew_blockin its place.
-
In
init_mem_pool you asked some questions on the return value. I guess you want it to indicate whether the initialization was successful or not - which is if root is NULL or not so you can just write this:return root != NULL;Code Snippets
result = malloc(default_sizes[i]);
if (result != NULL)
{
}result = malloc(default_sizes[i]);
if (result == NULL) { continue; }typedef struct memory_ {
void *mem_pool; //starting address for this block
struct memory_ *next; //next node
size_t *position; //current position
size_t *available; //how much memory was allocated?
} memory_t;return root != NULL;Context
StackExchange Code Review Q#32689, answer score: 2
Revisions (0)
No revisions yet.