Heavy refactor as last ditch effort to fix problems

allocator_internal.h

@@ -72,8 +72,8 @@ void rotate_right(TreeAlloc **root_ptr, TreeAlloc *ta);
 void repair_tree_after_insert(TreeAlloc **root_ptr, TreeAlloc *ta);
 void remove_node(TreeAlloc** root_ptr, TreeAlloc* node);
 void insert_singleton(TreeAlloc **root_ptr, TreeAlloc *to_insert);
-void insert_right(TreeAlloc** root_ptr, TreeAlloc* to_insert, TreeAlloc* after);
-void insert_left(TreeAlloc** root_ptr, TreeAlloc* to_insert, TreeAlloc* before);
+void insert_by_size(TreeAlloc** root_ptr, TreeAlloc* to_insert);
+void insert_by_addr(TreeAlloc** root_ptr, TreeAlloc* to_insert);
 void unalloc(Arena *arena, void *addr);
 void *alloc(Arena *arena, uintptr_t size, uintptr_t align);
 

main.c

@@ -8,7 +8,7 @@
 
 #include "alloc_api.h"
 
-void *get_new_region(uintptr_t size) {
+void *main_get_new_region(uintptr_t size) {
   void *m = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
   if (m == MAP_FAILED) {
     return NULL;
@@ -103,7 +103,7 @@ int act(struct AllocationRecord *dummy, struct Arena *arena) {
   struct AllocationRecord *head = dummy->next;
   while (head != dummy) {
     if (!validate_record(head)) {
-      printf("validation failed at id %i\n", head->id);
+      //printf("validation failed at id %i\n", head->id);
       die = 1;
     }
     head = head->next;
@@ -116,7 +116,7 @@ int main() {
   struct Arena arena = {
     NULL,
     NULL,
-    get_new_region,
+    main_get_new_region,
     error,
   };
   void *reg = alloc(&arena, 20, 4);
@@ -129,7 +129,7 @@ int main() {
   };
   dummy.next = &dummy;
   dummy.prev = &dummy;
-  for (int ii = 0; ii < 4000; ii++) {
+  for (int ii = 0; ii < 100; ii++) {
     act(&dummy, &arena);
   }
 	return 0;

tree_alloc.c

@@ -4,12 +4,12 @@
 
 #include "allocator_internal.h"
 
-#ifdef DEBUG
-#include <stdio.h>
-
 #define IS_BLACK_NODE(n) (n == NULL || n->color == COLOR_BLACK)
 #define IS_RED_NODE(n) (n != NULL && n->color == COLOR_RED)
 
+#ifdef DEBUG
+#include <stdio.h>
+
 int debug_tree_black_height(TreeAlloc *node) {
 	if (node == NULL) {
 		return 1;
@@ -45,9 +45,9 @@ void debug_print_tree(int indent, void *p, int safe) {
 	if (node != NULL) {
 		int bad = debug_tree_black_height(node->left) != debug_tree_black_height(node->right);
 		bad |= IS_RED_NODE(node) && (
-			 IS_RED_NODE(node->left) || 
-			 IS_RED_NODE(node->right) || 
-			 IS_RED_NODE(node->parent) );
+				IS_RED_NODE(node->left) || 
+				IS_RED_NODE(node->right) || 
+				IS_RED_NODE(node->parent) );
 		bad &= !safe;
 		debug_print_tree(indent + 1, node->left, safe);
 		debug_print_node(indent, node, bad);
@@ -67,17 +67,30 @@ TreeAlloc *insert_node_at(void *address, uintptr_t padding, uintptr_t align, uin
 TreeAlloc *search_by_address(TreeAlloc *root, void *address) {
 	TreeAlloc *head = root;
 	while (1) {
-		if (head >= (TreeAlloc*) address) {
-			if (head->left == NULL) {
-				return NULL;
-			} else {
+		void *region_start = head;
+		void *region_end = head + head->size;
+		if (address < region_start) {
+			// The requested address is before this region's start.
+			if (head->left) {
+				// There is another region that comes before this one
+				// in memory.
 				head = head->left;
+			} else {
+				// This address is before any of the allocated regions.
+				return NULL;
 			}
+		} else if (address <= region_end) {
+			// The requested address is within the range of this region.
+			return head;
 		} else {
-			if (head->right == NULL || head->right > (TreeAlloc*) address) {
-				return head;
-			} else {
+			// The requested address is after this region's end.
+			if (head->right) {
+				// There is another region that comes after this one
+				// in memory.
 				head = head->right;
+			} else {
+				// This address is after any of the allocated regions.
+				return NULL;
 			}
 		}
 	}
@@ -263,27 +276,34 @@ void insert_singleton(TreeAlloc **root_ptr, TreeAlloc *to_insert) {
 #endif
 }
 
-void insert_right(TreeAlloc** root_ptr, TreeAlloc* to_insert, TreeAlloc* after) {
+void insert_by_size(TreeAlloc** root_ptr, TreeAlloc* to_insert) {
 #ifdef DEBUG
-	printf("=== PRE-INSERT-RIGHT ===\n");
+	printf("=== PRE-INSERT-BY-SIZE ===\n");
 	printf("===== INSERTING =====\n");
 	debug_print_node(0, to_insert, 0);
-	printf("\tafter\n");
-	debug_print_node(0, after, 0);
 	printf("===== CURRENT TREE =====\n");
 	debug_print_tree(0, *root_ptr, 0);
 	printf("===== END OF TREES =====\n");
 #endif
-	if (after->right != NULL) {
-		after = after->right;
-		while (after->left != NULL) {
-			after = after->left;
+	TreeAlloc *tree_ptr = *root_ptr;
+	while (1) {
+		if (to_insert->size < tree_ptr->size) {
+			if (tree_ptr->left) {
+				tree_ptr = tree_ptr->left;
+			} else {
+				tree_ptr->left = to_insert;
+				to_insert->parent = tree_ptr;
+				break;
+			}
+		} else {
+			if (tree_ptr->right) {
+				tree_ptr = tree_ptr->right;
+			} else {
+				tree_ptr->right = to_insert;
+				to_insert->parent = tree_ptr;
+				break;
+			}
 		}
-		after->left = to_insert;
-		to_insert->parent = after;
-	} else {
-		after->right = to_insert;
-		to_insert->parent = after;
 	}
 	to_insert->color = COLOR_RED;
 	repair_tree_after_insert(root_ptr, to_insert);
@@ -295,30 +315,34 @@ void insert_right(TreeAlloc** root_ptr, TreeAlloc* to_insert, TreeAlloc* after)
 #endif
 }
 
-void insert_left(TreeAlloc** root_ptr, TreeAlloc* to_insert, TreeAlloc* before) {
+void insert_by_addr(TreeAlloc** root_ptr, TreeAlloc* to_insert) {
 #ifdef DEBUG
-	printf("=== PRE-INSERT-LEFT ====\n");
+	printf("=== PRE-INSERT-BY-ADDR ===\n");
 	printf("===== INSERTING =====\n");
 	debug_print_tree(0, to_insert, 0);
-	printf("\tbefore\n");
-	debug_print_node(0, before, 0);
 	printf("===== CURRENT TREE =====\n");
 	debug_print_tree(0, *root_ptr, 0);
 	printf("===== END OF TREES =====\n");
 #endif
-	if (!before)
-		before = *root_ptr;
-
-	if (before->left != NULL) {
-		before = before->left;
-		while (before->right != NULL) {
-			before = before->right;
+	TreeAlloc *tree_ptr = *root_ptr;
+	while (1) {
+		if (to_insert < tree_ptr) {
+			if (tree_ptr->left) {
+				tree_ptr = tree_ptr->left;
+			} else {
+				tree_ptr->left = to_insert;
+				to_insert->parent = tree_ptr;
+				break;
+			}
+		} else {
+			if (tree_ptr->right) {
+				tree_ptr = tree_ptr->right;
+			} else {
+				tree_ptr->right = to_insert;
+				to_insert->parent = tree_ptr;
+				break;
+			}
 		}
-		before->right = to_insert;
-		to_insert->parent = before;
-	} else {
-		before->left = to_insert;
-		to_insert->parent = before;
 	}
 	to_insert->color = COLOR_RED;
 	repair_tree_after_insert(root_ptr, to_insert);
@@ -471,40 +495,26 @@ void remove_node(TreeAlloc **root_ptr, TreeAlloc *to_remove) {
 #endif
 }
 
-int add_new_region(Arena *arena, uintptr_t size, uintptr_t padding, uintptr_t align) {
+TreeAlloc *get_new_region(Arena *arena, uintptr_t size, uintptr_t padding, uintptr_t align) {
 	uintptr_t realsize = size + align + alignof(WatermarkAlloc) + padding - 1;
+#ifdef DEBUG
+	printf("Attemping request of size %ld\n", realsize);
+#endif 
 	if (realsize < MIN_NEW_MEM_SIZE) {
 		realsize = MIN_NEW_MEM_SIZE;
 	}
-	FreeSpace *reg = (FreeSpace*) arena->get_new_region(realsize);
+	TreeAlloc *reg = (TreeAlloc *) arena->get_new_region(realsize);
 	if (reg == NULL) {
 		arena->error("can't allocate a new memory region!");
-		return 0;
-	}
-	FreeSpace *newreg = align_after(reg, alignof(WatermarkAlloc));
-	newreg->left = NULL;
-	newreg->right = NULL;
-	realsize -= (void*) newreg - (void*) reg;
-	realsize -= realsize % alignof(WatermarkAlloc);
-	newreg->size = realsize;
-	if (arena->root_freespace == NULL) {
-		insert_singleton((TreeAlloc**) &arena->root_freespace, (TreeAlloc*) newreg);
 	} else {
-		FreeSpace *head = arena->root_freespace;
-		while (head->right != NULL) {
-			head = head->right;
-		}
-		insert_right((TreeAlloc**) &arena->root_freespace, (TreeAlloc*) newreg, (TreeAlloc*) head);
+		reg->parent = NULL;
+		reg->left = NULL;
+		reg->right = NULL;
+		reg->before = NULL;
+		reg->after = NULL;
+		reg->size = realsize;
 	}
-#ifdef DEBUG
-	printf("= POST-REGION-CREATION =\n");
-	printf("==== FREESPACE TREE ====\n");
-	debug_print_tree(0, arena->root_freespace, 0);
-	printf("==== TREEALLOC TREE ====\n");
-	debug_print_tree(0, arena->root_treealloc, 0);
-	printf("===== END OF TREES =====\n");
-#endif
-	return 1;
+	return reg;
 }
 
 void unalloc(Arena *arena, void *addr) {
@@ -554,19 +564,16 @@ void unalloc(Arena *arena, void *addr) {
 	} else {
 		TreeAlloc *insert_point = search_by_size((TreeAlloc*) arena->root_freespace, 0, 1, size);
 		if (insert_point == NULL) {
-			TreeAlloc *head = (TreeAlloc*) arena->root_freespace;
-			while (head->right != NULL) {
-				head = head->right;
-			}
-			insert_right((TreeAlloc**) &arena->root_freespace, (TreeAlloc*) start, head);
+			insert_by_size((TreeAlloc**) &arena->root_freespace, (TreeAlloc*) start);
 		} else {
-			insert_left((TreeAlloc**) &arena->root_freespace, (TreeAlloc*) start, insert_point);
+			insert_by_size((TreeAlloc**) &arena->root_freespace, (TreeAlloc*) start);
 		}
 	}
 }
 
 void *alloc(Arena *arena, uintptr_t size, uintptr_t align) {
 	uintptr_t actual_align = lcm(alignof(struct WatermarkAlloc), align);
+
 #ifdef DEBUG
 	printf("==== ALLOCATING =====\n");
 	printf("=== FREESPACE TREE ===\n");
@@ -575,106 +582,83 @@ void *alloc(Arena *arena, uintptr_t size, uintptr_t align) {
 	debug_print_tree(0, arena->root_treealloc, 0);
 	printf("==== END OF TREES ====\n");
 #endif
+
+	TreeAlloc *region;
 	if (arena->root_freespace == NULL) {
 		// Handle being out of freespace.
 #ifdef DEBUG
 		printf("Out of freespace nodes; getting more\n");
 #endif
-		if (!add_new_region(arena, size, sizeof(TreeAlloc), actual_align)) {
-			return NULL;
-		}
-		return alloc(arena, size, align);
+		region = get_new_region(arena, size, sizeof(TreeAlloc), actual_align);
 	} else {
-		TreeAlloc *region = search_by_size((TreeAlloc*) arena->root_freespace, sizeof(TreeAlloc), actual_align, size);
+		region = search_by_size((TreeAlloc*) arena->root_freespace, sizeof(TreeAlloc), actual_align, size);
 		if (region == NULL) {
 			// Handle insufficient freespace or fragmentation.
 #ifdef DEBUG
 			printf("Out of sufficiently large freespace nodes; getting more\n");
 #endif
-			if (!add_new_region(arena, size, sizeof(TreeAlloc), actual_align)) {
-				return NULL;
-			}
-			return alloc(arena, size, align);
-		}
-		remove_node((TreeAlloc**) &arena->root_freespace, region);
-		void *true_end = align_after(align_after(((void*) region) + sizeof(TreeAlloc), actual_align) + size, alignof(WatermarkAlloc));
-		// The size of the new allocation (adjusted for region header and alignment
-		uintptr_t new_size = true_end - (void*) region;
-		// The size of the free space region following the new allocation
-		uintptr_t new_free_size = region->size - new_size;
-		region->right = NULL;
-		region->left = NULL;
-		region->type = RT_TREE_NODE;
-#ifdef DEBUG
-		printf("start: %p, end: %p, adjusted end: %p\n", region, ((void*) region) + size, true_end);
-		printf("size: %lu -> %lu\n", size, new_size);
-		printf("new_free_size: %lu\n", new_free_size);
-#endif
-		if (arena->root_treealloc == NULL) {
-			insert_singleton((TreeAlloc**) &arena->root_treealloc, region);
+			region = get_new_region(arena, size, sizeof(TreeAlloc), actual_align);
 		} else {
+			remove_node((TreeAlloc**) &arena->root_freespace, region);
+		}
+	}
+
+	void *true_end = align_after(align_after(((void*) region) + sizeof(TreeAlloc), actual_align) + size, alignof(WatermarkAlloc));
+	// The size of the new allocation (adjusted for region header and alignment
+	uintptr_t new_size = true_end - (void*) region;
+	// The size of the free space region following the new allocation
+	uintptr_t new_free_size = region->size - new_size;
+	region->right = NULL;
+	region->left = NULL;
+	region->type = RT_TREE_NODE;
+	region->size = size;
+
 #ifdef DEBUG
-			printf("searching for an insert point\n");
-#endif
-			TreeAlloc *insert_point = search_by_address((TreeAlloc*) arena->root_treealloc, region);
-			if (insert_point == NULL) {
-				TreeAlloc *head = arena->root_treealloc;
-				while (head->left != NULL) {
-					head = head->left;
-				}
-#ifdef DEBUG
-				printf("none found; inserting before %p\n", head);
-#endif
-				insert_left(&arena->root_treealloc, region, head);
-			} else {
-#ifdef DEBUG
-				printf("found one: %p\n", insert_point);
+	printf("start: %p, end: %p, adjusted end: %p\n", region, ((void*) region) + size, true_end);
+	printf("size: %lu -> %lu\n", size, new_size);
+	printf("new_free_size: %lu\n", new_free_size);
 #endif
-				insert_right(&arena->root_treealloc, region, insert_point);
-			}
-		}
-		if (region->size >= new_size + sizeof(FreeSpace)) {
-			// If there's enough free space after the allocation, use it!
-			region->size = new_size;  // Safe because the allocated region tree is not sorted by size.
-			FreeSpace *new_free = (FreeSpace*) ((void*) region + new_size);
-			new_free->left = NULL;
-			new_free->right = NULL;
-			new_free->parent = NULL;
-			new_free->type = RT_FREESPACE;
-			new_free->size = new_free_size;
-			if (arena->root_freespace == NULL) {
-				insert_singleton((TreeAlloc**) &arena->root_freespace, (TreeAlloc*) new_free);
-			} else {
-				FreeSpace *insert_point = (FreeSpace*) search_by_size((TreeAlloc*) arena->root_freespace, 0, 1, new_free_size);
-				insert_left((TreeAlloc**) &arena->root_freespace, (TreeAlloc*) new_free, (TreeAlloc*) insert_point);
-			}
-			// Set the region following this one to be the new free space
-			region->after = (TreeAlloc*) new_free;
-		} else {
-			// There isn't a free space after this one, so put the `next` pointer at the next allocated
-			// region.
-			region->after = succ(region);
-		}
-		// I seem to have forgotten about the fact that memory may not be contiguous
-		if (region->after != NULL && region->after != (void*) region + region->size) {
-			region->after = NULL;
-		}
-		// Also make sure the `before` pointer is correct.
-		TreeAlloc *before_alloc = pred(region);
-		if (before_alloc == NULL || ((void*) before_alloc) + before_alloc->size < (void*) region) {
-			region->before = search_by_address((TreeAlloc*) arena->root_freespace, region);
+
+	if (arena->root_treealloc == NULL) {
+		insert_singleton((TreeAlloc**) &arena->root_treealloc, region);
+	} else {
+		insert_by_addr(&arena->root_treealloc, region);
+	}
+	if (region->size >= new_size + sizeof(FreeSpace)) {
+		// If there's enough free space after the allocation, use it!
+		region->size = new_size;  // Safe because the allocated region tree is not sorted by size.
+		FreeSpace *new_free = (FreeSpace*) ((void*) region + new_size);
+		new_free->left = NULL;
+		new_free->right = NULL;
+		new_free->parent = NULL;
+		new_free->type = RT_FREESPACE;
+		new_free->size = new_free_size;
+		if (arena->root_freespace == NULL) {
+			insert_singleton((TreeAlloc**) &arena->root_freespace, (TreeAlloc*) new_free);
 		} else {
-			region->before = before_alloc;
-		}
-		// I seem to have forgotten about the fact that memory may not be contiguous
-		if (region->before != NULL && region->before != (void*) region->before + region->before->size) {
-			region->before = NULL;
+			insert_by_size((TreeAlloc**) &arena->root_freespace, (TreeAlloc*) new_free);
 		}
+		// Set the region following this one to be the new free space
+		region->after = (TreeAlloc*) new_free;
+	} else {
+		// There isn't a free space after this one, so put the `next` pointer at the next allocated
+		// region if there is one.
+		region->after = search_by_address((TreeAlloc *) &arena->root_treealloc, region + region->size + 1);
+	}
+
+	// Are there any allocations before this one?
+	region->before = search_by_address((TreeAlloc *) &arena->root_treealloc, region - 1);
+
 #ifdef DEBUG
-		printf("region is still at %p\n", region);
+	printf("region is still at %p\n", region);
+	printf("=== POST-ALLOCATION ===\n");
+	printf("=== FREESPACE TREE ===\n");
+	debug_print_tree(0, arena->root_freespace, 0);
+	printf("=== TREEALLOC TREE ===\n");
+	debug_print_tree(0, arena->root_treealloc, 0);
+	printf("==== END OF TREES ====\n");
 #endif
-		return align_after((void*) region + sizeof(TreeAlloc), actual_align);
-	}
+	return align_after((void*) region + sizeof(TreeAlloc), actual_align);
 }
 
 void *alloc_growable(Arena *arena, uintptr_t size, uintptr_t align) {