MEM_RESERVE style arena and obj file format parser

Hello. I am writing obj file format parser with using memory arenas. So I need three types of arrays:vertices,textures,normals .Depends on which header I parsed I should push things into storage. When I first time thought how to do that push I wanted implement stretchy buffers. But then at discord I got advice to implement RESERVE/COMMIT style arena where I firstly reserve memory and then commit as I need. So I have some questions. 1) What I should consider when I choose default commit size and need I care about aligning? 2) When I pass 2GB with MEM_RESERVE flag to VirtualAlloc it doesn't allocate memory but when I pass 1GB all is okay. Why? 3) I doubt about correctnes of that way to push things:

                push_array(&arena_for_vertices,float,3);
                vertices[0] = v0;
                vertices[1] = v1;
                vertices[2] = v2;

At the end of parsing I push things to "permanent" storage and doing memcpy what seems not good. Thanks for any reply and advice. Code:

int
main(int argc, char *argv[])
{
void *general_memory = VirtualAlloc(0, MB(512), MEM_RESERVE|MEM_COMMIT, PAGE_READWRITE);
    ASSERT(general_memory);
    MemoryArena arena = {0};
    init_arena(&arena, MB(512), general_memory);
    
    // TODO(shvayko): doesn't allocate 2GB memory
    void *memory_for_vertices = VirtualAlloc(0 ,GB(1), MEM_RESERVE,PAGE_READWRITE);
    ASSERT(memory_for_vertices);
    void *memory_for_textures = VirtualAlloc(0, GB(1), MEM_RESERVE,PAGE_READWRITE);
    ASSERT(memory_for_textures);
    void *memory_for_normals  = VirtualAlloc(0, GB(1), MEM_RESERVE,PAGE_READWRITE);
    ASSERT(memory_for_normals);
    
    MemoryArena arena_for_vertices = {0};
    MemoryArena arena_for_textures = {0};
    MemoryArena arena_for_normals  = {0};
    
    init_arena(&arena_for_vertices, GB(1), memory_for_vertices);
    init_arena(&arena_for_textures, GB(1), memory_for_textures);
    init_arena(&arena_for_normals,  GB(1), memory_for_normals);
    
    MeshData *mesh = push_size(&arena, MeshData);
   
    for(u32 lines_index = 0;
        lines_index < lines_total;
        lines_index++)
    {
        LineInfo *line = lines_info + lines_index;
        u8 *token = line->p;
        u32 line_length = line->length;
        u8 *end = token + line_length;
        while(token != end)
        {
            if((token[0] == 'v') && (token[1] == ' '))
            {
                token++;
                f32 v0 = 0;
                f32 v1 = 0;
                f32 v2 = 0;
                token = ASCII_to_float(&token, &v0);
                token = ASCII_to_float(&token, &v1);
                token = ASCII_to_float(&token, &v2);
                
                float *vertices = 
                push_array(&arena_for_vertices,float,3);
                vertices[0] = v0;
                vertices[1] = v1;
                vertices[2] = v2;
                
                vertices_count++;
            } 
         }
     } // END PARSING

    // pushing into "permanent" storage and freeing "temps arenas"
    mesh->vertices = push_array(&arena,float, vertices_count);
    mesh->vertices = memcpy(mesh->vertices, arena_for_vertices.base, vertices_count*3*sizeof(float));
    arena_release(&arena_for_vertices);
    
}


#define push_size(arena, type) (type*)push_size_(arena, sizeof(type))
#define push_array(arena, type, count) (type*)push_size_(arena, sizeof(type)*count)

#define ALIGN_POW_2(x,p) (((x) + (p) - 1)&~((p) - 1))
#define DEFAULT_COMMIT_SIZE (MB(1))

void*
push_size_(MemoryArena *arena, size_t size)
{
    void *result = 0;
    ASSERT((arena->used+size) <= arena->size);
    
    result = arena->base + arena->used;
    arena->used += size;
    size_t commited = arena->commited;
    
    if(arena->used > commited)
    {
        size_t new_commited_size = ALIGN_POW_2(arena->used, MAX(size,DEFAULT_COMMIT_SIZE - 1));;
        VirtualAlloc(arena->base + commited,new_commited_size, MEM_COMMIT, PAGE_READWRITE);
        arena->commited += new_commited_size;
    }
    arena->size -= size;
    
    return result;
}

void
arena_release(MemoryArena *arena)
{
    ASSERT(arena);
    if(arena->base)
    {
        VirtualFree(arena->base,0,MEM_RELEASE);
    }
}


1) What is "default commit size"? You can commit in 4KB increments. Doing it for every 4KB can be too expensive as it is syscall. So do it every 1MB or something. 8MB is also good. Basically keep two extra positions in your arena - how much it is used, and how much it is committed. Once your used position gets to committed position, you increase committed position by 8MB (or more if push requests more) and commit that region. Now your size can increase for another 8MB before next commit.

What do you need to align? virtual memory is allocated with 64KB alignment. It is aligned already a lot. If you treat whole arena as array for same sized data, there's nothing much to do - just increment pointer and write data to it.

2) what does "doesn't allocate memory" mean? Returns NULL? What is GetLastError in such case? Is your GB macro correct?

3) what does not work with your push code?


Edited by Mārtiņš Možeiko on

Thanks for reply. When I pass 2GB instead if 1GB it throws code 87(The parameter is incorrect). I have tested macros and when I do GB(1) it gives right number of bytes but if I do GB(2) it gives very big number which is not correct. Looks like some weird overflow. Macros:

#define KB(x) (x * 1024)
#define MB(x) (KB(x) * 1024)
#define GB(x) (MB(x) * 1024)

I thought I need align requested commit size by power of two for performance(not sure about that)

{
size_t new_commited_size = ALIGN_POW_2(arena->used,MAX(size,DEFAULT_COMMIT_SIZE));
        VirtualAlloc(arena->base + commited,new_commited_size, MEM_COMMIT, PAGE_READWRITE);
        arena->commited += new_commited_size;
}

Push code works fine. Here question was more about code organization. But now I think and it is looks fine. I had thought there is more elegant way to do that:

float *vertices = push_array(&arena_for_vertices,float,3);
vertices[0] = v0;
vertices[1] = v1;
vertices[2] = v2;

Replying to mmozeiko (#25194)

2*1024*1024*1024 is 2147483648 which does not fit into int type. It is int type because 2 is int type. So then the 2147483648 value will wrap around to negative value -2147483648. Which then gets casted to SIZE_T for VirtualAlloc, which on 64-bit process means using 0xffffffff80000000 value.

If you want large unsigned numbers, you should be multiplying 2ULL constant with your 1024 values.

Or you do explicit cast in your macros: #define KB(x) ((SIZE_T)(x) * 1024)


Edited by Mārtiņš Možeiko on
Replying to ExTray2020 (#25195)