From c32e0cee2ca24f7b45f67f391798fe482ac15351 Mon Sep 17 00:00:00 2001 From: Patrick Jakobsen Date: Tue, 26 Sep 2023 13:37:09 +0200 Subject: [PATCH] Introduced a level of indirection for the rdic freelist --- rdic.h | 24 ++++++++++++-------- test/rdic_test.c | 57 ++++++++++++++++++++++++++++++------------------ 2 files changed, 51 insertions(+), 30 deletions(-) diff --git a/rdic.h b/rdic.h index 506407b..de8ef09 100644 --- a/rdic.h +++ b/rdic.h @@ -27,9 +27,13 @@ typedef struct { // NOTE(Zelaven): This idea is bad because it assumes that a reference won't be reallocated. Generations are much more reliable. +typedef struct { + RDIC_Node *head; +} RDIC_Freelist; + typedef struct { RDIC_Node *root; - RDIC_Node *node_freelist; + RDIC_Freelist *freelist; RDIC_Node *current_parent; RDIC_Node *frame_current_node; @@ -146,9 +150,9 @@ void rdic_cull_subtrees( } } - current->sibling = context->node_freelist; + current->sibling = context->freelist->head; current->generation += 1; // Invalidate references. - context->node_freelist = current; + context->freelist->head = current; current = next; } @@ -165,17 +169,19 @@ RDIC_Node_Reference rdic_context_start_frame( RDIC_Node_Reference node_reference = last_root; int need_new_node = (last_root.node == NULL) || (last_root.generation != last_root.node->generation); - if(need_new_node && context->node_freelist == NULL) + int can_get_node_from_freelist = + (context->freelist == NULL || context->freelist->head == NULL); + if(need_new_node && can_get_node_from_freelist) { return (RDIC_Node_Reference){0}; } if(need_new_node) { RDIC_Node_Reference result; - result.node = context->node_freelist; + result.node = context->freelist->head; result.generation = result.node->generation; - context->node_freelist = context->node_freelist->sibling; + context->freelist->head = context->freelist->head->sibling; RDIC_Node *root = result.node; root->sibling = NULL; @@ -260,7 +266,7 @@ RDIC_Node_Reference rdic_get_node( reference_is_valid && (context->current_parent == node_reference.node->parent); - if(!reference_is_valid && context->node_freelist == NULL) + if(!reference_is_valid && context->freelist->head == NULL) { if(out_flags != NULL) { @@ -316,9 +322,9 @@ RDIC_Node_Reference rdic_get_node( { // Make new node // NOTE(Zelaven): We guard against empty freelist, so this is safe. RDIC_Node_Reference result = {0}; - result.node = context->node_freelist; + result.node = context->freelist->head; result.generation = result.node->generation; - context->node_freelist = context->node_freelist->sibling; + context->freelist->head = context->freelist->head->sibling; // TODO(Zelaven): Should the various fields be filled out here, or outside? // What will be the strategy to avoid redundant work? diff --git a/test/rdic_test.c b/test/rdic_test.c index 411029c..bff34cd 100644 --- a/test/rdic_test.c +++ b/test/rdic_test.c @@ -48,7 +48,8 @@ void init_node_freelist(GUI_Node *nodes, int node_count) RDIC_Context context = {0};\ GUI_Node freelist_nodes[FREELIST_LENGTH] = {0};\ init_node_freelist(freelist_nodes, ARRAYLENGTH(freelist_nodes));\ - context.node_freelist = &freelist_nodes[0].rn;\ + RDIC_Freelist freelist = {&freelist_nodes[0].rn};\ + context.freelist = &freelist;\ RDIC_Node_Reference root = {0}; @@ -74,7 +75,7 @@ RDIC_Node_Reference test_get_node( // NOTE(Zelaven): Helpers can be buggy too and could in principle have tests. int freelist_contains(RDIC_Context *context, RDIC_Node *node) { - RDIC_Node *current = context->node_freelist; + RDIC_Node *current = context->freelist->head; int result = 0; while(!result && current != NULL) { @@ -211,7 +212,8 @@ void test__start_frame__succeed(void) RDIC_Context context = {0}; GUI_Node freelist_nodes[1] = {0}; init_node_freelist(freelist_nodes, ARRAYLENGTH(freelist_nodes)); - context.node_freelist = &freelist_nodes[0].rn; + RDIC_Freelist freelist = {&freelist_nodes[0].rn}; + context.freelist = &freelist; RDIC_Node_Reference root = {0}; // Act. @@ -229,7 +231,8 @@ void test__finish_frame__succeed(void) RDIC_Context context = {0}; GUI_Node freelist_nodes[1] = {0}; init_node_freelist(freelist_nodes, ARRAYLENGTH(freelist_nodes)); - context.node_freelist = &freelist_nodes[0].rn; + RDIC_Freelist freelist = {&freelist_nodes[0].rn}; + context.freelist = &freelist; RDIC_Node_Reference root = {0}; root = rdic_context_start_frame( &context, @@ -253,20 +256,21 @@ void test__get_node_with_empty_freelist__return_null_reference(void) RDIC_Context context = {0}; GUI_Node freelist_nodes[1] = {0}; init_node_freelist(freelist_nodes, ARRAYLENGTH(freelist_nodes)); - context.node_freelist = &freelist_nodes[0].rn; + RDIC_Freelist freelist = {&freelist_nodes[0].rn}; + context.freelist = &freelist; RDIC_Node_Reference root = {0}; root = rdic_context_start_frame( &context, root); - assert(context.node_freelist == NULL); - // Act. RDIC_Node_Reference node = {0}; // Invalid reference -> new node. node = GET_NODE(&context, node); // Assert. // NOTE(Zelaven): We specifically assert that the reference is all 0. + if(context.freelist->head != NULL) + {TEST_ERROR("Freelist not empty.");} if(node.node != NULL) {TEST_ERROR("Non-NULL node despite empty freelist.");} if(node.generation != 0) @@ -279,7 +283,8 @@ void test__get_node_with_empty_freelist_with_nonzero_generation__return_null_ref RDIC_Context context = {0}; GUI_Node freelist_nodes[1] = {0}; init_node_freelist(freelist_nodes, ARRAYLENGTH(freelist_nodes)); - context.node_freelist = &freelist_nodes[0].rn; + RDIC_Freelist freelist = {&freelist_nodes[0].rn}; + context.freelist = &freelist; RDIC_Node_Reference root = {0}; root = rdic_context_start_frame( &context, @@ -304,7 +309,8 @@ void test__get_node__succeed(void) RDIC_Context context = {0}; GUI_Node freelist_nodes[2] = {0}; init_node_freelist(freelist_nodes, ARRAYLENGTH(freelist_nodes)); - context.node_freelist = &freelist_nodes[0].rn; + RDIC_Freelist freelist = {&freelist_nodes[0].rn}; + context.freelist = &freelist; RDIC_Node_Reference root = {0}; root = rdic_context_start_frame( &context, @@ -329,7 +335,8 @@ void test__get_node_finish_frame__succeed(void) RDIC_Context context = {0}; GUI_Node freelist_nodes[2] = {0}; init_node_freelist(freelist_nodes, ARRAYLENGTH(freelist_nodes)); - context.node_freelist = &freelist_nodes[0].rn; + RDIC_Freelist freelist = {&freelist_nodes[0].rn}; + context.freelist = &freelist; RDIC_Node_Reference root = {0}; root = rdic_context_start_frame( &context, @@ -381,7 +388,7 @@ void test__single_node__returned_to_freelist(void) if(node.node->generation != 1) {TEST_ERROR("Node submitted to freelist increased generation more than once?");} ASSERT_REFERENCE_INVALID(node, "(Skipped in second frame)"); - if(context.node_freelist != node.node) + if(context.freelist->head != node.node) {TEST_ERROR("Node not prepended to freelist.");} @@ -398,7 +405,8 @@ void test__single_node__nodes_are_stable(void) RDIC_Context context = {0}; GUI_Node freelist_nodes[2] = {0}; init_node_freelist(freelist_nodes, ARRAYLENGTH(freelist_nodes)); - context.node_freelist = &freelist_nodes[0].rn; + RDIC_Freelist freelist = {&freelist_nodes[0].rn}; + context.freelist = &freelist; RDIC_Node_Reference root = {0}; RDIC_Node_Reference node = {0}; @@ -456,7 +464,7 @@ void test__single_node_freelist_reuse__succeed(void) if(node.generation != 1) {TEST_ERROR("`node` generation is not 1.");} ASSERT_REFERENCE_VALID(node); - if(context.node_freelist != NULL) + if(context.freelist->head != NULL) {TEST_ERROR("Freelist should be empty.");} } @@ -470,7 +478,8 @@ void test__parent_child_frame__succeed(void) RDIC_Context context = {0}; GUI_Node freelist_nodes[3] = {0}; init_node_freelist(freelist_nodes, ARRAYLENGTH(freelist_nodes)); - context.node_freelist = &freelist_nodes[0].rn; + RDIC_Freelist freelist = {&freelist_nodes[0].rn}; + context.freelist = &freelist; RDIC_Node_Reference root = {0}; RDIC_Node_Reference parent_node = {0}; RDIC_Node_Reference child_node = {0}; @@ -499,7 +508,8 @@ void test__parent_child_free_child__child_to_freelist(void) RDIC_Context context = {0}; GUI_Node freelist_nodes[3] = {0}; init_node_freelist(freelist_nodes, ARRAYLENGTH(freelist_nodes)); - context.node_freelist = &freelist_nodes[0].rn; + RDIC_Freelist freelist = {&freelist_nodes[0].rn}; + context.freelist = &freelist; RDIC_Node_Reference root = {0}; RDIC_Node_Reference parent_node = {0}; RDIC_Node_Reference child_node = {0}; @@ -530,7 +540,7 @@ void test__parent_child_free_child__child_to_freelist(void) if(child_node.node->generation != 1) {TEST_ERROR("Node submitted to freelist retained same generation?");} ASSERT_REFERENCE_INVALID(child_node, "(Skipped in second frame)"); - if(context.node_freelist != child_node.node) + if(context.freelist->head != child_node.node) {TEST_ERROR("Node not prepended to freelist.");} } @@ -540,7 +550,8 @@ void test__parent_child_free_child__parent_to_freelist(void) RDIC_Context context = {0}; GUI_Node freelist_nodes[3] = {0}; init_node_freelist(freelist_nodes, ARRAYLENGTH(freelist_nodes)); - context.node_freelist = &freelist_nodes[0].rn; + RDIC_Freelist freelist = {&freelist_nodes[0].rn}; + context.freelist = &freelist; RDIC_Node_Reference root = {0}; RDIC_Node_Reference parent = {0}; RDIC_Node_Reference child = {0}; @@ -642,7 +653,8 @@ void test__parent_child_both_reused__succeed(void) RDIC_Context context = {0};\ GUI_Node freelist_nodes[5] = {0};\ init_node_freelist(freelist_nodes, ARRAYLENGTH(freelist_nodes));\ - context.node_freelist = &freelist_nodes[0].rn;\ + RDIC_Freelist freelist = {&freelist_nodes[0].rn};\ + context.freelist = &freelist;\ RDIC_Node_Reference root = {0};\ RDIC_Node_Reference parent = {0};\ RDIC_Node_Reference child1 = {0};\ @@ -816,7 +828,8 @@ void test__exhaust_freelist_larger_tree__no_crash(void) RDIC_Context context = {0}; GUI_Node freelist_nodes[3] = {0}; init_node_freelist(freelist_nodes, ARRAYLENGTH(freelist_nodes)); - context.node_freelist = &freelist_nodes[0].rn; + RDIC_Freelist freelist = {&freelist_nodes[0].rn}; + context.freelist = &freelist; RDIC_Node_Reference root = {0}; RDIC_Node_Reference n1 = {0}; RDIC_Node_Reference n1_1 = {0}; @@ -879,7 +892,8 @@ void test__large_tree_constructed__succeed(void) RDIC_Context context = {0}; GUI_Node freelist_nodes[14] = {0}; init_node_freelist(freelist_nodes, ARRAYLENGTH(freelist_nodes)); - context.node_freelist = &freelist_nodes[0].rn; + RDIC_Freelist freelist = {&freelist_nodes[0].rn}; + context.freelist = &freelist; RDIC_Node_Reference root = {0}; RDIC_Node_Reference n1 = {0}; RDIC_Node_Reference n1_1 = {0}; @@ -954,7 +968,8 @@ void test__culling_that_needs_to_use_stash__succeed(void) RDIC_Context context = {0}; GUI_Node freelist_nodes[14] = {0}; init_node_freelist(freelist_nodes, ARRAYLENGTH(freelist_nodes)); - context.node_freelist = &freelist_nodes[0].rn; + RDIC_Freelist freelist = {&freelist_nodes[0].rn}; + context.freelist = &freelist; RDIC_Node_Reference root = {0}; RDIC_Node_Reference n1 = {0}; RDIC_Node_Reference n1_1 = {0};