-
Notifications
You must be signed in to change notification settings - Fork 16
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
mallocator: align allocations to 64 bytes #282
Comments
Ah, but Go directly uses |
The best thing I can think of is to wrap aligned_alloc but that carries a number of caveats (requires C11, not portable to Windows, does not support realloc, cannot align an allocation smaller than the alignment). Maybe we can consider this if there's enough demand. |
We're still using a |
I was looking at that. In that case maybe we just make |
Possibly, we'd have to make sure that the release callback still releases the original pointer, but otherwise it could work if we make sure to overallocate. |
Well otherwise we'd have to change the allocator API to return the |
That's a good point! It's definitely easier to manage in the buffer class and use the slice! |
The other complication is resizing, since resizing won't preserve the alignment; we may have to copy |
Resizing frequently requires a copy anyways, so as long as we handle it properly it should be okay I think? |
Yeah. The other thing is the native Go allocator already does the trick, so we need to take care not to double-do it. So I'm thinking I'll either add a new "AllocateAligned" that does return the pair, or a flag on the allocator interface to indicate whether the allocator handles it natively or not. |
Go has an |
Well then we'd have to allocate and try again? I'm just saying that, if the allocator already does the padding trick underneath (or otherwise somehow supports aligning the allocation), then we shouldn't add the padding ourselves in Buffer |
This reverts commit ebe02d0.
Describe the enhancement requested
Some types want 16 byte (or more) alignment, such as decimal256, and it's easiest just to overallocate + align to avoid issues (even if it's not strictly required by the spec, many things in practice assume it). We have to overallocate since the allocator API is not given an alignment.
Context: apache/arrow-adbc#2526
Component(s)
Other
The text was updated successfully, but these errors were encountered: