odin-lang / Odin

Odin Programming Language
https://odin-lang.org
BSD 3-Clause "New" or "Revised" License
6.9k stars 606 forks source link

Using maps with dynamic pool allocator causes panic #4195

Open abseee opened 2 months ago

abseee commented 2 months ago

Context

Expected Behavior

The dynamic pool allocator should work with maps by default

Current Behavior

Inserting into a map causes a panic panic: allocation not aligned to a cache line from map_alloc_dynamic

Steps to Reproduce

    pool := mem.Dynamic_Pool{}
    mem.dynamic_pool_init(&pool)
    // pool.alignment = 64
    defer mem.dynamic_pool_destroy(&pool)
    pool_alloc := mem.dynamic_pool_allocator(&pool)
    context.allocator = pool_alloc
    my_map := map[string]bool {}
    defer delete(my_map)
    my_map["foo"] = true
Feoramund commented 2 months ago

It might be more correct for the allocator to assert if the size and alignment requested by an allocation don't match the values set at init, if this is supposed to reflect the design in Bill's article about pool allocators.

Though, it looks like Dynamic_Pool allocates chunks, which are then split up as needed, and each allocation within a chunk has a forced alignment. I remember having this issue when I was rewriting the test runner and surveying allocators to use. I'm not wholly sure what the intent here is without some further input.

laytan commented 2 months ago

This specific allocator should imo just be changed to adhere to the requested alignment per allocation, because it has no individual free implementation that should be pretty easy to do.

There are more allocators that are bad like this one, the buddy allocator for example. That allocator does have individual free and I am thinking it should do the same thing as our heap allocator and allocate space for a small header in front of the actual allocation with alignment information, see: https://github.com/odin-lang/Odin/blob/b2c5998e78599db2e2ee20db508b96fe2d5acb10/base/runtime/heap_allocator.odin#L22.

In general, having allocators not respect the requested alignment is bad and we should avoid that in all our allocators, it violates the allocator interface.

laytan commented 1 month ago

Related: https://github.com/odin-lang/Odin/discussions/4216