From 5dacbd4c42171e447e2f07144faf502774dc921a Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Wed, 23 Mar 2016 09:52:13 +0100 Subject: [PATCH] Cleanup hashtable.h Issue #26588: * Pass the hash table rather than the key size to hash and compare functions * _Py_HASHTABLE_READ_KEY() and _Py_HASHTABLE_ENTRY_READ_KEY() macros now expect the hash table as the first parameter, rather than the key size * tracemalloc_get_traces_fill(): use _Py_HASHTABLE_ENTRY_READ_DATA() rather than pointer dereference * Remove the _Py_HASHTABLE_ENTRY_WRITE_PKEY() macro * Move "PKEY" and "PDATA" macros inside hashtable.c --- Modules/_tracemalloc.c | 50 ++++++++++++++++++------------------- Modules/hashtable.c | 55 ++++++++++++++++++++++++----------------- Modules/hashtable.h | 56 ++++++++++++++++++------------------------ Python/marshal.c | 2 +- 4 files changed, 82 insertions(+), 81 deletions(-) diff --git a/Modules/_tracemalloc.c b/Modules/_tracemalloc.c index e6465a3b749..169fd2c57df 100644 --- a/Modules/_tracemalloc.c +++ b/Modules/_tracemalloc.c @@ -223,23 +223,23 @@ set_reentrant(int reentrant) static Py_uhash_t -hashtable_hash_pyobject(size_t key_size, const void *pkey) +hashtable_hash_pyobject(_Py_hashtable_t *ht, const void *pkey) { PyObject *obj; - _Py_HASHTABLE_READ_KEY(key_size, pkey, obj); + _Py_HASHTABLE_READ_KEY(ht, pkey, obj); return PyObject_Hash(obj); } static int -hashtable_compare_unicode(size_t key_size, const void *pkey, +hashtable_compare_unicode(_Py_hashtable_t *ht, const void *pkey, const _Py_hashtable_entry_t *entry) { PyObject *key1, *key2; - _Py_HASHTABLE_READ_KEY(key_size, pkey, key1); - _Py_HASHTABLE_ENTRY_READ_KEY(key_size, entry, key2); + _Py_HASHTABLE_READ_KEY(ht, pkey, key1); + _Py_HASHTABLE_ENTRY_READ_KEY(ht, entry, key2); if (key1 != NULL && key2 != NULL) return (PyUnicode_Compare(key1, key2) == 0); @@ -249,12 +249,12 @@ hashtable_compare_unicode(size_t key_size, const void *pkey, static Py_uhash_t -hashtable_hash_pointer_t(size_t key_size, const void *pkey) +hashtable_hash_pointer_t(_Py_hashtable_t *ht, const void *pkey) { pointer_t ptr; Py_uhash_t hash; - _Py_HASHTABLE_READ_KEY(key_size, pkey, ptr); + _Py_HASHTABLE_READ_KEY(ht, pkey, ptr); hash = (Py_uhash_t)_Py_HashPointer((void*)ptr.ptr); hash ^= ptr.domain; @@ -263,13 +263,13 @@ hashtable_hash_pointer_t(size_t key_size, const void *pkey) int -hashtable_compare_pointer_t(size_t key_size, const void *pkey, +hashtable_compare_pointer_t(_Py_hashtable_t *ht, const void *pkey, const _Py_hashtable_entry_t *entry) { pointer_t ptr1, ptr2; - _Py_HASHTABLE_READ_KEY(key_size, pkey, ptr1); - _Py_HASHTABLE_ENTRY_READ_KEY(key_size, entry, ptr2); + _Py_HASHTABLE_READ_KEY(ht, pkey, ptr1); + _Py_HASHTABLE_ENTRY_READ_KEY(ht, entry, ptr2); /* compare pointer before domain, because pointer is more likely to be different */ @@ -304,25 +304,25 @@ raw_free(void *ptr) static Py_uhash_t -hashtable_hash_traceback(size_t key_size, const void *pkey) +hashtable_hash_traceback(_Py_hashtable_t *ht, const void *pkey) { traceback_t *traceback; - _Py_HASHTABLE_READ_KEY(key_size, pkey, traceback); + _Py_HASHTABLE_READ_KEY(ht, pkey, traceback); return traceback->hash; } static int -hashtable_compare_traceback(size_t key_size, const void *pkey, - const _Py_hashtable_entry_t *he) +hashtable_compare_traceback(_Py_hashtable_t *ht, const void *pkey, + const _Py_hashtable_entry_t *entry) { traceback_t *traceback1, *traceback2; const frame_t *frame1, *frame2; int i; - _Py_HASHTABLE_READ_KEY(key_size, pkey, traceback1); - _Py_HASHTABLE_ENTRY_READ_KEY(key_size, he, traceback2); + _Py_HASHTABLE_READ_KEY(ht, pkey, traceback1); + _Py_HASHTABLE_ENTRY_READ_KEY(ht, entry, traceback2); if (traceback1->nframe != traceback2->nframe) return 0; @@ -395,8 +395,7 @@ tracemalloc_get_frame(PyFrameObject *pyframe, frame_t *frame) /* intern the filename */ entry = _Py_HASHTABLE_GET_ENTRY(tracemalloc_filenames, filename); if (entry != NULL) { - _Py_HASHTABLE_ENTRY_READ_KEY(tracemalloc_filenames->key_size, entry, - filename); + _Py_HASHTABLE_ENTRY_READ_KEY(tracemalloc_filenames, entry, filename); } else { /* tracemalloc_filenames is responsible to keep a reference @@ -490,8 +489,7 @@ traceback_new(void) /* intern the traceback */ entry = _Py_HASHTABLE_GET_ENTRY(tracemalloc_tracebacks, traceback); if (entry != NULL) { - _Py_HASHTABLE_ENTRY_READ_KEY(tracemalloc_tracebacks->key_size, entry, - traceback); + _Py_HASHTABLE_ENTRY_READ_KEY(tracemalloc_tracebacks, entry, traceback); } else { traceback_t *copy; @@ -873,7 +871,7 @@ tracemalloc_clear_filename(_Py_hashtable_t *ht, _Py_hashtable_entry_t *entry, { PyObject *filename; - _Py_HASHTABLE_ENTRY_READ_KEY(ht->key_size, entry, filename); + _Py_HASHTABLE_ENTRY_READ_KEY(ht, entry, filename); Py_DECREF(filename); return 0; } @@ -885,7 +883,7 @@ traceback_free_traceback(_Py_hashtable_t *ht, _Py_hashtable_entry_t *entry, { traceback_t *traceback; - _Py_HASHTABLE_ENTRY_READ_KEY(ht->key_size, entry, traceback); + _Py_HASHTABLE_ENTRY_READ_KEY(ht, entry, traceback); raw_free(traceback); return 0; } @@ -1246,21 +1244,21 @@ tracemalloc_get_traces_fill(_Py_hashtable_t *traces, _Py_hashtable_entry_t *entr { get_traces_t *get_traces = user_data; _PyTraceMalloc_domain_t domain; - trace_t *trace; + trace_t trace; PyObject *tracemalloc_obj; int res; if (tracemalloc_config.use_domain) { pointer_t key; - _Py_HASHTABLE_ENTRY_READ_KEY(traces->key_size, entry, key); + _Py_HASHTABLE_ENTRY_READ_KEY(traces, entry, key); domain = key.domain; } else { domain = DEFAULT_DOMAIN; } - trace = (trace_t *)_Py_HASHTABLE_ENTRY_DATA(traces, entry); + _Py_HASHTABLE_ENTRY_READ_DATA(traces, entry, trace); - tracemalloc_obj = trace_to_pyobject(domain, trace, get_traces->tracebacks); + tracemalloc_obj = trace_to_pyobject(domain, &trace, get_traces->tracebacks); if (tracemalloc_obj == NULL) return 1; diff --git a/Modules/hashtable.c b/Modules/hashtable.c index d80acc6b217..b53cc240832 100644 --- a/Modules/hashtable.c +++ b/Modules/hashtable.c @@ -61,6 +61,20 @@ #define HASHTABLE_ITEM_SIZE(HT) \ (sizeof(_Py_hashtable_entry_t) + (HT)->key_size + (HT)->data_size) +#define ENTRY_READ_PDATA(TABLE, ENTRY, DATA_SIZE, PDATA) \ + do { \ + assert((DATA_SIZE) == (TABLE)->data_size); \ + Py_MEMCPY((PDATA), _Py_HASHTABLE_ENTRY_PDATA(TABLE, (ENTRY)), \ + (DATA_SIZE)); \ + } while (0) + +#define ENTRY_WRITE_PDATA(TABLE, ENTRY, DATA_SIZE, PDATA) \ + do { \ + assert((DATA_SIZE) == (TABLE)->data_size); \ + Py_MEMCPY((void *)_Py_HASHTABLE_ENTRY_PDATA((TABLE), (ENTRY)), \ + (PDATA), (DATA_SIZE)); \ + } while (0) + /* Forward declaration */ static void hashtable_rehash(_Py_hashtable_t *ht); @@ -91,21 +105,21 @@ _Py_slist_remove(_Py_slist_t *list, _Py_slist_item_t *previous, Py_uhash_t -_Py_hashtable_hash_ptr(size_t key_size, const void *pkey) +_Py_hashtable_hash_ptr(struct _Py_hashtable_t *ht, const void *pkey) { void *key; - _Py_HASHTABLE_READ_KEY(key_size, pkey, key); - return (Py_uhash_t)_Py_HashPointer((void *)key); + _Py_HASHTABLE_READ_KEY(ht, pkey, key); + return (Py_uhash_t)_Py_HashPointer(key); } int -_Py_hashtable_compare_direct(size_t key_size, const void *pkey, +_Py_hashtable_compare_direct(_Py_hashtable_t *ht, const void *pkey, const _Py_hashtable_entry_t *entry) { - const void *pkey2 = _Py_HASHTABLE_ENTRY_KEY(entry); - return (memcmp(pkey, pkey2, key_size) == 0); + const void *pkey2 = _Py_HASHTABLE_ENTRY_PKEY(entry); + return (memcmp(pkey, pkey2, ht->key_size) == 0); } @@ -245,12 +259,11 @@ _Py_hashtable_get_entry(_Py_hashtable_t *ht, assert(key_size == ht->key_size); - key_hash = ht->hash_func(key_size, pkey); + key_hash = ht->hash_func(ht, pkey); index = key_hash & (ht->num_buckets - 1); for (entry = TABLE_HEAD(ht, index); entry != NULL; entry = ENTRY_NEXT(entry)) { - if (entry->key_hash == key_hash - && ht->compare_func(key_size, pkey, entry)) + if (entry->key_hash == key_hash && ht->compare_func(ht, pkey, entry)) break; } @@ -268,13 +281,12 @@ _Py_hashtable_pop_entry(_Py_hashtable_t *ht, size_t key_size, const void *pkey, assert(key_size == ht->key_size); - key_hash = ht->hash_func(key_size, pkey); + key_hash = ht->hash_func(ht, pkey); index = key_hash & (ht->num_buckets - 1); previous = NULL; for (entry = TABLE_HEAD(ht, index); entry != NULL; entry = ENTRY_NEXT(entry)) { - if (entry->key_hash == key_hash - && ht->compare_func(key_size, pkey, entry)) + if (entry->key_hash == key_hash && ht->compare_func(ht, pkey, entry)) break; previous = entry; } @@ -287,7 +299,7 @@ _Py_hashtable_pop_entry(_Py_hashtable_t *ht, size_t key_size, const void *pkey, ht->entries--; if (data != NULL) - _Py_HASHTABLE_ENTRY_READ_PDATA(ht, entry, data_size, data); + ENTRY_READ_PDATA(ht, entry, data_size, data); ht->alloc.free(entry); if ((float)ht->entries / (float)ht->num_buckets < HASHTABLE_LOW) @@ -315,7 +327,7 @@ _Py_hashtable_set(_Py_hashtable_t *ht, size_t key_size, const void *pkey, assert(entry == NULL); #endif - key_hash = ht->hash_func(key_size, pkey); + key_hash = ht->hash_func(ht, pkey); index = key_hash & (ht->num_buckets - 1); entry = ht->alloc.malloc(HASHTABLE_ITEM_SIZE(ht)); @@ -325,8 +337,8 @@ _Py_hashtable_set(_Py_hashtable_t *ht, size_t key_size, const void *pkey, } entry->key_hash = key_hash; - _Py_HASHTABLE_ENTRY_WRITE_PKEY(key_size, entry, pkey); - _Py_HASHTABLE_ENTRY_WRITE_PDATA(ht, entry, data_size, data); + Py_MEMCPY((void *)_Py_HASHTABLE_ENTRY_PKEY(entry), pkey, ht->key_size); + ENTRY_WRITE_PDATA(ht, entry, data_size, data); _Py_slist_prepend(&ht->buckets[index], (_Py_slist_item_t*)entry); ht->entries++; @@ -348,7 +360,7 @@ _Py_hashtable_get(_Py_hashtable_t *ht, size_t key_size,const void *pkey, entry = _Py_hashtable_get_entry(ht, key_size, pkey); if (entry == NULL) return 0; - _Py_HASHTABLE_ENTRY_READ_PDATA(ht, entry, data_size, data); + ENTRY_READ_PDATA(ht, entry, data_size, data); return 1; } @@ -399,7 +411,6 @@ _Py_hashtable_foreach(_Py_hashtable_t *ht, static void hashtable_rehash(_Py_hashtable_t *ht) { - const size_t key_size = ht->key_size; size_t buckets_size, new_size, bucket; _Py_slist_t *old_buckets = NULL; size_t old_num_buckets; @@ -429,7 +440,7 @@ hashtable_rehash(_Py_hashtable_t *ht) size_t entry_index; - assert(ht->hash_func(key_size, _Py_HASHTABLE_ENTRY_KEY(entry)) == entry->key_hash); + assert(ht->hash_func(ht, _Py_HASHTABLE_ENTRY_PKEY(entry)) == entry->key_hash); next = ENTRY_NEXT(entry); entry_index = entry->key_hash & (new_size - 1); @@ -499,9 +510,9 @@ _Py_hashtable_copy(_Py_hashtable_t *src) for (bucket=0; bucket < src->num_buckets; bucket++) { entry = TABLE_HEAD(src, bucket); for (; entry; entry = ENTRY_NEXT(entry)) { - const void *pkey = _Py_HASHTABLE_ENTRY_KEY(entry); - const void *data = _Py_HASHTABLE_ENTRY_DATA(src, entry); - err = _Py_hashtable_set(dst, key_size, pkey, data_size, data); + const void *pkey = _Py_HASHTABLE_ENTRY_PKEY(entry); + const void *pdata = _Py_HASHTABLE_ENTRY_PDATA(src, entry); + err = _Py_hashtable_set(dst, key_size, pkey, data_size, pdata); if (err) { _Py_hashtable_destroy(dst); return NULL; diff --git a/Modules/hashtable.h b/Modules/hashtable.h index e3e8148fcc4..18fed096c1a 100644 --- a/Modules/hashtable.h +++ b/Modules/hashtable.h @@ -29,60 +29,52 @@ typedef struct { /* key (key_size bytes) and then data (data_size bytes) follows */ } _Py_hashtable_entry_t; -#define _Py_HASHTABLE_ENTRY_KEY(ENTRY) \ +#define _Py_HASHTABLE_ENTRY_PKEY(ENTRY) \ ((const void *)((char *)(ENTRY) \ + sizeof(_Py_hashtable_entry_t))) -#define _Py_HASHTABLE_ENTRY_DATA(TABLE, ENTRY) \ +#define _Py_HASHTABLE_ENTRY_PDATA(TABLE, ENTRY) \ ((const void *)((char *)(ENTRY) \ + sizeof(_Py_hashtable_entry_t) \ + (TABLE)->key_size)) /* Get a key value from pkey: use memcpy() rather than a pointer dereference to avoid memory alignment issues. */ -#define _Py_HASHTABLE_READ_KEY(KEY_SIZE, PKEY, DST_KEY) \ +#define _Py_HASHTABLE_READ_KEY(TABLE, PKEY, DST_KEY) \ do { \ - assert(sizeof(DST_KEY) == (KEY_SIZE)); \ - memcpy(&(DST_KEY), (PKEY), sizeof(DST_KEY)); \ + assert(sizeof(DST_KEY) == (TABLE)->key_size); \ + Py_MEMCPY(&(DST_KEY), (PKEY), sizeof(DST_KEY)); \ } while (0) -#define _Py_HASHTABLE_ENTRY_READ_KEY(KEY_SIZE, ENTRY, KEY) \ +#define _Py_HASHTABLE_ENTRY_READ_KEY(TABLE, ENTRY, KEY) \ do { \ - assert(sizeof(KEY) == (KEY_SIZE)); \ - memcpy(&(KEY), _Py_HASHTABLE_ENTRY_KEY(ENTRY), sizeof(KEY)); \ - } while (0) - -#define _Py_HASHTABLE_ENTRY_WRITE_PKEY(KEY_SIZE, ENTRY, PKEY) \ - do { \ - memcpy((void *)_Py_HASHTABLE_ENTRY_KEY(ENTRY), (PKEY), (KEY_SIZE)); \ - } while (0) - -#define _Py_HASHTABLE_ENTRY_READ_PDATA(TABLE, ENTRY, DATA_SIZE, PDATA) \ - do { \ - assert((DATA_SIZE) == (TABLE)->data_size); \ - memcpy((PDATA), _Py_HASHTABLE_ENTRY_DATA(TABLE, (ENTRY)), \ - (DATA_SIZE)); \ + assert(sizeof(KEY) == (TABLE)->key_size); \ + Py_MEMCPY(&(KEY), _Py_HASHTABLE_ENTRY_PKEY(ENTRY), sizeof(KEY)); \ } while (0) #define _Py_HASHTABLE_ENTRY_READ_DATA(TABLE, ENTRY, DATA) \ - _Py_HASHTABLE_ENTRY_READ_PDATA((TABLE), (ENTRY), sizeof(DATA), &(DATA)) - -#define _Py_HASHTABLE_ENTRY_WRITE_PDATA(TABLE, ENTRY, DATA_SIZE, PDATA) \ do { \ - assert((DATA_SIZE) == (TABLE)->data_size); \ - memcpy((void *)_Py_HASHTABLE_ENTRY_DATA((TABLE), (ENTRY)), \ - (PDATA), (DATA_SIZE)); \ + assert(sizeof(DATA) == (TABLE)->data_size); \ + Py_MEMCPY(&(DATA), _Py_HASHTABLE_ENTRY_PDATA(TABLE, (ENTRY)), \ + sizeof(DATA)); \ } while (0) #define _Py_HASHTABLE_ENTRY_WRITE_DATA(TABLE, ENTRY, DATA) \ - _Py_HASHTABLE_ENTRY_WRITE_PDATA(TABLE, ENTRY, sizeof(DATA), &(DATA)) + do { \ + assert(sizeof(DATA) == (TABLE)->data_size); \ + Py_MEMCPY((void *)_Py_HASHTABLE_ENTRY_PDATA((TABLE), (ENTRY)), \ + &(DATA), sizeof(DATA)); \ + } while (0) /* _Py_hashtable: prototypes */ -typedef Py_uhash_t (*_Py_hashtable_hash_func) (size_t key_size, +/* Forward declaration */ +struct _Py_hashtable_t; + +typedef Py_uhash_t (*_Py_hashtable_hash_func) (struct _Py_hashtable_t *ht, const void *pkey); -typedef int (*_Py_hashtable_compare_func) (size_t key_size, +typedef int (*_Py_hashtable_compare_func) (struct _Py_hashtable_t *ht, const void *pkey, const _Py_hashtable_entry_t *he); @@ -97,7 +89,7 @@ typedef struct { /* _Py_hashtable: table */ -typedef struct { +typedef struct _Py_hashtable_t { size_t num_buckets; size_t entries; /* Total number of entries in the table. */ _Py_slist_t *buckets; @@ -111,12 +103,12 @@ typedef struct { /* hash a pointer (void*) */ PyAPI_FUNC(Py_uhash_t) _Py_hashtable_hash_ptr( - size_t key_size, + struct _Py_hashtable_t *ht, const void *pkey); /* comparison using memcmp() */ PyAPI_FUNC(int) _Py_hashtable_compare_direct( - size_t key_size, + _Py_hashtable_t *ht, const void *pkey, const _Py_hashtable_entry_t *entry); diff --git a/Python/marshal.c b/Python/marshal.c index 3be77a82f43..627a8428e53 100644 --- a/Python/marshal.c +++ b/Python/marshal.c @@ -588,7 +588,7 @@ w_decref_entry(_Py_hashtable_t *ht, _Py_hashtable_entry_t *entry, { PyObject *entry_key; - _Py_HASHTABLE_ENTRY_READ_KEY(ht->key_size, entry, entry_key); + _Py_HASHTABLE_ENTRY_READ_KEY(ht, entry, entry_key); Py_XDECREF(entry_key); return 0; }