diff options
author | Neil Jerram <neil@ossau.uklinux.net> | 2009-03-25 22:34:23 +0000 |
---|---|---|
committer | Neil Jerram <neil@ossau.uklinux.net> | 2009-03-27 21:37:41 +0000 |
commit | 10b11bab3317f72f74b59c1b5becf731215ae2f4 (patch) | |
tree | ec2411c7f0da77a9244891c3de61adbc967e54d2 | |
parent | 1f2707cc9473548863b483fd12d97afe7d0c94c2 (diff) | |
download | guile-10b11bab3317f72f74b59c1b5becf731215ae2f4.tar.gz |
Make the interned symbols hash thread-safe
These changes introduce a mutex that protects all accesses to and
modifications of the symbols hash. They allow the test-define-race
test to run without errors for a long time. (Well, for 200s anyway.)
* libguile/environments.c (obarray_enter, obarray_replace): Extra
parameter on scm_i_rehash calls.
* libguile/hashtab.c: Include symbols.h.
(scm_i_rehash): New mutex parameter. When non-NULL, unlock and
relock this mutex around allocations, and recheck new size
calculation (in case another thread has changed it).
(rehash_after_gc): Special treatment for the symbol hash.
(scm_hash_fn_create_handle_x, scm_hash_fn_remove_x): Extra parameter on scm_i_rehash calls.
* libguile/hashtab.h (scm_i_rehash): New mutex parameter.
* libguile/symbols.c (symbols): Rename scm_i_symbols and make non-static.
(symbols_mutex): New variable.
(scm_sys_symbols): symbols -> scm_i_symbols.
(lookup_interned_symbol): Lock symbols_mutex while performing lookup.
(intern_symbol): Add name, len and raw_hash parameters. Lock
symbols_mutex while checking and modifying the symbols hash. Once
the mutex is locked, recheck in case another thread has already
interned the symbol. Return interned symbol, in order to tell
callers about this last case.
(scm_i_c_mem2symbol, scm_i_mem2symbol, scm_take_locale_symboln):
Update intern_symbol calls.
(scm_i_rehash_symbols_after_gc): New function.
(scm_symbols_prehistory): symbols -> scm_i_symbols. Initialize
symbols_mutex.
* libguile/symbols.h (scm_i_symbols, scm_i_rehash_symbols_after_gc):
New declarations.
-rw-r--r-- | libguile/environments.c | 4 | ||||
-rw-r--r-- | libguile/hashtab.c | 53 | ||||
-rw-r--r-- | libguile/hashtab.h | 3 | ||||
-rw-r--r-- | libguile/symbols.c | 93 | ||||
-rw-r--r-- | libguile/symbols.h | 2 |
5 files changed, 113 insertions, 42 deletions
diff --git a/libguile/environments.c b/libguile/environments.c index 13d63c035..077a3970a 100644 --- a/libguile/environments.c +++ b/libguile/environments.c @@ -516,7 +516,7 @@ obarray_enter (SCM obarray, SCM symbol, SCM data) SCM_SET_HASHTABLE_BUCKET (obarray, hash, slot); SCM_HASHTABLE_INCREMENT (obarray); if (SCM_HASHTABLE_N_ITEMS (obarray) > SCM_HASHTABLE_UPPER (obarray)) - scm_i_rehash (obarray, scm_i_hash_symbol, 0, "obarray_enter"); + scm_i_rehash (obarray, scm_i_hash_symbol, 0, "obarray_enter", NULL); return entry; } @@ -550,7 +550,7 @@ obarray_replace (SCM obarray, SCM symbol, SCM data) SCM_SET_HASHTABLE_BUCKET (obarray, hash, slot); SCM_HASHTABLE_INCREMENT (obarray); if (SCM_HASHTABLE_N_ITEMS (obarray) > SCM_HASHTABLE_UPPER (obarray)) - scm_i_rehash (obarray, scm_i_hash_symbol, 0, "obarray_replace"); + scm_i_rehash (obarray, scm_i_hash_symbol, 0, "obarray_replace", NULL); return SCM_BOOL_F; } diff --git a/libguile/hashtab.c b/libguile/hashtab.c index ea7fc69fa..029201233 100644 --- a/libguile/hashtab.c +++ b/libguile/hashtab.c @@ -30,6 +30,7 @@ #include "libguile/root.h" #include "libguile/vectors.h" #include "libguile/ports.h" +#include "libguile/symbols.h" #include "libguile/validate.h" #include "libguile/hashtab.h" @@ -117,13 +118,15 @@ void scm_i_rehash (SCM table, unsigned long (*hash_fn)(), void *closure, - const char* func_name) + const char* func_name, + scm_i_pthread_mutex_t *mutex) { - SCM buckets, new_buckets; - int i; + SCM buckets, new_buckets = SCM_BOOL_F; + int i = 0; unsigned long old_size; unsigned long new_size; + restart: if (SCM_HASHTABLE_N_ITEMS (table) < SCM_HASHTABLE_LOWER (table)) { /* rehashing is not triggered when i <= min_size */ @@ -133,7 +136,7 @@ scm_i_rehash (SCM table, while (i > SCM_HASHTABLE (table)->min_size_index && SCM_HASHTABLE_N_ITEMS (table) < hashtable_size[i] / 4); } - else + else if (SCM_HASHTABLE_N_ITEMS (table) > SCM_HASHTABLE_UPPER (table)) { i = SCM_HASHTABLE (table)->size_index + 1; if (i >= HASHTABLE_SIZE_N) @@ -157,12 +160,28 @@ scm_i_rehash (SCM table, SCM_HASHTABLE (table)->upper = 9 * new_size / 10; buckets = SCM_HASHTABLE_VECTOR (table); - if (SCM_HASHTABLE_WEAK_P (table)) - new_buckets = scm_i_allocate_weak_vector (SCM_HASHTABLE_FLAGS (table), - scm_from_ulong (new_size), - SCM_EOL); - else - new_buckets = scm_c_make_vector (new_size, SCM_EOL); + if (scm_is_false (new_buckets) || + (SCM_SIMPLE_VECTOR_LENGTH (new_buckets) != new_size)) + { + /* Need to allocate or reallocate the new_buckets vector. */ + if (mutex) + scm_i_pthread_mutex_unlock (mutex); + + if (SCM_HASHTABLE_WEAK_P (table)) + new_buckets = scm_i_allocate_weak_vector (SCM_HASHTABLE_FLAGS (table), + scm_from_ulong (new_size), + SCM_EOL); + else + new_buckets = scm_c_make_vector (new_size, SCM_EOL); + + if (mutex) + scm_i_pthread_mutex_lock (mutex); + + /* Another thread could have messed with the hashtable while the + mutex was unlocked. So we now have to recalculate the rehash + size from scratch. */ + goto restart; + } /* When this is a weak hashtable, running the GC might change it. We need to cope with this while rehashing its elements. We do @@ -271,11 +290,15 @@ rehash_after_gc (void *dummy1 SCM_UNUSED, h = first; do { - /* Rehash only when we have a hash_fn. + /* Special treatment for the symbol hash, as it is protected + by a mutex. */ + if (scm_is_eq (h, scm_i_symbols)) + scm_i_rehash_symbols_after_gc (); + /* Otherwise, rehash only when we have a hash_fn. */ - if (SCM_HASHTABLE (h)->hash_fn) + else if (SCM_HASHTABLE (h)->hash_fn) scm_i_rehash (h, SCM_HASHTABLE (h)->hash_fn, NULL, - "rehash_after_gc"); + "rehash_after_gc", NULL); last = h; h = SCM_HASHTABLE_NEXT (h); } while (!scm_is_null (h)); @@ -490,7 +513,7 @@ scm_hash_fn_create_handle_x (SCM table, SCM obj, SCM init, unsigned long (*hash_ SCM_HASHTABLE_INCREMENT (table); if (SCM_HASHTABLE_N_ITEMS (table) < SCM_HASHTABLE_LOWER (table) || SCM_HASHTABLE_N_ITEMS (table) > SCM_HASHTABLE_UPPER (table)) - scm_i_rehash (table, hash_fn, closure, FUNC_NAME); + scm_i_rehash (table, hash_fn, closure, FUNC_NAME, NULL); } return SCM_CAR (new_bucket); } @@ -556,7 +579,7 @@ scm_hash_fn_remove_x (SCM table, SCM obj, { SCM_HASHTABLE_DECREMENT (table); if (SCM_HASHTABLE_N_ITEMS (table) < SCM_HASHTABLE_LOWER (table)) - scm_i_rehash (table, hash_fn, closure, "scm_hash_fn_remove_x"); + scm_i_rehash (table, hash_fn, closure, "scm_hash_fn_remove_x", NULL); } } return h; diff --git a/libguile/hashtab.h b/libguile/hashtab.h index 101735460..3eca04cb9 100644 --- a/libguile/hashtab.h +++ b/libguile/hashtab.h @@ -96,7 +96,8 @@ SCM_API SCM scm_weak_key_hash_table_p (SCM h); SCM_API SCM scm_weak_value_hash_table_p (SCM h); SCM_API SCM scm_doubly_weak_hash_table_p (SCM h); -SCM_API void scm_i_rehash (SCM table, unsigned long (*hash_fn)(), void *closure, const char*func_name); +SCM_API void scm_i_rehash (SCM table, unsigned long (*hash_fn)(), + void *closure, const char *func_name, scm_i_pthread_mutex_t *mutex); SCM_API void scm_i_scan_weak_hashtables (void); SCM_API SCM scm_hash_fn_get_handle (SCM table, SCM obj, unsigned long (*hash_fn) (), SCM (*assoc_fn) (), void * closure); diff --git a/libguile/symbols.c b/libguile/symbols.c index b1329fadc..0311bf395 100644 --- a/libguile/symbols.c +++ b/libguile/symbols.c @@ -46,7 +46,8 @@ -static SCM symbols; +SCM scm_i_symbols; +static scm_i_pthread_mutex_t symbols_mutex; #ifdef GUILE_DEBUG SCM_DEFINE (scm_sys_symbols, "%symbols", 0, 0, 0, @@ -54,7 +55,7 @@ SCM_DEFINE (scm_sys_symbols, "%symbols", 0, 0, 0, "Return the system symbol obarray.") #define FUNC_NAME s_scm_sys_symbols { - return symbols; + return scm_i_symbols; } #undef FUNC_NAME #endif @@ -85,20 +86,19 @@ scm_i_hash_symbol (SCM obj, unsigned long n, void *closure) } static SCM -lookup_interned_symbol (const char *name, size_t len, - unsigned long raw_hash) +locked_lookup (const char *name, size_t len, unsigned long raw_hash, unsigned long hash) { - /* Try to find the symbol in the symbols table */ + /* Look up a symbol in the symbols table, assuming that the symbols + mutex is already locked. */ SCM l; - unsigned long hash = raw_hash % SCM_HASHTABLE_N_BUCKETS (symbols); - for (l = SCM_HASHTABLE_BUCKET (symbols, hash); + for (l = SCM_HASHTABLE_BUCKET (scm_i_symbols, hash); !scm_is_null (l); l = SCM_CDR (l)) { SCM sym = SCM_CAAR (l); - if (scm_i_symbol_hash (sym) == raw_hash - && scm_i_symbol_length (sym) == len) + if ((scm_i_symbol_hash (sym) == raw_hash) && + (scm_i_symbol_length (sym) == len)) { const char *chrs = scm_i_symbol_chars (sym); size_t i = len; @@ -119,22 +119,56 @@ lookup_interned_symbol (const char *name, size_t len, return SCM_BOOL_F; } +static SCM +lookup_interned_symbol (const char *name, size_t len, + unsigned long raw_hash) +{ + /* Try to find the symbol in the symbols table */ + unsigned long hash; + SCM sym; + + scm_i_pthread_mutex_lock (&symbols_mutex); + + hash = raw_hash % SCM_HASHTABLE_N_BUCKETS (scm_i_symbols); + sym = locked_lookup (name, len, raw_hash, hash); + + scm_i_pthread_mutex_unlock (&symbols_mutex); + + return sym; +} + /* Intern SYMBOL, an uninterned symbol. */ -static void +static SCM intern_symbol (SCM symbol) { - SCM slot, cell; - unsigned long hash; + SCM sym, new_bucket; + unsigned long raw_hash, hash; - hash = scm_i_symbol_hash (symbol) % SCM_HASHTABLE_N_BUCKETS (symbols); - slot = SCM_HASHTABLE_BUCKET (symbols, hash); - cell = scm_cons (symbol, SCM_UNDEFINED); + /* Allocate new cell and bucket before locking the mutex. */ + new_bucket = scm_acons (symbol, SCM_UNDEFINED, SCM_BOOL_F); - SCM_SET_HASHTABLE_BUCKET (symbols, hash, scm_cons (cell, slot)); - SCM_HASHTABLE_INCREMENT (symbols); + scm_i_pthread_mutex_lock (&symbols_mutex); + + raw_hash = scm_i_symbol_hash (symbol); + hash = raw_hash % SCM_HASHTABLE_N_BUCKETS (scm_i_symbols); + sym = locked_lookup (scm_i_symbol_chars (symbol), + scm_i_symbol_length (symbol), + raw_hash, + hash); + if (scm_is_false (sym)) + { + SCM_SETCDR (new_bucket, SCM_HASHTABLE_BUCKET (scm_i_symbols, hash)); + SCM_SET_HASHTABLE_BUCKET (scm_i_symbols, hash, new_bucket); + SCM_HASHTABLE_INCREMENT (scm_i_symbols); + if (SCM_HASHTABLE_N_ITEMS (scm_i_symbols) > SCM_HASHTABLE_UPPER (scm_i_symbols)) + scm_i_rehash (scm_i_symbols, scm_i_hash_symbol, 0, "intern_symbol", + &symbols_mutex); + sym = symbol; + } - if (SCM_HASHTABLE_N_ITEMS (symbols) > SCM_HASHTABLE_UPPER (symbols)) - scm_i_rehash (symbols, scm_i_hash_symbol, 0, "intern_symbol"); + scm_i_pthread_mutex_unlock (&symbols_mutex); + + return sym; } static SCM @@ -149,7 +183,7 @@ scm_i_c_mem2symbol (const char *name, size_t len) /* The symbol was not found, create it. */ symbol = scm_i_c_make_symbol (name, len, 0, raw_hash, scm_cons (SCM_BOOL_F, SCM_EOL)); - intern_symbol (symbol); + symbol = intern_symbol (symbol); } return symbol; @@ -169,12 +203,22 @@ scm_i_mem2symbol (SCM str) /* The symbol was not found, create it. */ symbol = scm_i_make_symbol (str, 0, raw_hash, scm_cons (SCM_BOOL_F, SCM_EOL)); - intern_symbol (symbol); + symbol = intern_symbol (symbol); } return symbol; } +void +scm_i_rehash_symbols_after_gc () +{ + scm_i_pthread_mutex_lock (&symbols_mutex); + + scm_i_rehash (scm_i_symbols, scm_i_hash_symbol, 0, "rehash_after_gc", + &symbols_mutex); + + scm_i_pthread_mutex_unlock (&symbols_mutex); +} static SCM scm_i_mem2uninterned_symbol (SCM str) @@ -417,7 +461,7 @@ scm_take_locale_symboln (char *sym, size_t len) { res = scm_i_c_take_symbol (sym, len, 0, raw_hash, scm_cons (SCM_BOOL_F, SCM_EOL)); - intern_symbol (res); + res = intern_symbol (res); } else free (sym); @@ -434,8 +478,9 @@ scm_take_locale_symbol (char *sym) void scm_symbols_prehistory () { - symbols = scm_make_weak_key_hash_table (scm_from_int (2139)); - scm_permanent_object (symbols); + scm_i_symbols = scm_make_weak_key_hash_table (scm_from_int (2139)); + scm_permanent_object (scm_i_symbols); + scm_i_pthread_mutex_init (&symbols_mutex, NULL); } diff --git a/libguile/symbols.h b/libguile/symbols.h index f70d65578..30c30057e 100644 --- a/libguile/symbols.h +++ b/libguile/symbols.h @@ -34,6 +34,7 @@ #define SCM_I_F_SYMBOL_UNINTERNED 0x100 +extern SCM scm_i_symbols; #ifdef GUILE_DEBUG SCM_API SCM scm_sys_symbols (void); @@ -63,6 +64,7 @@ SCM_API SCM scm_take_locale_symboln (char *sym, size_t len); SCM_API unsigned long scm_i_hash_symbol (SCM obj, unsigned long n, void *closure); +void scm_i_rehash_symbols_after_gc (void); SCM_API void scm_symbols_prehistory (void); SCM_API void scm_init_symbols (void); |