DirectAllocator alignments > os.page_size on posix #939
Conversation
|
|
||
| if(alloc_size == n) return @intToPtr(&u8, addr)[0..n]; | ||
|
|
||
| var aligned_addr = addr & ~usize(alignment - 1); |
There was a problem hiding this comment.
What does ~usize(alignment - 1) compute when alignment == 0? 0x1fffffff? That'd do the wrong thing when &-ed with the address. What about alignments that aren't a power of two?
There was a problem hiding this comment.
Are non-po2 alignments supported? The current implementation of DirectAllocator for posix assumes po2, and I have never heard of a use case for anything other than po2 alignment.
That said, the Windows implementation allows it. If it is not supported, then we should assert alignment is po2, but for now I'll take a look at supporting it.
Align of 0 is < os.page_size and therefore would not trigger this code.
There was a problem hiding this comment.
I'll add this documentation to std.mem.Allocator:
- alignment is guaranteed to be >= 1
- alignment is guaranteed to be a power of 2
There was a problem hiding this comment.
This is actually enforced by the compiler:
return ([]align(alignment) T)(@alignCast(alignment, byte_slice));if you passed 0 or non power of 2 to alignment the cast in this line will give you a compile error.
| // pass munmap bytes that exist outside our allocated pages or it | ||
| // will happily eat us too | ||
|
|
||
| //Since alignment > page_size, we are by definition on a page boundry |
| //It is impossible that there is an unoccupied page at the top of our | ||
| // mmap. | ||
|
|
||
| return @intToPtr(&u8, aligned_addr)[0..n]; |
There was a problem hiding this comment.
Correct me if I'm wrong but doesn't this leak the first pages when the result is passed to .free()?
There was a problem hiding this comment.
Any pages we'd mapped that are below our new alignment address are unmapped already not 6 lines prior. As far as I can tell this is working as expected.
| fn testAllocatorLargeAlignment(allocator: &mem.Allocator) mem.Allocator.Error!void { | ||
| //Maybe a platform's page_size is actually the same as or | ||
| // very near usize? | ||
| if(os.page_size << 2 > @maxValue(usize)) return; |
There was a problem hiding this comment.
Isn't this logically always false if os.page_size == @maxValue(usize)? As well, this check is the kind of hypothetical I wouldn't worry about until it becomes an actual problem.
There was a problem hiding this comment.
Since everything involved is a literal, I don't believe there is any overflow to worry about. However, @MaxValue(usize) really should be (1 << usize.bit_count). Of course this all assumes po2 page size, but I don't think that's worth worrying about until LLVM supports a ternary native architecture.
|
Thanks! Out of curiosity, when do you need an alignment greater than page size? |
Added support to DirectAllocator for alignments larger than the page size on posix. This is implemented by using mmap to acquire enough address space to ensure we can align the specified size, then calling munmap on any unused pages.