Browse Source

Fix bug with allocator not getting set on a `map`

gingerBill 2 years ago
parent
commit
366779f8c7
3 changed files with 31 additions and 51 deletions
  1. 4 9
      core/mem/allocators.odin
  2. 5 11
      core/runtime/core_builtin.odin
  3. 22 31
      core/runtime/dynamic_map_internal.odin

+ 4 - 9
core/mem/allocators.odin

@@ -880,7 +880,7 @@ tracking_allocator :: proc(data: ^Tracking_Allocator) -> Allocator {
 
 tracking_allocator_proc :: proc(allocator_data: rawptr, mode: Allocator_Mode,
                                 size, alignment: int,
-                                old_memory: rawptr, old_size: int, loc := #caller_location) -> ([]byte, Allocator_Error) {
+                                old_memory: rawptr, old_size: int, loc := #caller_location) -> (result: []byte, err: Allocator_Error) {
 	data := (^Tracking_Allocator)(allocator_data)
 	if mode == .Query_Info {
 		info := (^Allocator_Query_Info)(old_memory)
@@ -892,21 +892,16 @@ tracking_allocator_proc :: proc(allocator_data: rawptr, mode: Allocator_Mode,
 			info.pointer = nil
 		}
 
-		return nil, nil
+		return
 	}
 
-	result: []byte
-	err: Allocator_Error
 	if mode == .Free && old_memory != nil && old_memory not_in data.allocation_map {
 		append(&data.bad_free_array, Tracking_Allocator_Bad_Free_Entry{
 			memory = old_memory,
 			location = loc,
 		})
 	} else {
-		result, err = data.backing.procedure(data.backing.data, mode, size, alignment, old_memory, old_size, loc)
-		if err != nil {
-			return result, err
-		}
+		result = data.backing.procedure(data.backing.data, mode, size, alignment, old_memory, old_size, loc) or_return
 	}
 	result_ptr := raw_data(result)
 
@@ -954,6 +949,6 @@ tracking_allocator_proc :: proc(allocator_data: rawptr, mode: Allocator_Mode,
 		unreachable()
 	}
 
-	return result, err
+	return
 }
 

+ 5 - 11
core/runtime/core_builtin.odin

@@ -231,7 +231,7 @@ make_dynamic_array_len_cap :: proc($T: typeid/[dynamic]$E, #any_int len: int, #a
 	return
 }
 @(builtin)
-make_map :: proc($T: typeid/map[$K]$E, #any_int cap: int = DEFAULT_RESERVE_CAPACITY, allocator := context.allocator, loc := #caller_location) -> T {
+make_map :: proc($T: typeid/map[$K]$E, #any_int cap: int = 1<<MAP_MIN_LOG2_CAPACITY, allocator := context.allocator, loc := #caller_location) -> T {
 	make_map_expr_error_loc(loc, cap)
 	context.allocator = allocator
 
@@ -283,19 +283,13 @@ reserve_map :: proc(m: ^$T/map[$K]$V, capacity: int, loc := #caller_location) {
 }
 
 /*
-	Shrinks the capacity of a map down to the current length, or the given capacity.
-
-	If `new_cap` is negative, then `len(m)` is used.
-
-	Returns false if `cap(m) < new_cap`, or the allocator report failure.
-
-	If `len(m) < new_cap`, then `len(m)` will be left unchanged.
+	Shrinks the capacity of a map down to the current length.
 */
 @builtin
-shrink_map :: proc(m: ^$T/map[$K]$V, new_cap := -1, loc := #caller_location) -> (did_shrink: bool) {
+shrink_map :: proc(m: ^$T/map[$K]$V, loc := #caller_location) -> (did_shrink: bool) {
 	if m != nil {
-		new_cap := new_cap if new_cap >= 0 else len(m)
-		return __dynamic_map_shrink(__get_map_header(m), new_cap, loc)
+		err := map_shrink_dynamic((^Raw_Map)(m), map_info(T), loc)
+		did_shrink = err == nil
 	}
 	return
 }

+ 22 - 31
core/runtime/dynamic_map_internal.odin

@@ -316,9 +316,9 @@ map_total_allocation_size :: #force_inline proc "contextless" (capacity: uintptr
 
 // The only procedure which needs access to the context is the one which allocates the map.
 map_alloc_dynamic :: proc "odin" (info: ^Map_Info, log2_capacity: uintptr, allocator := context.allocator, loc := #caller_location) -> (result: Raw_Map, err: Allocator_Error) {
+	result.allocator = allocator // set the allocator always
 	if log2_capacity == 0 {
-		// Empty map, but set the allocator.
-		return { 0, 0, allocator }, nil
+		return
 	}
 
 	if log2_capacity >= 64 {
@@ -337,12 +337,8 @@ map_alloc_dynamic :: proc "odin" (info: ^Map_Info, log2_capacity: uintptr, alloc
 	if intrinsics.expect(data_ptr & CACHE_MASK != 0, false) {
 		panic("allocation not aligned to a cache line", loc)
 	} else {
-		result = {
-			// Tagged pointer representation for capacity.
-			data_ptr | log2_capacity,
-			0,
-			allocator,
-		}
+		result.data = data_ptr | log2_capacity // Tagged pointer representation for capacity.
+		result.len = 0
 
 		map_clear_dynamic(&result, info)
 	}
@@ -513,20 +509,19 @@ map_add_hash_dynamic :: proc "odin" (m: Raw_Map, #no_alias info: ^Map_Info, h: M
 
 @(optimization_mode="size")
 map_grow_dynamic :: proc "odin" (#no_alias m: ^Raw_Map, #no_alias info: ^Map_Info, loc := #caller_location) -> Allocator_Error {
-	allocator := m.allocator
-	if allocator.procedure == nil {
-		allocator = context.allocator
+	if m.allocator.procedure == nil {
+		m.allocator = context.allocator
 	}
 
 	log2_capacity := map_log2_cap(m^)
 
 	if m.data == 0 {
-		n := map_alloc_dynamic(info, MAP_MIN_LOG2_CAPACITY, allocator, loc) or_return
+		n := map_alloc_dynamic(info, MAP_MIN_LOG2_CAPACITY, m.allocator, loc) or_return
 		m.data = n.data
 		return nil
 	}
 
-	resized := map_alloc_dynamic(info, log2_capacity + 1, allocator, loc) or_return
+	resized := map_alloc_dynamic(info, log2_capacity + 1, m.allocator, loc) or_return
 
 	old_capacity := uintptr(1) << log2_capacity
 
@@ -554,7 +549,7 @@ map_grow_dynamic :: proc "odin" (#no_alias m: ^Raw_Map, #no_alias info: ^Map_Inf
 	}
 	map_free_dynamic(m^, info, loc) or_return
 
-	m.data = resized.data // Should copy the capacity too
+	m^ = resized
 
 	return nil
 }
@@ -562,9 +557,8 @@ map_grow_dynamic :: proc "odin" (#no_alias m: ^Raw_Map, #no_alias info: ^Map_Inf
 
 @(optimization_mode="size")
 map_reserve_dynamic :: proc "odin" (#no_alias m: ^Raw_Map, #no_alias info: ^Map_Info, new_capacity: uintptr, loc := #caller_location) -> Allocator_Error {
-	allocator := m.allocator
-	if allocator.procedure == nil {
-		allocator = context.allocator
+	if m.allocator.procedure == nil {
+		m.allocator = context.allocator
 	}
 
 	new_capacity := new_capacity
@@ -580,11 +574,11 @@ map_reserve_dynamic :: proc "odin" (#no_alias m: ^Raw_Map, #no_alias info: ^Map_
 	log2_new_capacity := size_of(uintptr) - intrinsics.count_leading_zeros(new_capacity-1)
 
 	if m.data == 0 {
-		m^ = map_alloc_dynamic(info, MAP_MIN_LOG2_CAPACITY, allocator, loc) or_return
+		m^ = map_alloc_dynamic(info, MAP_MIN_LOG2_CAPACITY, m.allocator, loc) or_return
 		return nil
 	}
 
-	resized := map_alloc_dynamic(info, log2_new_capacity, allocator, loc) or_return
+	resized := map_alloc_dynamic(info, log2_new_capacity, m.allocator, loc) or_return
 
 	ks, vs, hs, _, _ := map_kvh_data_dynamic(m^, info)
 
@@ -609,7 +603,7 @@ map_reserve_dynamic :: proc "odin" (#no_alias m: ^Raw_Map, #no_alias info: ^Map_
 		}
 	}
 
-	mem_free(rawptr(ks), allocator)
+	map_free_dynamic(m^, info, loc) or_return
 
 	m^ = resized // Should copy the capacity too
 
@@ -618,11 +612,9 @@ map_reserve_dynamic :: proc "odin" (#no_alias m: ^Raw_Map, #no_alias info: ^Map_
 
 
 @(optimization_mode="size")
-map_shrink_dynamic :: proc "odin" (#no_alias m: ^Raw_Map, #no_alias info: ^Map_Info) -> Allocator_Error {
-	allocator := m.allocator
-	if allocator.procedure == nil {
-		// TODO(bill): is this correct behaviour?
-		allocator = context.allocator
+map_shrink_dynamic :: proc "odin" (#no_alias m: ^Raw_Map, #no_alias info: ^Map_Info, loc := #caller_location) -> Allocator_Error {
+	if m.allocator.procedure == nil {
+		m.allocator = context.allocator
 	}
 
 	// Cannot shrink the capacity if the number of items in the map would exceed
@@ -633,7 +625,7 @@ map_shrink_dynamic :: proc "odin" (#no_alias m: ^Raw_Map, #no_alias info: ^Map_I
 		return nil
 	}
 
-	shrinked := map_alloc_dynamic(info, log2_capacity - 1, allocator) or_return
+	shrinked := map_alloc_dynamic(info, log2_capacity - 1, m.allocator) or_return
 
 	capacity := uintptr(1) << log2_capacity
 
@@ -662,9 +654,9 @@ map_shrink_dynamic :: proc "odin" (#no_alias m: ^Raw_Map, #no_alias info: ^Map_I
 		}
 	}
 
-	mem_free(rawptr(ks), allocator)
+	map_free_dynamic(m^, info, loc) or_return
 
-	m.data = shrinked.data // Should copy the capacity too
+	m^ = shrinked
 
 	return nil
 }
@@ -672,9 +664,8 @@ map_shrink_dynamic :: proc "odin" (#no_alias m: ^Raw_Map, #no_alias info: ^Map_I
 @(require_results)
 map_free_dynamic :: proc "odin" (m: Raw_Map, info: ^Map_Info, loc := #caller_location) -> Allocator_Error {
 	ptr := rawptr(map_data(m))
-	size := map_total_allocation_size(uintptr(map_cap(m)), info)
-
-	return mem_free_with_size(ptr, int(size), m.allocator, loc)
+	size := int(map_total_allocation_size(uintptr(map_cap(m)), info))
+	return mem_free_with_size(ptr, size, m.allocator, loc)
 }
 
 @(optimization_mode="speed")