6
\$\begingroup\$

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 <stdint.h>

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 <stdlib.h>

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

#include <assert.h>

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 < pool->blocks->index; i++) {
        struct block* block = pool->blocks->items[i];
        free(block->data);
        free(block);
    }
}
\$\endgroup\$
  • \$\begingroup\$ Sure. So it's an allocator that is used in replace of malloc if you do a lot of rapid, but small allocations on the heap. Mostly exists as a performance improvement to reduce system calls. It works, but I've never written an allocator before, so I wondered if theres any improvements I can make generally to the concept as well in terms of readability, or if there are any edge cases I'm missing. \$\endgroup\$ – flooblebit Nov 26 '16 at 18:35
5
\$\begingroup\$

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).

\$\endgroup\$
  • \$\begingroup\$ The first two points are good to know, but luckily for me they don't apply to my use case. Good catch with that bug though! As for the last point, is there any reason why? I figured it didn't really need to be allocated on the heap. \$\endgroup\$ – flooblebit Nov 26 '16 at 19:40
  • \$\begingroup\$ @flooblebit Not really, as I said it's just a personal preference, I don't particularly like passing structures by value, I'd tend to refer to it as a pointer so I would naturally malloc it. \$\endgroup\$ – forsvarir Nov 26 '16 at 19:45
  • \$\begingroup\$ Ah, fair enough. Other than the listed items, everything looks okay? \$\endgroup\$ – flooblebit Nov 26 '16 at 19:52
  • \$\begingroup\$ @flooblebit The only other thing that raised a flag for me is that you don't seem to clean up pool.blocks. Since you haven't posted list.h, it is impossible to say for sure, but that may be a memory leak. \$\endgroup\$ – forsvarir Nov 26 '16 at 20:23
  • \$\begingroup\$ Oops yep, another good catch haha. There should be a destroy_list(pool->blocks). \$\endgroup\$ – flooblebit Nov 26 '16 at 20:25

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.