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

improve std.heap.c_allocator to use aligned memory functions #3783

Closed
andrewrk opened this issue Nov 26, 2019 · 5 comments · Fixed by #6385
Closed

improve std.heap.c_allocator to use aligned memory functions #3783

andrewrk opened this issue Nov 26, 2019 · 5 comments · Fixed by #6385
Labels
contributor friendly This issue is limited in scope and/or knowledge of Zig internals. enhancement Solving this issue will likely involve adding new logic or components to the codebase. standard library This issue involves writing Zig code for the standard library.
Milestone

Comments

@andrewrk
Copy link
Member

Currently, c_allocator just wraps realloc. This guarantees an alignment of 8 bytes only. However, Zig allocators are supposed to support any alignment. In particular, an async function frame (on some targets, such as x86_64) requires alignment 16. I briefly tried to use one of the libc aligned memory allocation functions in the past, but it wasn't available on all the libcs we wanted to target.

However, that was before we had libc versioning support, and upon further reflection, it seems that all the libcs we want to target do support aligned allocation in some form or other.

Here's some brief research:

  • musl supports memalign, posix_memalign, and aligned_alloc. (we provide musl so it will always have all of these)
  • glibc supports memalign in all versions; posix_memalign since 2.1.91, aligned_alloc since 2.16.
  • FreeBSD and NetBSD support posix_memalign and aligned_alloc
  • macOS supports posix_memalign
  • mingw-w64 and windows UCRT have _aligned_realloc

I believe that covers all the targets, so we can solve this with some additional logic in std.heap.cRealloc and std.heap.cShrink.

@andrewrk andrewrk added contributor friendly This issue is limited in scope and/or knowledge of Zig internals. standard library This issue involves writing Zig code for the standard library. labels Nov 26, 2019
@andrewrk andrewrk added this to the 0.7.0 milestone Nov 26, 2019
@andrewrk andrewrk added the enhancement Solving this issue will likely involve adding new logic or components to the codebase. label Nov 26, 2019
@daurnimator
Copy link
Contributor

TIL about posix_memalign.
From my reading of the man page, it looks like realloc isn't necessarily supported? And even if it was, there isn't a way to increase alignment?
i.e. we'd have to implement realloc ourselves by doing a new allocation and copying into it.
With such a limitation, we probably need to dispatch to the current (realloc-using) routine if alignment is <= 8, but switch to posix_memalign if the alignment requested is higher?

@andrewrk
Copy link
Member Author

With such a limitation, we probably need to dispatch to the current (realloc-using) routine if alignment is <= 8, but switch to posix_memalign if the alignment requested is higher?

I think that will be a reasonable, safe approach to take. One thing to consider is that in theory we should be able to improve the way interfaces work in zig so that alignment is comptime. So this <= 8 branch would be free, at least.

@daurnimator
Copy link
Contributor

One thing to consider is that in theory we should be able to improve the way interfaces work in zig so that alignment is comptime.

I'm not sure how that would work if we keep the function-pointer style unless function pointers gained known constants for comptime arguments?

@andrewrk
Copy link
Member Author

Rich Felker popped by and pointed out that libc specifies max_align_t to tell the alignment that malloc provides. So this should go into the libc bits/ files and then it is a comptime value that can be used for logic in the C allocator wrapper.

Combined with some kind of better interface pattern that allows comptime alignment, this will provide a zero cost wrapper for libc allocation, if the required alignment is within max_align_t.

LemonBoy added a commit to LemonBoy/zig that referenced this issue Sep 20, 2020
Use posix_memalign where available and the _aligned_{malloc,free} API on
Windows.

Closes ziglang#3783
LemonBoy added a commit to LemonBoy/zig that referenced this issue Oct 23, 2020
Use posix_memalign where available and the _aligned_{malloc,free} API on
Windows.

Closes ziglang#3783
LemonBoy added a commit to LemonBoy/zig that referenced this issue Nov 5, 2020
Use posix_memalign where available and the _aligned_{malloc,free} API on
Windows.

Closes ziglang#3783
@andrewrk andrewrk modified the milestones: 0.7.0, 0.8.0 Nov 6, 2020
@andrewrk
Copy link
Member Author

std.heap.raw_c_allocator added in 13cccdd.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributor friendly This issue is limited in scope and/or knowledge of Zig internals. enhancement Solving this issue will likely involve adding new logic or components to the codebase. standard library This issue involves writing Zig code for the standard library.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants