From c31b0def42942c7d9f61b87e9aedf665363970ae Mon Sep 17 00:00:00 2001 From: normal Date: Mon, 29 Jun 2015 18:10:00 +0000 Subject: [PATCH] st.c: use ccan linked-list (try 3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- ChangeLog | 23 ++++ include/ruby/st.h | 2 +- st.c | 283 ++++++++++++++++++++-------------------------- 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 + + * 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 * 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; }