st.c: use ccan linked-list (try 3)

This improves the bm_vm2_bighash benchmark significantly by
removing branches during insert, but slows down anything
requiring iteration with the more complex loop termination
checking.

Speedup ratio of 1.10 - 1.20 is typical for the vm2_bighash
benchmark.

v3 - st_head calculates list_head address in two steps
     to avoid a bug in old gcc 4.4 (Debian 4.4.7-2)
     bug which incorrectly warned with:
     warning: dereferencing pointer ‘({anonymous})’ does break
     strict-aliasing rules

* include/ruby/st.h (struct st_table): hide struct list_head
* st.c (struct st_table_entry): adjust struct
  (head, tail): remove shortcut macros
  (st_head): new wrapper function
  (st_init_table_with_size): adjust to new struct and API
  (st_clear): ditto
  (add_direct): ditto
  (unpack_entries): ditto
  (rehash): ditto
  (st_copy): ditto
  (remove_entry): ditto
  (st_shift): ditto
  (st_foreach_check): ditto
  (st_foreach): ditto
  (get_keys): ditto
  (get_values): ditto
  (st_values_check): ditto
  (st_reverse_foreach_check): ditto (unused)
  (st_reverse_foreach): ditto (unused)
  [ruby-core:69726] [Misc #10278]

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@51064 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

3 files changed, 145 insertions, 163 deletions

diff --git a/ChangeLog b/ChangeLog
index 1f55ab2b74..c0c05e78fb 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,26 @@
+Tue Jun 30 02:47:02 2015  Eric Wong  <e@80x24.org>
+
+	* include/ruby/st.h (struct st_table): hide struct list_head
+	* st.c (struct st_table_entry): adjust struct
+	  (head, tail): remove shortcut macros
+	  (st_head): new wrapper function
+	  (st_init_table_with_size): adjust to new struct and API
+	  (st_clear): ditto
+	  (add_direct): ditto
+	  (unpack_entries): ditto
+	  (rehash): ditto
+	  (st_copy): ditto
+	  (remove_entry): ditto
+	  (st_shift): ditto
+	  (st_foreach_check): ditto
+	  (st_foreach): ditto
+	  (get_keys): ditto
+	  (get_values): ditto
+	  (st_values_check): ditto
+	  (st_reverse_foreach_check): ditto (unused)
+	  (st_reverse_foreach): ditto (unused)
+	  [ruby-core:69726] [Misc #10278]
+
 Mon Jun 29 17:38:01 2015  Nobuyoshi Nakada  <nobu@ruby-lang.org>
 
 	* insns.def (defineclass): do not quote unprintable characters at
diff --git a/include/ruby/st.h b/include/ruby/st.h
index b4fdf7e8ea..190bad2a35 100644
--- a/include/ruby/st.h
+++ b/include/ruby/st.h
@@ -86,7 +86,7 @@ struct st_table {
     union {
 	struct {
 	    struct st_table_entry **bins;
-	    struct st_table_entry *head, *tail;
+	    void *private_list_head[2];
 	} big;
 	struct {
 	    struct st_packed_entry *entries;
diff --git a/st.c b/st.c
index df616d4ade..887b9dce4d 100644
--- a/st.c
+++ b/st.c
@@ -23,7 +23,7 @@ struct st_table_entry {
     st_data_t key;
     st_data_t record;
     st_table_entry *next;
-    st_table_entry *fore, *back;
+    struct list_node olist;
 };
 
 typedef struct st_packed_entry {
@@ -108,8 +108,6 @@ st_realloc_bins(st_table_entry **bins, st_index_t newsize, st_index_t oldsize)
 
 /* Shortcut */
 #define bins as.big.bins
-#define head as.big.head
-#define tail as.big.tail
 #define real_entries as.packed.real_entries
 
 /* preparation for possible packing improvements */
@@ -195,6 +193,13 @@ stat_col(void)
 }
 #endif
 
+static struct list_head *
+st_head(const st_table *tbl)
+{
+    uintptr_t addr = (uintptr_t)&tbl->as.big.private_list_head;
+    return (struct list_head *)addr;
+}
+
 st_table*
 st_init_table_with_size(const struct st_hash_type *type, st_index_t size)
 {
@@ -220,14 +225,14 @@ st_init_table_with_size(const struct st_hash_type *type, st_index_t size)
     tbl->entries_packed = size <= MAX_PACKED_HASH;
     if (tbl->entries_packed) {
 	size = ST_DEFAULT_PACKED_TABLE_SIZE;
+	tbl->real_entries = 0;
     }
     else {
 	size = new_size(size);	/* round up to power-of-two */
+	list_head_init(st_head(tbl));
     }
     tbl->num_bins = size;
     tbl->bins = st_alloc_bins(size);
-    tbl->head = 0;
-    tbl->tail = 0;
 
     return tbl;
 }
@@ -278,7 +283,6 @@ void
 st_clear(st_table *table)
 {
     register st_table_entry *ptr, *next;
-    st_index_t i;
 
     if (table->entries_packed) {
         table->num_entries = 0;
@@ -286,18 +290,13 @@ st_clear(st_table *table)
         return;
     }
 
-    for (i = 0; i < table->num_bins; i++) {
-	ptr = table->bins[i];
-	table->bins[i] = 0;
-	while (ptr != 0) {
-	    next = ptr->next;
-	    st_free_entry(ptr);
-	    ptr = next;
-	}
+    list_for_each_safe(st_head(table), ptr, next, olist) {
+	/* list_del is not needed */
+	st_free_entry(ptr);
     }
     table->num_entries = 0;
-    table->head = 0;
-    table->tail = 0;
+    MEMZERO(table->bins, st_table_entry*, table->num_bins);
+    list_head_init(st_head(table));
 }
 
 void
@@ -465,16 +464,7 @@ add_direct(st_table *table, st_data_t key, st_data_t value,
     }
 
     entry = new_entry(table, key, value, hash_val, bin_pos);
-
-    if (table->head != 0) {
-	entry->fore = 0;
-	(entry->back = table->tail)->fore = entry;
-	table->tail = entry;
-    }
-    else {
-	table->head = table->tail = entry;
-	entry->fore = entry->back = 0;
-    }
+    list_add_tail(st_head(table), &entry->olist);
     table->num_entries++;
 }
 
@@ -483,7 +473,7 @@ unpack_entries(register st_table *table)
 {
     st_index_t i;
     st_packed_entry packed_bins[MAX_PACKED_HASH];
-    register st_table_entry *entry, *preventry = 0, **chain;
+    register st_table_entry *entry;
     st_table tmp_table = *table;
 
     MEMCPY(packed_bins, PACKED_BINS(table), st_packed_entry, MAX_PACKED_HASH);
@@ -495,22 +485,24 @@ unpack_entries(register st_table *table)
     tmp_table.bins = st_realloc_bins(tmp_table.bins, ST_DEFAULT_INIT_TABLE_SIZE, tmp_table.num_bins);
     tmp_table.num_bins = ST_DEFAULT_INIT_TABLE_SIZE;
 #endif
+
+    /*
+     * order is important here, we need to keep the original table
+     * walkable during GC (GC may be triggered by new_entry call)
+     */
     i = 0;
-    chain = &tmp_table.head;
+    list_head_init(st_head(&tmp_table));
     do {
 	st_data_t key = packed_bins[i].key;
 	st_data_t val = packed_bins[i].val;
 	st_index_t hash = packed_bins[i].hash;
 	entry = new_entry(&tmp_table, key, val, hash,
 			  hash_pos(hash, ST_DEFAULT_INIT_TABLE_SIZE));
-	*chain = entry;
-	entry->back = preventry;
-	preventry = entry;
-	chain = &entry->fore;
+	list_add_tail(st_head(&tmp_table), &entry->olist);
     } while (++i < MAX_PACKED_HASH);
-    *chain = NULL;
-    tmp_table.tail = entry;
     *table = tmp_table;
+    list_head_init(st_head(table));
+    list_append_list(st_head(table), st_head(&tmp_table));
 }
 
 static void
@@ -620,12 +612,10 @@ rehash(register st_table *table)
     table->num_bins = new_num_bins;
     table->bins = new_bins;
 
-    if ((ptr = table->head) != 0) {
-	do {
-	    hash_val = hash_pos(ptr->hash, new_num_bins);
-	    ptr->next = new_bins[hash_val];
-	    new_bins[hash_val] = ptr;
-	} while ((ptr = ptr->fore) != 0);
+    list_for_each(st_head(table), ptr, olist) {
+	hash_val = hash_pos(ptr->hash, new_num_bins);
+	ptr->next = new_bins[hash_val];
+	new_bins[hash_val] = ptr;
     }
 }
 
@@ -633,9 +623,8 @@ st_table*
 st_copy(st_table *old_table)
 {
     st_table *new_table;
-    st_table_entry *ptr, *entry, *prev, **tailp;
+    st_table_entry *ptr, *entry;
     st_index_t num_bins = old_table->num_bins;
-    st_index_t hash_val;
 
     new_table = st_alloc_table();
     if (new_table == 0) {
@@ -655,24 +644,12 @@ st_copy(st_table *old_table)
         return new_table;
     }
 
-    if ((ptr = old_table->head) != 0) {
-	prev = 0;
-	tailp = &new_table->head;
-	do {
-	    entry = st_alloc_entry();
-	    if (entry == 0) {
-		st_free_table(new_table);
-		return 0;
-	    }
-	    *entry = *ptr;
-	    hash_val = hash_pos(entry->hash, num_bins);
-	    entry->next = new_table->bins[hash_val];
-	    new_table->bins[hash_val] = entry;
-	    entry->back = prev;
-	    *tailp = prev = entry;
-	    tailp = &entry->fore;
-	} while ((ptr = ptr->fore) != 0);
-	new_table->tail = prev;
+    list_head_init(st_head(new_table));
+
+    list_for_each(st_head(old_table), ptr, olist) {
+	entry = new_entry(new_table, ptr->key, ptr->record, ptr->hash,
+			    hash_pos(ptr->hash, num_bins));
+	list_add_tail(st_head(new_table), &entry->olist);
     }
 
     return new_table;
@@ -681,17 +658,7 @@ st_copy(st_table *old_table)
 static inline void
 remove_entry(st_table *table, st_table_entry *ptr)
 {
-    if (ptr->fore == 0 && ptr->back == 0) {
-	table->head = 0;
-	table->tail = 0;
-    }
-    else {
-	st_table_entry *fore = ptr->fore, *back = ptr->back;
-	if (fore) fore->back = back;
-	if (back) back->fore = fore;
-	if (ptr == table->head) table->head = fore;
-	if (ptr == table->tail) table->tail = back;
-    }
+    list_del(&ptr->olist);
     table->num_entries--;
 }
 
@@ -771,6 +738,7 @@ st_delete_safe(register st_table *table, register st_data_t *key, st_data_t *val
 int
 st_shift(register st_table *table, register st_data_t *key, st_data_t *value)
 {
+    st_table_entry *old;
     st_table_entry **prev;
     register st_table_entry *ptr;
 
@@ -786,12 +754,13 @@ st_shift(register st_table *table, register st_data_t *key, st_data_t *value)
         return 1;
     }
 
-    prev = &table->bins[hash_pos(table->head->hash, table->num_bins)];
-    while ((ptr = *prev) != table->head) prev = &ptr->next;
+    old = list_pop(st_head(table), st_table_entry, olist);
+    table->num_entries--;
+    prev = &table->bins[hash_pos(old->hash, table->num_bins)];
+    while ((ptr = *prev) != old) prev = &ptr->next;
     *prev = ptr->next;
     if (value != 0) *value = ptr->record;
     *key = ptr->key;
-    remove_entry(table, ptr);
     st_free_entry(ptr);
     return 1;
 }
@@ -918,7 +887,8 @@ st_update(st_table *table, st_data_t key, st_update_callback_func *func, st_data
 int
 st_foreach_check(st_table *table, int (*func)(ANYARGS), st_data_t arg, st_data_t never)
 {
-    st_table_entry *ptr, **last, *tmp;
+    st_table_entry *ptr, **last, *tmp, *next;
+    struct list_head *head;
     enum st_retval retval;
     st_index_t i;
 
@@ -935,8 +905,10 @@ st_foreach_check(st_table *table, int (*func)(ANYARGS), st_data_t arg, st_data_t
 		FIND_ENTRY(table, ptr, hash, i);
 		if (retval == ST_CHECK) {
 		    if (!ptr) goto deleted;
-		    goto unpacked_continue;
 		}
+		if (table->num_entries == 0) return 0;
+		head = st_head(table);
+		next = list_entry(ptr->olist.next, st_table_entry, olist);
 		goto unpacked;
 	    }
 	    switch (retval) {
@@ -961,14 +933,10 @@ st_foreach_check(st_table *table, int (*func)(ANYARGS), st_data_t arg, st_data_t
 	}
 	return 0;
     }
-    else {
-	ptr = table->head;
-    }
 
-    if (ptr != 0) {
-	do {
-	    if (ptr->key == never)
-		goto unpacked_continue;
+    head = st_head(table);
+    list_for_each_safe(head, ptr, next, olist) {
+	if (ptr->key != never) {
 	    i = hash_pos(ptr->hash, table->num_bins);
 	    retval = (*func)(ptr->key, ptr->record, arg, 0);
 	  unpacked:
@@ -984,8 +952,6 @@ st_foreach_check(st_table *table, int (*func)(ANYARGS), st_data_t arg, st_data_t
 		}
 		/* fall through */
 	      case ST_CONTINUE:
-	      unpacked_continue:
-		ptr = ptr->fore;
 		break;
 	      case ST_STOP:
 		return 0;
@@ -993,16 +959,15 @@ st_foreach_check(st_table *table, int (*func)(ANYARGS), st_data_t arg, st_data_t
 		last = &table->bins[hash_pos(ptr->hash, table->num_bins)];
 		for (; (tmp = *last) != 0; last = &tmp->next) {
 		    if (ptr == tmp) {
-			tmp = ptr->fore;
 			remove_entry(table, ptr);
 			ptr->key = ptr->record = never;
 			ptr->hash = 0;
-			ptr = tmp;
 			break;
 		    }
 		}
+		if (table->num_entries == 0) return 0;
 	    }
-	} while (ptr && table->head);
+	}
     }
     return 0;
 }
@@ -1010,8 +975,9 @@ st_foreach_check(st_table *table, int (*func)(ANYARGS), st_data_t arg, st_data_t
 int
 st_foreach(st_table *table, int (*func)(ANYARGS), st_data_t arg)
 {
-    st_table_entry *ptr, **last, *tmp;
+    st_table_entry *ptr, **last, *tmp, *next;
     enum st_retval retval;
+    struct list_head *head;
     st_index_t i;
 
     if (table->entries_packed) {
@@ -1025,6 +991,8 @@ st_foreach(st_table *table, int (*func)(ANYARGS), st_data_t arg)
 	    if (!table->entries_packed) {
 		FIND_ENTRY(table, ptr, hash, i);
 		if (!ptr) return 0;
+		head = st_head(table);
+		next = list_entry(ptr->olist.next, st_table_entry, olist);
 		goto unpacked;
 	    }
 	    switch (retval) {
@@ -1041,36 +1009,30 @@ st_foreach(st_table *table, int (*func)(ANYARGS), st_data_t arg)
 	}
 	return 0;
     }
-    else {
-	ptr = table->head;
-    }
 
-    if (ptr != 0) {
-	do {
-	    i = hash_pos(ptr->hash, table->num_bins);
-	    retval = (*func)(ptr->key, ptr->record, arg, 0);
-	  unpacked:
-	    switch (retval) {
-	      case ST_CONTINUE:
-		ptr = ptr->fore;
-		break;
-	      case ST_CHECK:
-	      case ST_STOP:
-		return 0;
-	      case ST_DELETE:
-		last = &table->bins[hash_pos(ptr->hash, table->num_bins)];
-		for (; (tmp = *last) != 0; last = &tmp->next) {
-		    if (ptr == tmp) {
-			tmp = ptr->fore;
-			*last = ptr->next;
-			remove_entry(table, ptr);
-			st_free_entry(ptr);
-			ptr = tmp;
-			break;
-		    }
+    head = st_head(table);
+    list_for_each_safe(head, ptr, next, olist) {
+	i = hash_pos(ptr->hash, table->num_bins);
+	retval = (*func)(ptr->key, ptr->record, arg, 0);
+      unpacked:
+	switch (retval) {
+	  case ST_CONTINUE:
+	    break;
+	  case ST_CHECK:
+	  case ST_STOP:
+	    return 0;
+	  case ST_DELETE:
+	    last = &table->bins[hash_pos(ptr->hash, table->num_bins)];
+	    for (; (tmp = *last) != 0; last = &tmp->next) {
+		if (ptr == tmp) {
+		    *last = ptr->next;
+		    remove_entry(table, ptr);
+		    st_free_entry(ptr);
+		    break;
 		}
 	    }
-	} while (ptr && table->head);
+	    if (table->num_entries == 0) return 0;
+	}
     }
     return 0;
 }
@@ -1092,9 +1054,11 @@ get_keys(st_table *table, st_data_t *keys, st_index_t size, int check, st_data_t
 	}
     }
     else {
-	st_table_entry *ptr = table->head;
+	st_table_entry *ptr;
 	st_data_t *keys_end = keys + size;
-	for (; ptr && keys < keys_end; ptr = ptr->fore) {
+
+	list_for_each(st_head(table), ptr, olist) {
+	    if (keys >= keys_end) break;
 	    key = ptr->key;
 	    if (check && key == never) continue;
 	    *keys++ = key;
@@ -1133,9 +1097,11 @@ get_values(st_table *table, st_data_t *values, st_index_t size, int check, st_da
 	}
     }
     else {
-	st_table_entry *ptr = table->head;
+	st_table_entry *ptr;
 	st_data_t *values_end = values + size;
-	for (; ptr && values < values_end; ptr = ptr->fore) {
+
+	list_for_each(st_head(table), ptr, olist) {
+	    if (values >= values_end) break;
 	    key = ptr->key;
 	    if (check && key == never) continue;
 	    *values++ = ptr->record;
@@ -1161,7 +1127,8 @@ st_values_check(st_table *table, st_data_t *values, st_index_t size, st_data_t n
 int
 st_reverse_foreach_check(st_table *table, int (*func)(ANYARGS), st_data_t arg, st_data_t never)
 {
-    st_table_entry *ptr, **last, *tmp;
+    st_table_entry *ptr, **last, *tmp, *next;
+    struct list_head *head;
     enum st_retval retval;
     st_index_t i;
 
@@ -1179,8 +1146,10 @@ st_reverse_foreach_check(st_table *table, int (*func)(ANYARGS), st_data_t arg, s
 		FIND_ENTRY(table, ptr, hash, i);
 		if (retval == ST_CHECK) {
 		    if (!ptr) goto deleted;
-		    goto unpacked_continue;
 		}
+		if (table->num_entries == 0) return 0;
+		head = st_head(table);
+		next = list_entry(ptr->olist.next, st_table_entry, olist);
 		goto unpacked;
 	    }
 	    switch (retval) {
@@ -1205,14 +1174,10 @@ st_reverse_foreach_check(st_table *table, int (*func)(ANYARGS), st_data_t arg, s
 	}
 	return 0;
     }
-    else {
-	ptr = table->tail;
-    }
 
-    if (ptr != 0) {
-	do {
-	    if (ptr->key == never)
-		goto unpacked_continue;
+    head = st_head(table);
+    list_for_each_rev_safe(head, ptr, next, olist) {
+	if (ptr->key != never) {
 	    i = hash_pos(ptr->hash, table->num_bins);
 	    retval = (*func)(ptr->key, ptr->record, arg, 0);
 	  unpacked:
@@ -1228,8 +1193,6 @@ st_reverse_foreach_check(st_table *table, int (*func)(ANYARGS), st_data_t arg, s
 		}
 		/* fall through */
 	      case ST_CONTINUE:
-	      unpacked_continue:
-		ptr = ptr->back;
 		break;
 	      case ST_STOP:
 		return 0;
@@ -1237,16 +1200,15 @@ st_reverse_foreach_check(st_table *table, int (*func)(ANYARGS), st_data_t arg, s
 		last = &table->bins[hash_pos(ptr->hash, table->num_bins)];
 		for (; (tmp = *last) != 0; last = &tmp->next) {
 		    if (ptr == tmp) {
-			tmp = ptr->back;
 			remove_entry(table, ptr);
 			ptr->key = ptr->record = never;
 			ptr->hash = 0;
-			ptr = tmp;
 			break;
 		    }
 		}
+		if (table->num_entries == 0) return 0;
 	    }
-	} while (ptr && table->head);
+	}
     }
     return 0;
 }
@@ -1254,8 +1216,9 @@ st_reverse_foreach_check(st_table *table, int (*func)(ANYARGS), st_data_t arg, s
 int
 st_reverse_foreach(st_table *table, int (*func)(ANYARGS), st_data_t arg)
 {
-    st_table_entry *ptr, **last, *tmp;
+    st_table_entry *ptr, **last, *tmp, *next;
     enum st_retval retval;
+    struct list_head *head;
     st_index_t i;
 
     if (table->entries_packed) {
@@ -1270,6 +1233,8 @@ st_reverse_foreach(st_table *table, int (*func)(ANYARGS), st_data_t arg)
 	    if (!table->entries_packed) {
 		FIND_ENTRY(table, ptr, hash, i);
 		if (!ptr) return 0;
+		head = st_head(table);
+		next = list_entry(ptr->olist.next, st_table_entry, olist);
 		goto unpacked;
 	    }
 	    switch (retval) {
@@ -1285,36 +1250,30 @@ st_reverse_foreach(st_table *table, int (*func)(ANYARGS), st_data_t arg)
 	}
 	return 0;
     }
-    else {
-	ptr = table->tail;
-    }
 
-    if (ptr != 0) {
-	do {
-	    i = hash_pos(ptr->hash, table->num_bins);
-	    retval = (*func)(ptr->key, ptr->record, arg, 0);
-	  unpacked:
-	    switch (retval) {
-	      case ST_CONTINUE:
-		ptr = ptr->back;
-		break;
-	      case ST_CHECK:
-	      case ST_STOP:
-		return 0;
-	      case ST_DELETE:
-		last = &table->bins[hash_pos(ptr->hash, table->num_bins)];
-		for (; (tmp = *last) != 0; last = &tmp->next) {
-		    if (ptr == tmp) {
-			tmp = ptr->back;
-			*last = ptr->next;
-			remove_entry(table, ptr);
-			st_free_entry(ptr);
-			ptr = tmp;
-			break;
-		    }
+    head = st_head(table);
+    list_for_each_rev_safe(head, ptr, next, olist) {
+	i = hash_pos(ptr->hash, table->num_bins);
+	retval = (*func)(ptr->key, ptr->record, arg, 0);
+      unpacked:
+	switch (retval) {
+	  case ST_CONTINUE:
+	    break;
+	  case ST_CHECK:
+	  case ST_STOP:
+	    return 0;
+	  case ST_DELETE:
+	    last = &table->bins[hash_pos(ptr->hash, table->num_bins)];
+	    for (; (tmp = *last) != 0; last = &tmp->next) {
+		if (ptr == tmp) {
+		    *last = ptr->next;
+		    remove_entry(table, ptr);
+		    st_free_entry(ptr);
+		    break;
 		}
 	    }
-	} while (ptr && table->head);
+	    if (table->num_entries == 0) return 0;
+	}
     }
     return 0;
 }