HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMinor

Pool allocator in C

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
poolallocatorstackoverflow

Problem

This is an allocator that is used in replace of malloc if you do a lot of rapid, but small allocations on the heap. I wrote this for reducing system calls to improve performance. It works, but I've never written an allocator before, so I wondered if there is any improvements I can make generally with regards to readability, edge cases, or the concept in general.

pool.h

#ifndef POOL_H
#define POOL_H

#include 

struct list;

struct block {
    void* data;
    uint64_t len;
};

struct data_pool {
    struct list* blocks;
    struct block* current;
    uint64_t page_size;
};

struct data_pool 
make_pool(uint64_t capacity);

void* 
alloc(struct data_pool* pool, size_t len);

void 
destroy_pool(struct data_pool* pool);

#endif


pool.c

#include 

#include "pool.h"
#include "list.h"

#include 

struct data_pool 
make_pool(uint64_t page_size) {
    struct data_pool pool;
    pool.blocks = make_list(4);
    pool.page_size = page_size;
    return pool;
}

void 
push_block(struct data_pool* pool) {
    struct block* b = malloc(sizeof(*b));
    assert(b != NULL);

    b->data = malloc(sizeof(*b->data) * pool->page_size);
    assert(b->data != NULL);
    b->len = 0;
    push_item(pool->blocks, b);
    pool->current = b;
}

void* 
alloc(struct data_pool* pool, size_t len) {
    if (pool->current->len + len >= pool->page_size) {
        push_block(pool);
    }
    pool->current->len += len;
    return &pool->current->data[pool->current->len];
}

void 
destroy_pool(struct data_pool* pool) {
    for (uint64_t i = 0; i blocks->index; i++) {
        struct block* block = pool->blocks->items[i];
        free(block->data);
        free(block);
    }
}

Solution

A few thoughts to get your going...

  • Your implementation isn't thread safe, where as if malloc probably is (if you're linking to the right library for threaded development). This might not be an issue if you don't need it but is something to be aware of.



  • You can't free individual elements, you can only delete the entire pool. Again, this might not be an issue if you expect the items you're creating to have related lifetime.



-
This looks odd:

pool->current->len += len;
return &pool->current->data[pool->current->len];


It looks a lot to me like you're returning a pointer to the block of memory after the allocated block. Shouldn't you be returning:

return &pool->current->data[pool->current->len - len];


-
This is personal preference, but I'd have make_pool return a pointer to a malloced version of the data_pool rather than returning the structure itself.

-
It's hard to say for sure without seeing your list implementation, however you initialise pool.blocks in make_pool using make_list(4). When you're cleaning up the list in destroy_pool, there's no obvious cleanup of the pool->blocks pointer (which could indicate a potential memory leak).

Code Snippets

pool->current->len += len;
return &pool->current->data[pool->current->len];
return &pool->current->data[pool->current->len - len];

Context

StackExchange Code Review Q#148177, answer score: 5

Revisions (0)

No revisions yet.