Merge pull request #3032 from laytan/runtime-arena-edge-cases

Runtime arena edge cases

core/mem/virtual/arena.odin

@@ -98,15 +98,15 @@ arena_alloc :: proc(arena: ^Arena, size: uint, alignment: uint, loc := #caller_l
 
 	switch arena.kind {
 	case .Growing:
-		if arena.curr_block == nil || (safe_add(arena.curr_block.used, size) or_else 0) > arena.curr_block.reserved {
-			size = mem.align_forward_uint(size, alignment)
+		needed := mem.align_forward_uint(size, alignment)
+		if arena.curr_block == nil || (safe_add(arena.curr_block.used, needed) or_else 0) > arena.curr_block.reserved {
 			if arena.minimum_block_size == 0 {
 				arena.minimum_block_size = DEFAULT_ARENA_GROWING_MINIMUM_BLOCK_SIZE
 			}
 
-			block_size := max(size, arena.minimum_block_size)
+			block_size := max(needed, arena.minimum_block_size)
 
-			new_block := memory_block_alloc(size, block_size, {}) or_return
+			new_block := memory_block_alloc(needed, block_size, alignment, {}) or_return
 			new_block.prev = arena.curr_block
 			arena.curr_block = new_block
 			arena.total_reserved += new_block.reserved

core/mem/virtual/virtual.odin

@@ -68,7 +68,7 @@ align_formula :: #force_inline proc "contextless" (size, align: uint) -> uint {
 }
 
 @(require_results)
-memory_block_alloc :: proc(committed, reserved: uint, flags: Memory_Block_Flags) -> (block: ^Memory_Block, err: Allocator_Error) {
+memory_block_alloc :: proc(committed, reserved: uint, alignment: uint = 0, flags: Memory_Block_Flags = {}) -> (block: ^Memory_Block, err: Allocator_Error) {
 	page_size := DEFAULT_PAGE_SIZE
 	assert(mem.is_power_of_two(uintptr(page_size)))
 
@@ -79,8 +79,8 @@ memory_block_alloc :: proc(committed, reserved: uint, flags: Memory_Block_Flags)
 	reserved  = align_formula(reserved, page_size)
 	committed = clamp(committed, 0, reserved)
 	
-	total_size     := uint(reserved + size_of(Platform_Memory_Block))
-	base_offset    := uintptr(size_of(Platform_Memory_Block))
+	total_size     := uint(reserved + max(alignment, size_of(Platform_Memory_Block)))
+	base_offset    := uintptr(max(alignment, size_of(Platform_Memory_Block)))
 	protect_offset := uintptr(0)
 	
 	do_protection := false
@@ -183,4 +183,4 @@ memory_block_dealloc :: proc(block_to_free: ^Memory_Block) {
 safe_add :: #force_inline proc "contextless" (x, y: uint) -> (uint, bool) {
 	z, did_overflow := intrinsics.overflow_add(x, y)
 	return z, !did_overflow
-}
+}

core/runtime/default_allocators_arena.odin

@@ -28,11 +28,11 @@ safe_add :: #force_inline proc "contextless" (x, y: uint) -> (uint, bool) {
 }
 
 @(require_results)
-memory_block_alloc :: proc(allocator: Allocator, capacity: uint, loc := #caller_location) -> (block: ^Memory_Block, err: Allocator_Error) {
-	total_size := uint(capacity + size_of(Memory_Block))
-	base_offset    := uintptr(size_of(Memory_Block))
+memory_block_alloc :: proc(allocator: Allocator, capacity: uint, alignment: uint, loc := #caller_location) -> (block: ^Memory_Block, err: Allocator_Error) {
+	total_size  := uint(capacity + max(alignment, size_of(Memory_Block)))
+	base_offset := uintptr(max(alignment, size_of(Memory_Block)))
 
-	min_alignment: int = max(16, align_of(Memory_Block))
+	min_alignment: int = max(16, align_of(Memory_Block), int(alignment))
 	data := mem_alloc(int(total_size), min_alignment, allocator, loc) or_return
 	block = (^Memory_Block)(raw_data(data))
 	end := uintptr(raw_data(data)[len(data):])
@@ -102,20 +102,20 @@ arena_alloc :: proc(arena: ^Arena, size, alignment: uint, loc := #caller_locatio
 	if size == 0 {
 		return
 	}
-
-	if arena.curr_block == nil || (safe_add(arena.curr_block.used, size) or_else 0) > arena.curr_block.capacity {
-		size = align_forward_uint(size, alignment)
+	
+	needed := align_forward_uint(size, alignment)
+	if arena.curr_block == nil || (safe_add(arena.curr_block.used, needed) or_else 0) > arena.curr_block.capacity {
 		if arena.minimum_block_size == 0 {
 			arena.minimum_block_size = DEFAULT_ARENA_GROWING_MINIMUM_BLOCK_SIZE
 		}
 
-		block_size := max(size, arena.minimum_block_size)
+		block_size := max(needed, arena.minimum_block_size)
 
 		if arena.backing_allocator.procedure == nil {
 			arena.backing_allocator = default_allocator()
 		}
 
-		new_block := memory_block_alloc(arena.backing_allocator, block_size, loc) or_return
+		new_block := memory_block_alloc(arena.backing_allocator, block_size, alignment, loc) or_return
 		new_block.prev = arena.curr_block
 		arena.curr_block = new_block
 		arena.total_capacity += new_block.capacity
@@ -134,7 +134,7 @@ arena_init :: proc(arena: ^Arena, size: uint, backing_allocator: Allocator, loc
 	arena^ = {}
 	arena.backing_allocator = backing_allocator
 	arena.minimum_block_size = max(size, 1<<12) // minimum block size of 4 KiB
-	new_block := memory_block_alloc(arena.backing_allocator, arena.minimum_block_size, loc) or_return
+	new_block := memory_block_alloc(arena.backing_allocator, arena.minimum_block_size, 0, loc) or_return
 	arena.curr_block = new_block
 	arena.total_capacity += new_block.capacity
 	return nil

tests/core/Makefile

@@ -20,7 +20,8 @@ all: c_libc_test \
 	 reflect_test \
 	 slice_test \
 	 strings_test \
-	 thread_test
+	 thread_test \
+	 runtime_test
 
 download_test_assets:
 	$(PYTHON) download_assets.py
@@ -84,3 +85,6 @@ fmt_test:
 
 thread_test:
 	$(ODIN) run thread -out:test_core_thread
+
+runtime_test:
+	$(ODIN) run runtime -out:test_core_runtime

tests/core/build.bat

@@ -95,3 +95,8 @@ echo ---
 echo Running core:thread tests
 echo ---
 %PATH_TO_ODIN% run thread %COMMON% %COLLECTION% -out:test_core_thread.exe || exit /b
+
+echo ---
+echo Running core:runtime tests
+echo ---
+%PATH_TO_ODIN% run runtime %COMMON% %COLLECTION% -out:test_core_runtime.exe || exit /b

tests/core/runtime/test_core_runtime.odin

@@ -0,0 +1,72 @@
+package test_core_runtime
+
+import "core:fmt"
+import "core:intrinsics"
+import "core:mem"
+import "core:os"
+import "core:reflect"
+import "core:runtime"
+import "core:testing"
+
+TEST_count := 0
+TEST_fail  := 0
+
+when ODIN_TEST {
+	expect_value :: testing.expect_value
+} else {
+	expect_value :: proc(t: ^testing.T, value, expected: $T, loc := #caller_location) -> bool where intrinsics.type_is_comparable(T) {
+		TEST_count += 1
+		ok := value == expected || reflect.is_nil(value) && reflect.is_nil(expected)
+		if !ok {
+			TEST_fail += 1
+			fmt.printf("[%v] expected %v, got %v\n", loc, expected, value)
+		}
+		return ok
+	}
+}
+
+main :: proc() {
+	t := testing.T{}
+
+	test_temp_allocator_big_alloc_and_alignment(&t)
+	test_temp_allocator_alignment_boundary(&t)
+	test_temp_allocator_returns_correct_size(&t)
+
+	fmt.printf("%v/%v tests successful.\n", TEST_count - TEST_fail, TEST_count)
+	if TEST_fail > 0 {
+		os.exit(1)
+	}
+}
+
+// Tests that having space for the allocation, but not for the allocation and alignment
+// is handled correctly.
+@(test)
+test_temp_allocator_alignment_boundary :: proc(t: ^testing.T) {
+	arena: runtime.Arena
+	context.allocator = runtime.arena_allocator(&arena)
+
+	_, _ = mem.alloc(int(runtime.DEFAULT_ARENA_GROWING_MINIMUM_BLOCK_SIZE)-120)
+	_, err := mem.alloc(112, 32)
+	expect_value(t, err, nil)
+}
+
+// Tests that big allocations with big alignments are handled correctly.
+@(test)
+test_temp_allocator_big_alloc_and_alignment :: proc(t: ^testing.T) {
+	arena: runtime.Arena
+	context.allocator = runtime.arena_allocator(&arena)
+
+	mappy: map[[8]int]int
+	err := reserve(&mappy, 50000)
+	expect_value(t, err, nil)
+}
+
+@(test)
+test_temp_allocator_returns_correct_size :: proc(t: ^testing.T) {
+	arena: runtime.Arena
+	context.allocator = runtime.arena_allocator(&arena)
+
+	bytes, err := mem.alloc_bytes(10, 16)
+	expect_value(t, err, nil)
+	expect_value(t, len(bytes), 10)
+}