I can't resist to propose another minor code improvement.

For this one I even recall where I learned the trick: early in my CS
studies, we been taken to analyse how we could do better than the
straight forward implementation of double linked lists.
(Which would be the implementation of e.g., the finalizer_list
in runtime.c)

The idea is: unlinking requires too many conditionals.
The solution would be: you don't want to keep a pointer to the list
elements (which in case of the empty list would be NULL) as root.
Instead you use a static list node (thereby wasting the unused memory
for the payload).  As end-of-list marker you don't use NULL, but
the address of that root node.  You initialize the root nodes
previous and next to the address of the root node.

Now you can save all those conditionals.  Unlinking is turned into
two straight forward updates of the previous and next pointers
handing on the corresponding fields of the node.

Saves a few line, conditional and looks IMHO more clever.
Though I'm biased by the abovementioned history.

Find the diff attached.

/Jörg


Index: runtime.c
===================================================================
--- runtime.c
+++ runtime.c
@@ -441,11 +441,11 @@
   pending_finalizer_count,
   callback_returned_flag;
 static C_TLS WEAK_TABLE_ENTRY *weak_item_table;
 static C_TLS C_GC_ROOT *gc_root_list = NULL;
 static C_TLS FINALIZER_NODE
-  *finalizer_list,
+  finalizer_list,
   *finalizer_free_list,
   **pending_finalizer_indices;
 static C_TLS void *current_module_handle;
 static C_TLS int flonum_print_precision = FLONUM_PRINT_PRECISION;
 static C_TLS HDUMP_BUCKET **hdump_table;
@@ -638,11 +638,11 @@
     if((weak_item_table = (WEAK_TABLE_ENTRY *)C_calloc(WEAK_TABLE_SIZE, sizeof(WEAK_TABLE_ENTRY))) == NULL)
       return 0;
   }
 
   /* Initialize finalizer lists: */
-  finalizer_list = NULL;
+  finalizer_list.next = finalizer_list.previous = &finalizer_list;
   finalizer_free_list = NULL;
   pending_finalizer_indices =
       (FINALIZER_NODE **)C_malloc(C_max_pending_finalizers * sizeof(FINALIZER_NODE *));
 
   if(pending_finalizer_indices == NULL) return 0;
@@ -2802,11 +2802,11 @@
 	if(gc_report_flag)
 	  C_dbg(C_text("GC"), C_text("%d finalized item(s) still pending\n"), j);
 
 	j = fcount = 0;
 
-	for(flist = finalizer_list; flist != NULL; flist = flist->next) {
+	for(flist = finalizer_list.next; flist != &finalizer_list; flist = flist->next) {
 	  mark(&flist->item);
 	  mark(&flist->finalizer);
 	  ++fcount;
 	}
 
@@ -2820,19 +2820,19 @@
       }
       else {
 	j = fcount = 0;
 
 	/* move into pending */
-	for(flist = finalizer_list; flist != NULL; flist = flist->next) {
+	for(flist = finalizer_list.next; flist != &finalizer_list; flist = flist->next) {
 	  if(j < C_max_pending_finalizers) {
 	    if(!is_fptr(C_block_header(flist->item)))
 	      pending_finalizer_indices[ j++ ] = flist;
 	  }
 	}
 
 	/* mark */
-	for(flist = finalizer_list; flist != NULL; flist = flist->next) {
+	for(flist = finalizer_list.next; flist != &finalizer_list; flist = flist->next) {
 	  mark(&flist->item);
 	  mark(&flist->finalizer);
 	}
 
 	/* mark finalizable GC roots: */
@@ -2864,14 +2864,12 @@
 	for(i = 0; i < pending_finalizer_count; ++i) {
 	  flist = pending_finalizer_indices[ i ];
 	  C_set_block_item(last, 1 + i * 2, flist->item);
 	  C_set_block_item(last, 2 + i * 2, flist->finalizer);
 
-	  if(flist->previous != NULL) flist->previous->next = flist->next;
-	  else finalizer_list = flist->next;
-
-	  if(flist->next != NULL) flist->next->previous = flist->previous;
+	  flist->previous->next = flist->next;
+          flist->next->previous = flist->previous;
 
 	  flist->next = finalizer_free_list;
 	  flist->previous = NULL;
 	  finalizer_free_list = flist;
 	  --live_finalizer_count;
@@ -3232,11 +3230,11 @@
   /* Mark locative table: */
   for(i = 0; i < locative_table_count; ++i)
     remark(&locative_table[ i ]);
 
   /* Mark finalizer table: */
-  for(flist = finalizer_list; flist != NULL; flist = flist->next) {
+  for(flist = finalizer_list.next; flist != &finalizer_list; flist = flist->next) {
     remark(&flist->item);
     remark(&flist->finalizer);
   }
 
   /* Mark weakly held items: */
@@ -8097,15 +8095,14 @@
   else {
     flist = finalizer_free_list;
     finalizer_free_list = flist->next;
   }
 
-  if(finalizer_list != NULL) finalizer_list->previous = flist;
-
-  flist->previous = NULL;
-  flist->next = finalizer_list;
-  finalizer_list = flist;
+  flist->next = &finalizer_list;
+  flist->previous = finalizer_list.previous;
+  finalizer_list.previous->next = flist;
+  finalizer_list.previous = flist;
 
   if(C_in_stackp(x)) C_mutate(&flist->item, x);
   else flist->item = x;
 
   if(C_in_stackp(proc)) C_mutate(&flist->finalizer, proc);
@@ -8118,14 +8115,14 @@
 int C_do_unregister_finalizer(C_word x)
 {
   int n;
   FINALIZER_NODE *flist;
 
-  for(flist = finalizer_list; flist != NULL; flist = flist->next) {
+  for(flist = finalizer_list.next; flist != &finalizer_list; flist = flist->next) {
     if(flist->item == x) {
-      if(flist->previous == NULL) finalizer_list = flist->next;
-      else flist->previous->next = flist->next;
+      flist->previous->next = flist->next;
+      flist->next->previous = flist->previous;
 
       return 1;
     }
   }
 

_______________________________________________
Chicken-users mailing list
Chicken-users@nongnu.org
https://lists.nongnu.org/mailman/listinfo/chicken-users

Reply via email to