Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using maps with dynamic pool allocator causes panic #4195

Open
abseee opened this issue Sep 3, 2024 · 3 comments
Open

Using maps with dynamic pool allocator causes panic #4195

abseee opened this issue Sep 3, 2024 · 3 comments

Comments

@abseee
Copy link

abseee commented Sep 3, 2024

Context

  • Operating System & Odin Version:
    Odin: dev-2024-08
    OS: Arch Linux, Linux 6.10.7-arch1-1
    CPU: Intel(R) Core(TM) i7-6700K CPU @ 4.00GHz
    RAM: 15932 MiB
    Backend: LLVM 18.1.8

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
Copy link
Contributor

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
Copy link
Collaborator

laytan commented Sep 4, 2024

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:

aligned_alloc :: proc(size, alignment: int, old_ptr: rawptr, old_size: int, zero_memory := true) -> ([]byte, Allocator_Error) {
.

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
Copy link
Collaborator

laytan commented Sep 12, 2024

Related: #4216

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants