philipp/hdf5
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Revert "Switch ID code to use a hash table instead of a skip list (#52)" (#104)

Browse Source 
This reverts commit a50d211755.
This commit is contained in:
Dana Robinson
2020-11-19 18:54:00 -08:00
committed by GitHub
parent fc7ac84e19
commit 793b3d1bae
7 changed files with 30 additions and 1367 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
.github/workflows/clang-format-check.yml vendored
View File

@@ -14,4 +14,4 @@ jobs:
extensions: 'c,h,cpp,hpp' extensions: 'c,h,cpp,hpp'
clangFormatVersion: 10 clangFormatVersion: 10
style: file style: file
exclude: './config ./hl/src/H5LTanalyze.c ./hl/src/H5LTparse.c ./hl/src/H5LTparse.h ./src/H5Epubgen.h ./src/H5Einit.h ./src/H5Eterm.h ./src/H5Edefin.h ./src/H5version.h ./src/H5overflow.h ./src/uthash.h' exclude: './config ./hl/src/H5LTanalyze.c ./hl/src/H5LTparse.c ./hl/src/H5LTparse.h ./src/H5Epubgen.h ./src/H5Einit.h ./src/H5Eterm.h ./src/H5Edefin.h ./src/H5version.h ./src/H5overflow.h'

3
MANIFEST
View File

@@ -1045,11 +1045,10 @@
./src/H5Zshuffle.c ./src/H5Zshuffle.c
./src/H5Zszip.c ./src/H5Zszip.c
./src/H5Ztrans.c ./src/H5Ztrans.c
./src/H5win32defs.h
./src/Makefile.am ./src/Makefile.am
./src/hdf5.h ./src/hdf5.h
./src/libhdf5.settings.in ./src/libhdf5.settings.in
./src/uthash.h ./src/H5win32defs.h
./test/AtomicWriterReader.txt ./test/AtomicWriterReader.txt
./test/COPYING ./test/COPYING

1
bin/format_source
View File

@@ -19,7 +19,6 @@ find . -type d \( -path ./config \) -prune \
-or -name H5Edefin.h \ -or -name H5Edefin.h \
-or -name H5version.h \ -or -name H5version.h \
-or -name H5overflow.h \ -or -name H5overflow.h \
-or -name uthash.h \
\) \) \ \) \) \
-and \( -iname *.h -or -iname *.c -or -iname *.cpp -or -iname *.hpp \) \) \ -and \( -iname *.h -or -iname *.c -or -iname *.cpp -or -iname *.hpp \) \) \
| xargs clang-format -style=file -i -fallback-style=none | xargs clang-format -style=file -i -fallback-style=none

1
bin/format_source_patch
View File

@@ -19,7 +19,6 @@ find . -type d \( -path ./config \) -prune \
-or -name H5Edefin.h \ -or -name H5Edefin.h \
-or -name H5version.h \ -or -name H5version.h \
-or -name H5overflow.h \ -or -name H5overflow.h \
-or -name uthash.h \
\) \) \ \) \) \
-and \( -iname *.h -or -iname *.c -or -iname *.cpp -or -iname *.hpp \) \) \ -and \( -iname *.h -or -iname *.c -or -iname *.cpp -or -iname *.hpp \) \) \
| xargs clang-format -style=file -i -fallback-style=none | xargs clang-format -style=file -i -fallback-style=none

165
src/H5I.c
View File

@@ -43,8 +43,6 @@
#include "H5Tpkg.h" /* Datatypes */ #include "H5Tpkg.h" /* Datatypes */
#include "H5VLprivate.h" /* Virtual Object Layer */ #include "H5VLprivate.h" /* Virtual Object Layer */
#include "uthash.h" /* Hash table functionality */
/* Local Macros */ /* Local Macros */
/* Combine a Type number and an atom index into an atom */ /* Combine a Type number and an atom index into an atom */
@@ -54,11 +52,10 @@
/* Atom information structure used */ /* Atom information structure used */
typedef struct H5I_id_info_t { typedef struct H5I_id_info_t {
hid_t id; /* ID for this info */ hid_t id; /* ID for this info */
unsigned count; /* ref. count for this atom */ unsigned count; /* ref. count for this atom */
unsigned app_count; /* ref. count of application visible atoms */ unsigned app_count; /* ref. count of application visible atoms */
const void * obj_ptr; /* pointer associated with the atom */ const void *obj_ptr; /* pointer associated with the atom */
UT_hash_handle hh; /* Hash table handle (must be LAST) */
} H5I_id_info_t; } H5I_id_info_t;
/* ID type structure used */ /* ID type structure used */
@@ -68,14 +65,7 @@ typedef struct {
uint64_t id_count; /* Current number of IDs held */ uint64_t id_count; /* Current number of IDs held */
uint64_t nextid; /* ID to use for the next atom */ uint64_t nextid; /* ID to use for the next atom */
H5I_id_info_t * last_info; /* Info for most recent ID looked up */ H5I_id_info_t * last_info; /* Info for most recent ID looked up */
/* Library IDs are stored in a hash table, user IDs are stored in a skip H5SL_t * ids; /* Pointer to skip list that stores IDs */
* list, which is slower but handles the case where ID delete callbacks
* delete other IDs via a mark-and-sweep scheme.
*/
union {
H5I_id_info_t *hash_table; /* Hash table pointer for this ID type */
H5SL_t * skip_list; /* Pointer to skip list that stores IDs */
} ids;
} H5I_id_type_t; } H5I_id_type_t;
typedef struct { typedef struct {
@@ -179,25 +169,15 @@ H5I_term_package(void)
/* How many types are still being used? */ /* How many types are still being used? */
for (type = 0; type < H5I_next_type; type++) for (type = 0; type < H5I_next_type; type++)
if (H5I_IS_LIB_TYPE(type)) { if ((type_ptr = H5I_id_type_list_g[type]) && type_ptr->ids)
if ((type_ptr = H5I_id_type_list_g[type]) && type_ptr->ids.hash_table) n++;
n++;
}
else {
if ((type_ptr = H5I_id_type_list_g[type]) && type_ptr->ids.skip_list)
n++;
}
/* If no types are used then clean up */ /* If no types are used then clean up */
if (0 == n) { if (0 == n) {
for (type = 0; type < H5I_next_type; type++) { for (type = 0; type < H5I_next_type; type++) {
type_ptr = H5I_id_type_list_g[type]; type_ptr = H5I_id_type_list_g[type];
if (type_ptr) { if (type_ptr) {
if (H5I_IS_LIB_TYPE(type)) HDassert(NULL == type_ptr->ids);
HDassert(NULL == type_ptr->ids.hash_table);
else
HDassert(NULL == type_ptr->ids.skip_list);
type_ptr = H5FL_FREE(H5I_id_type_t, type_ptr); type_ptr = H5FL_FREE(H5I_id_type_t, type_ptr);
H5I_id_type_list_g[type] = NULL; H5I_id_type_list_g[type] = NULL;
n++; n++;
@@ -333,12 +313,8 @@ H5I_register_type(const H5I_class_t *cls)
type_ptr->id_count = 0; type_ptr->id_count = 0;
type_ptr->nextid = cls->reserved; type_ptr->nextid = cls->reserved;
type_ptr->last_info = NULL; type_ptr->last_info = NULL;
if (H5I_IS_LIB_TYPE(cls->type_id)) if (NULL == (type_ptr->ids = H5SL_create(H5SL_TYPE_HID, NULL)))
type_ptr->ids.hash_table = NULL; HGOTO_ERROR(H5E_ATOM, H5E_CANTCREATE, FAIL, "skip list creation failed")
else {
if (NULL == (type_ptr->ids.skip_list = H5SL_create(H5SL_TYPE_HID, NULL)))
HGOTO_ERROR(H5E_ATOM, H5E_CANTCREATE, FAIL, "skip list creation failed")
}
} /* end if */ } /* end if */
/* Increment the count of the times this type has been initialized */ /* Increment the count of the times this type has been initialized */
@@ -347,10 +323,9 @@ H5I_register_type(const H5I_class_t *cls)
done: done:
if (ret_value < 0) { /* Clean up on error */ if (ret_value < 0) { /* Clean up on error */
if (type_ptr) { if (type_ptr) {
if (FALSE == H5I_IS_LIB_TYPE(cls->type_id)) if (type_ptr->ids)
if (type_ptr->ids.skip_list) H5SL_close(type_ptr->ids);
H5SL_close(type_ptr->ids.skip_list); (void)H5FL_FREE(H5I_id_type_t, type_ptr);
H5FL_FREE(H5I_id_type_t, type_ptr);
} /* end if */ } /* end if */
} /* end if */ } /* end if */
@@ -581,23 +556,8 @@ H5I_clear_type(H5I_type_t type, hbool_t force, hbool_t app_ref)
udata.force = force; udata.force = force;
udata.app_ref = app_ref; udata.app_ref = app_ref;
if (H5I_IS_LIB_TYPE(type)) { /* Attempt to free all ids in the type */
H5I_id_info_t *item = NULL; if (H5SL_try_free_safe(udata.type_ptr->ids, H5I__clear_type_cb, &udata) < 0)
H5I_id_info_t *tmp = NULL;
/* This is a "delete-safe" iteration */
HASH_ITER(hh, udata.type_ptr->ids.hash_table, item, tmp)
{
htri_t ret = H5I__clear_type_cb((void *)item, NULL, (void *)&udata);
if (FAIL == ret)
HGOTO_ERROR(H5E_ATOM, H5E_BADITER, FAIL, "iteration failed")
if (TRUE == ret)
HASH_DELETE(hh, udata.type_ptr->ids.hash_table, item);
}
}
else
/* Attempt to free all ids in the type */
if (H5SL_try_free_safe(udata.type_ptr->ids.skip_list, H5I__clear_type_cb, &udata) < 0)
HGOTO_ERROR(H5E_ATOM, H5E_CANTDELETE, FAIL, "can't free ids in type") HGOTO_ERROR(H5E_ATOM, H5E_CANTDELETE, FAIL, "can't free ids in type")
done: done:
@@ -742,15 +702,9 @@ H5I__destroy_type(H5I_type_t type)
if (type_ptr->cls->flags & H5I_CLASS_IS_APPLICATION) if (type_ptr->cls->flags & H5I_CLASS_IS_APPLICATION)
type_ptr->cls = H5FL_FREE(H5I_class_t, (void *)type_ptr->cls); type_ptr->cls = H5FL_FREE(H5I_class_t, (void *)type_ptr->cls);
if (H5I_IS_LIB_TYPE(type)) { if (H5SL_close(type_ptr->ids) < 0)
HASH_CLEAR(hh, type_ptr->ids.hash_table); HGOTO_ERROR(H5E_ATOM, H5E_CANTCLOSEOBJ, FAIL, "can't close skip list")
type_ptr->ids.hash_table = NULL; type_ptr->ids = NULL;
}
else {
if (H5SL_close(type_ptr->ids.skip_list) < 0)
HGOTO_ERROR(H5E_ATOM, H5E_CANTCLOSEOBJ, FAIL, "can't close skip list")
type_ptr->ids.skip_list = NULL;
}
type_ptr = H5FL_FREE(H5I_id_type_t, type_ptr); type_ptr = H5FL_FREE(H5I_id_type_t, type_ptr);
H5I_id_type_list_g[type] = NULL; H5I_id_type_list_g[type] = NULL;
@@ -829,10 +783,7 @@ H5I_register(H5I_type_t type, const void *object, hbool_t app_ref)
id_ptr->obj_ptr = object; id_ptr->obj_ptr = object;
/* Insert into the type */ /* Insert into the type */
if (H5I_IS_LIB_TYPE(type)) if (H5SL_insert(type_ptr->ids, id_ptr, &id_ptr->id) < 0)
/* Insert into the hash table */
HASH_ADD(hh, type_ptr->ids.hash_table, id, sizeof(hid_t), id_ptr);
else if (H5SL_insert(type_ptr->ids.skip_list, id_ptr, &id_ptr->id) < 0)
HGOTO_ERROR(H5E_ATOM, H5E_CANTINSERT, H5I_INVALID_HID, "can't insert ID node into skip list") HGOTO_ERROR(H5E_ATOM, H5E_CANTINSERT, H5I_INVALID_HID, "can't insert ID node into skip list")
type_ptr->id_count++; type_ptr->id_count++;
type_ptr->nextid++; type_ptr->nextid++;
@@ -909,10 +860,7 @@ H5I_register_using_existing_id(H5I_type_t type, void *object, hbool_t app_ref, h
id_ptr->obj_ptr = object; id_ptr->obj_ptr = object;
/* Insert into the type */ /* Insert into the type */
if (H5I_IS_LIB_TYPE(type)) if (H5SL_insert(type_ptr->ids, id_ptr, &id_ptr->id) < 0)
/* Insert into the hash table */
HASH_ADD(hh, type_ptr->ids.hash_table, id, sizeof(hid_t), id_ptr);
else if (H5SL_insert(type_ptr->ids.skip_list, id_ptr, &id_ptr->id) < 0)
HGOTO_ERROR(H5E_ATOM, H5E_CANTINSERT, FAIL, "can't insert ID node into skip list") HGOTO_ERROR(H5E_ATOM, H5E_CANTINSERT, FAIL, "can't insert ID node into skip list")
type_ptr->id_count++; type_ptr->id_count++;
@@ -1253,7 +1201,7 @@ H5I__remove_verify(hid_t id, H5I_type_t id_type)
static void * static void *
H5I__remove_common(H5I_id_type_t *type_ptr, hid_t id) H5I__remove_common(H5I_id_type_t *type_ptr, hid_t id)
{ {
H5I_id_info_t *curr_id = NULL; /* Pointer to the current atom */ H5I_id_info_t *curr_id; /* Pointer to the current atom */
void * ret_value = NULL; /* Return value */ void * ret_value = NULL; /* Return value */
FUNC_ENTER_STATIC FUNC_ENTER_STATIC
@@ -1261,16 +1209,8 @@ H5I__remove_common(H5I_id_type_t *type_ptr, hid_t id)
/* Sanity check */ /* Sanity check */
HDassert(type_ptr); HDassert(type_ptr);
/* Delete the node */ /* Get the ID node for the ID */
if (H5I_IS_LIB_TYPE(type_ptr->cls->type_id)) { if (NULL == (curr_id = (H5I_id_info_t *)H5SL_remove(type_ptr->ids, &id)))
/* Remove the node from the hash table */
HASH_FIND(hh, type_ptr->ids.hash_table, &id, sizeof(hid_t), curr_id);
if (curr_id)
HASH_DELETE(hh, type_ptr->ids.hash_table, curr_id);
else
HGOTO_ERROR(H5E_ATOM, H5E_CANTDELETE, NULL, "can't remove ID node from hash table")
}
else if (NULL == (curr_id = (H5I_id_info_t *)H5SL_remove(type_ptr->ids.skip_list, &id)))
HGOTO_ERROR(H5E_ATOM, H5E_CANTDELETE, NULL, "can't remove ID node from skip list") HGOTO_ERROR(H5E_ATOM, H5E_CANTDELETE, NULL, "can't remove ID node from skip list")
/* Check if this ID was the last one accessed */ /* Check if this ID was the last one accessed */
@@ -1975,7 +1915,7 @@ H5Isearch(H5I_type_t type, H5I_search_func_t func, void *key)
/* Note that H5I_iterate returns an error code. We ignore it /* Note that H5I_iterate returns an error code. We ignore it
* here, as we can't do anything with it without revising the API. * here, as we can't do anything with it without revising the API.
*/ */
H5I_iterate(type, H5I__search_cb, &udata, TRUE); (void)H5I_iterate(type, H5I__search_cb, &udata, TRUE);
/* Set return value */ /* Set return value */
ret_value = udata.ret_obj; ret_value = udata.ret_obj;
@@ -2167,20 +2107,7 @@ H5I_iterate(H5I_type_t type, H5I_search_func_t func, void *udata, hbool_t app_re
iter_udata.obj_type = type; iter_udata.obj_type = type;
/* Iterate over IDs */ /* Iterate over IDs */
if (H5I_IS_LIB_TYPE(type)) { if ((iter_status = H5SL_iterate(type_ptr->ids, H5I__iterate_cb, &iter_udata)) < 0)
H5I_id_info_t *item = NULL;
H5I_id_info_t *tmp = NULL;
HASH_ITER(hh, type_ptr->ids.hash_table, item, tmp)
{
int ret = H5I__iterate_cb((void *)item, NULL, (void *)&iter_udata);
if (H5_ITER_ERROR == ret)
HGOTO_ERROR(H5E_ATOM, H5E_BADITER, FAIL, "iteration failed")
if (H5_ITER_STOP == ret)
break;
}
}
else if ((iter_status = H5SL_iterate(type_ptr->ids.skip_list, H5I__iterate_cb, &iter_udata)) < 0)
HGOTO_ERROR(H5E_ATOM, H5E_BADITER, FAIL, "iteration failed") HGOTO_ERROR(H5E_ATOM, H5E_BADITER, FAIL, "iteration failed")
} /* end if */ } /* end if */
@@ -2222,10 +2149,7 @@ H5I__find_id(hid_t id)
ret_value = type_ptr->last_info; ret_value = type_ptr->last_info;
else { else {
/* Locate the ID node for the ID */ /* Locate the ID node for the ID */
if (H5I_IS_LIB_TYPE(H5I_TYPE(id))) ret_value = (H5I_id_info_t *)H5SL_search(type_ptr->ids, &id);
HASH_FIND(hh, type_ptr->ids.hash_table, &id, sizeof(hid_t), ret_value);
else
ret_value = (H5I_id_info_t *)H5SL_search(type_ptr->ids.skip_list, &id);
/* Remember this ID */ /* Remember this ID */
type_ptr->last_info = ret_value; type_ptr->last_info = ret_value;
@@ -2405,20 +2329,7 @@ H5I_find_id(const void *object, H5I_type_t type, hid_t *id)
udata.ret_id = H5I_INVALID_HID; udata.ret_id = H5I_INVALID_HID;
/* Iterate over IDs for the ID type */ /* Iterate over IDs for the ID type */
if (H5I_IS_LIB_TYPE(type)) { if ((iter_status = H5SL_iterate(type_ptr->ids, H5I__find_id_cb, &udata)) < 0)
H5I_id_info_t *item = NULL;
H5I_id_info_t *tmp = NULL;
HASH_ITER(hh, type_ptr->ids.hash_table, item, tmp)
{
int ret = H5I__find_id_cb((void *)item, NULL, (void *)&udata);
if (H5_ITER_ERROR == ret)
HGOTO_ERROR(H5E_ATOM, H5E_BADITER, FAIL, "iteration failed")
if (H5_ITER_STOP == ret)
break;
}
}
else if ((iter_status = H5SL_iterate(type_ptr->ids.skip_list, H5I__find_id_cb, &udata)) < 0)
HGOTO_ERROR(H5E_ATOM, H5E_BADITER, FAIL, "iteration failed") HGOTO_ERROR(H5E_ATOM, H5E_BADITER, FAIL, "iteration failed")
*id = udata.ret_id; *id = udata.ret_id;
@@ -2542,27 +2453,7 @@ H5I_dump_ids_for_type(H5I_type_t type)
/* List */ /* List */
if (type_ptr->id_count > 0) { if (type_ptr->id_count > 0) {
HDfprintf(stderr, " List:\n"); HDfprintf(stderr, " List:\n");
H5SL_iterate(type_ptr->ids, H5I__id_dump_cb, &type);
if (H5I_IS_LIB_TYPE(type)) {
H5I_id_info_t *item = NULL;
H5I_id_info_t *tmp = NULL;
/* Normally we care about the callback's return value
* (H5I_ITER_CONT, etc.), but this is an iteration over all
* the IDs so we don't care.
*
* XXX: Update this to emit an error message on errors?
*/
HDfprintf(stderr, " (HASH TABLE)\n");
HASH_ITER(hh, type_ptr->ids.hash_table, item, tmp)
{
H5I__id_dump_cb((void *)item, NULL, (void *)&type);
}
}
else {
HDfprintf(stderr, " (SKIP LIST)\n");
H5SL_iterate(type_ptr->ids.skip_list, H5I__id_dump_cb, &type);
}
} }
} }
else else

1150
src/uthash.h
View File

File diff suppressed because it is too large Load Diff

75
test/tid.c
View File

@@ -763,79 +763,6 @@ out:
return -1; return -1;
} /* end test_remove_clear_type() */ } /* end test_remove_clear_type() */
/* Test that IDs can be deleted while iterating */
#define N_ITERATE_IDS 128
/* A callback that (maybe) closes a random ID when invoked */
static herr_t
iterate_delete_op(hid_t H5_ATTR_UNUSED id, void *udata)
{
hid_t *ids = (hid_t *)udata;
int closed_index;
hid_t closed_id;
/* Maybe close an ID
*
* Do nothing when an ID has already been closed so we don't
* close ALL the IDs.
*
* Closing the ID we're currently iterating on is also acceptable.
*/
closed_index = HDrandom() % N_ITERATE_IDS;
closed_id = ids[closed_index];
if (closed_id != H5I_INVALID_HID) {
if (H5Sclose(closed_id) < 0)
return -1;
ids[closed_index] = H5I_INVALID_HID;
}
return 0;
} /* end iterate_delete_op() */
static int
test_iteration_remove(void)
{
hid_t *ids = NULL;
int i;
/* Create a bunch of IDs of a single library type */
if (NULL == (ids = malloc(N_ITERATE_IDS * sizeof(hid_t))))
goto error;
for (i = 0; i < N_ITERATE_IDS; i++)
if ((ids[i] = H5Screate(H5S_NULL)) == H5I_INVALID_HID)
goto error;
/* Iterate over the IDs, (maybe) closing a random one in the callback */
if (H5Iiterate(H5I_DATASPACE, iterate_delete_op, ids) < 0)
goto error;
/* Close and free */
for (i = 0; i < N_ITERATE_IDS; i++) {
if (ids[i] == H5I_INVALID_HID)
continue;
if (H5Sclose(ids[i]) < 0)
goto error;
ids[i] = H5I_INVALID_HID;
}
HDfree(ids);
return 0;
error:
H5E_BEGIN_TRY
if (ids != NULL)
for (i = 0; i < N_ITERATE_IDS; i++)
H5Sclose(ids[i]);
H5E_END_TRY
HDfree(ids);
return -1;
} /* end test_iteration_remove() */
void void
test_ids(void) test_ids(void)
{ {
@@ -854,6 +781,4 @@ test_ids(void)
TestErrPrintf("ID type list test failed\n"); TestErrPrintf("ID type list test failed\n");
if (test_remove_clear_type() < 0) if (test_remove_clear_type() < 0)
TestErrPrintf("ID remove during H5Iclear_type test failed\n"); TestErrPrintf("ID remove during H5Iclear_type test failed\n");
if (test_iteration_remove() < 0)
TestErrPrintf("Removing random IDs while iterating failed\n");
} }