From 869ed6e043dfc20b6b6dfda6728c3442ef0409be Mon Sep 17 00:00:00 2001 From: James Laird Date: Fri, 5 Jan 2007 13:30:59 -0500 Subject: [svn-r13113] Refactoring. Cleaned up code, added a few sanity checks. Extracted duplicated code into functions. Tested on Windows, juniper, and kagiso. Will test on copper next. --- src/H5Omessage.c | 6 +- src/H5Oprivate.h | 2 - src/H5SM.c | 358 +++++++++++++++++++++++++++++++++++------------------- src/H5SMbtree2.c | 11 +- src/H5SMcache.c | 20 ++- src/H5SMpkg.h | 12 +- src/H5SMprivate.h | 2 +- test/tattr.c | 3 - test/tfile.c | 4 - test/th5s.c | 4 - test/tmisc.c | 1 - test/tsohm.c | 6 +- 12 files changed, 265 insertions(+), 164 deletions(-) diff --git a/src/H5Omessage.c b/src/H5Omessage.c index 1dd0287..515e131 100644 --- a/src/H5Omessage.c +++ b/src/H5Omessage.c @@ -1537,6 +1537,7 @@ H5O_msg_get_share(unsigned type_id, const void *mesg, H5O_shared_t *share) /* Check args */ HDassert(type_id < NELMTS(H5O_msg_class_g)); + HDassert(type_id != H5O_SHARED_ID); type = H5O_msg_class_g[type_id]; /* map the type ID to the actual type object */ HDassert(type); HDassert(type->get_share); @@ -1576,12 +1577,11 @@ H5O_msg_is_shared(unsigned type_id, const void *mesg) /* Check args */ HDassert(type_id < NELMTS(H5O_msg_class_g)); + HDassert(type_id != H5O_SHARED_ID); type = H5O_msg_class_g[type_id]; /* map the type ID to the actual type object */ HDassert(type); HDassert(mesg); - HDassert(type_id != H5O_SHARED_ID); /* JAMES: check for this mistake elsewhere, too */ - /* If there is no is_shared function, then obviously it's not a shared message! */ if(!(type->is_shared)) ret_value = FALSE; @@ -1617,6 +1617,7 @@ H5O_msg_set_share(unsigned type_id, H5O_shared_t *share, void *mesg) /* Check args */ HDassert(share); HDassert(type_id < NELMTS(H5O_msg_class_g)); + HDassert(type_id != H5O_SHARED_ID); type = H5O_msg_class_g[type_id]; /* map the type ID to the actual type object */ HDassert(type); HDassert(type->set_share); @@ -1659,6 +1660,7 @@ H5O_msg_reset_share(unsigned type_id, void *mesg) /* Check args */ HDassert(type_id < NELMTS(H5O_msg_class_g)); + HDassert(type_id != H5O_SHARED_ID); type = H5O_msg_class_g[type_id]; /* map the type ID to the actual type object */ HDassert(type); HDassert(type->set_share); diff --git a/src/H5Oprivate.h b/src/H5Oprivate.h index 615d646..9330d4a 100644 --- a/src/H5Oprivate.h +++ b/src/H5Oprivate.h @@ -76,9 +76,7 @@ typedef uint64_t H5SM_fheap_id_t; #define H5O_UPDATE_TIME 0x01u /* Hash value constants */ -/* JAMES: undefined hash value may not be great */ #define H5O_HASH_SIZE 32 -#define H5O_HASH_UNDEF ((uint32_t)FAIL) /* ========= Object Creation properties ============ */ #define H5O_CRT_ATTR_MAX_COMPACT_NAME "max compact attr" /* Max. # of attributes to store compactly */ diff --git a/src/H5SM.c b/src/H5SM.c index 9f93bd4..74315d7 100755 --- a/src/H5SM.c +++ b/src/H5SM.c @@ -52,8 +52,14 @@ /********************/ /* Local Prototypes */ /********************/ -static herr_t H5SM_create_index(H5F_t *f, H5SM_index_header_t *header, hid_t dxpl_id); +static herr_t H5SM_create_index(H5F_t *f, H5SM_index_header_t *header, + hid_t dxpl_id); +static herr_t H5SM_delete_index(H5F_t *f, H5SM_index_header_t *header, + hid_t dxpl_id, hbool_t delete_heap); static haddr_t H5SM_create_list(H5F_t *f, H5SM_index_header_t *header, hid_t dxpl_id); +static herr_t H5SM_convert_list_to_btree(H5F_t * f, H5SM_index_header_t * header, + H5SM_list_t **_list, hid_t dxpl_id); +static herr_t H5SM_convert_btree_to_list(H5F_t * f, H5SM_index_header_t * header, hid_t dxpl_id); static herr_t H5SM_write_mesg(H5F_t *f, hid_t dxpl_id, H5SM_index_header_t *header, unsigned type_id, void *mesg, unsigned *cache_flags_ptr); static herr_t H5SM_delete_from_index(H5F_t *f, hid_t dxpl_id, @@ -104,7 +110,7 @@ H5SM_init(H5F_t *f, H5P_genplist_t * fc_plist, hid_t dxpl_id) H5SM_master_table_t *table = NULL; haddr_t table_addr = HADDR_UNDEF; unsigned num_indexes; - unsigned list_to_btree, btree_to_list; + unsigned list_max, btree_min; unsigned index_type_flags[H5SM_MAX_NINDEXES]; unsigned minsizes[H5SM_MAX_NINDEXES]; unsigned type_flags_used; @@ -127,9 +133,9 @@ H5SM_init(H5F_t *f, H5P_genplist_t * fc_plist, hid_t dxpl_id) HGOTO_ERROR(H5E_PLIST, H5E_CANTGET, FAIL, "can't get number of indexes") if(H5P_get(fc_plist, H5F_CRT_SHMSG_INDEX_TYPES_NAME, &index_type_flags)<0) HGOTO_ERROR(H5E_PLIST, H5E_CANTGET, FAIL, "can't get SOHM type flags") - if(H5P_get(fc_plist, H5F_CRT_SHMSG_LIST_MAX_NAME, &list_to_btree)<0) + if(H5P_get(fc_plist, H5F_CRT_SHMSG_LIST_MAX_NAME, &list_max)<0) HGOTO_ERROR(H5E_PLIST, H5E_CANTGET, FAIL, "can't get SOHM list maximum") - if(H5P_get(fc_plist, H5F_CRT_SHMSG_BTREE_MIN_NAME, &btree_to_list)<0) + if(H5P_get(fc_plist, H5F_CRT_SHMSG_BTREE_MIN_NAME, &btree_min)<0) HGOTO_ERROR(H5E_PLIST, H5E_CANTGET, FAIL, "can't get SOHM btree minimum") if(H5P_get(fc_plist, H5F_CRT_SHMSG_INDEX_MINSIZE_NAME, &minsizes) < 0) HGOTO_ERROR(H5E_PLIST, H5E_CANTGET, FAIL, "can't get SOHM message min sizes") @@ -161,7 +167,7 @@ H5SM_init(H5F_t *f, H5P_genplist_t * fc_plist, hid_t dxpl_id) * list max has to be greater than or equal to one less than the btree * min. */ - HDassert(list_to_btree + 1 >= btree_to_list); + HDassert(list_max + 1 >= btree_min); HDassert(table->num_indexes > 0 && table->num_indexes <= H5SM_MAX_NINDEXES); /* Allocate the SOHM indexes as an array. */ @@ -173,15 +179,15 @@ H5SM_init(H5F_t *f, H5P_genplist_t * fc_plist, hid_t dxpl_id) */ for(x=0; xnum_indexes; x++) { - table->indexes[x].btree_to_list = btree_to_list; - table->indexes[x].list_to_btree = list_to_btree; + table->indexes[x].btree_min = btree_min; + table->indexes[x].list_max = list_max; table->indexes[x].mesg_types = index_type_flags[x]; table->indexes[x].min_mesg_size = minsizes[x]; table->indexes[x].index_addr = HADDR_UNDEF; table->indexes[x].heap_addr = HADDR_UNDEF; table->indexes[x].num_messages = 0; /* Indexes start as lists unless the list-to-btree threshold is zero */ - if(table->indexes[x].list_to_btree > 0) { + if(table->indexes[x].list_max > 0) { table->indexes[x].index_type = H5SM_LIST; } else { table->indexes[x].index_type = H5SM_BTREE; @@ -384,7 +390,6 @@ H5SM_get_fheap_addr(H5F_t *f, unsigned type_id, hid_t dxpl_id) if(NULL == (table = (H5SM_master_table_t *)H5AC_protect(f, dxpl_id, H5AC_SOHM_TABLE, f->shared->sohm_addr, NULL, NULL, H5AC_READ))) HGOTO_ERROR(H5E_CACHE, H5E_CANTPROTECT, HADDR_UNDEF, "unable to load SOHM master table") - /* JAMES! */ if((index_num = H5SM_get_index(table, type_id)) < 0) HGOTO_ERROR(H5E_SOHM, H5E_CANTPROTECT, HADDR_UNDEF, "unable to find correct SOHM index") @@ -402,7 +407,7 @@ done: /*------------------------------------------------------------------------- * Function: H5SM_create_index * - * Purpose: Allocates storage for an index. + * Purpose: Allocates storage for an index, populating the HEADER struct. * * Return: Non-negative on success/negative on failure * @@ -427,10 +432,10 @@ H5SM_create_index(H5F_t *f, H5SM_index_header_t *header, hid_t dxpl_id) HDassert(header); HDassert(header->index_addr == HADDR_UNDEF); - HDassert(header->btree_to_list <= header->list_to_btree + 1); + HDassert(header->btree_min <= header->list_max + 1); /* In most cases, the index starts as a list */ - if(header->list_to_btree > 0) + if(header->list_max > 0) { header->index_type = H5SM_LIST; @@ -487,6 +492,70 @@ done: /*------------------------------------------------------------------------- + * Function: H5SM_create_index + * + * Purpose: De-allocates storage for an index whose header is HEADER. + * + * If DELETE_HEAP is TRUE, deletes the index's heap, eliminating + * it completely. + * + * If DELETE_HEAP is FALSE, the heap is not deleted. This is + * useful when deleting only the index header as the index is + * converted from a list to a B-tree and back again. + * + * Return: Non-negative on success/negative on failure + * + * Programmer: James Laird + * Thursday, January 4, 2006 + * + *------------------------------------------------------------------------- + */ +static herr_t +H5SM_delete_index(H5F_t *f, H5SM_index_header_t *header, hid_t dxpl_id, hbool_t delete_heap) +{ + hsize_t list_size; /* Size of list on disk */ + herr_t ret_value = SUCCEED; + + FUNC_ENTER_NOAPI(H5SM_delete_index, FAIL) + + /* Determine whether index is a list or a B-tree. */ + if(header->index_type == H5SM_LIST) { + /* Eject entry from cache */ + if(H5AC_expunge_entry(f, dxpl_id, H5AC_SOHM_LIST, header->index_addr) < 0) + HGOTO_ERROR(H5E_HEAP, H5E_CANTREMOVE, FAIL, "unable to remove list index from cache") + + /* Free the file space used */ + list_size = H5SM_LIST_SIZE(f, header->list_max); + if(H5MF_xfree(f, H5FD_MEM_SOHM_INDEX, dxpl_id, header->index_addr, list_size) < 0) + HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, FAIL, "unable to free shared message list") + } else { + HDassert(header->index_type == H5SM_BTREE); + + /* Delete from the B-tree. */ + if(H5B2_delete(f, dxpl_id, H5SM_INDEX, header->index_addr, NULL, NULL) < 0) + HGOTO_ERROR(H5E_BTREE, H5E_CANTDELETE, FAIL, "unable to delete B-tree") + + /* Revert to list unless B-trees can have zero records */ + if(header->btree_min > 0) + header->index_type = H5SM_LIST; + } + + /* Free the index's heap if requested. */ + if(delete_heap == TRUE) { + if(H5HF_delete(f, dxpl_id, header->heap_addr) < 0) + HGOTO_ERROR(H5E_SOHM, H5E_CANTDELETE, FAIL, "unable to delete fractal heap") + } + + header->index_addr = HADDR_UNDEF; + header->heap_addr = HADDR_UNDEF; + header->num_messages = 0; + +done: + FUNC_LEAVE_NOAPI(ret_value) +}/* end H5SM_delete_index */ + + +/*------------------------------------------------------------------------- * Function: H5SM_create_list * * Purpose: Creates a list of SOHM messages. @@ -516,7 +585,7 @@ H5SM_create_list(H5F_t *f, H5SM_index_header_t * header, hid_t dxpl_id) HDassert(f); HDassert(header); - num_entries = header->list_to_btree; + num_entries = header->list_max; /* Allocate list in memory */ if((list = H5FL_MALLOC(H5SM_list_t)) == NULL) @@ -527,9 +596,8 @@ H5SM_create_list(H5F_t *f, H5SM_index_header_t * header, hid_t dxpl_id) /* Initialize messages in list */ HDmemset(list->messages, 0, sizeof(H5SM_sohm_t) * num_entries); - /* JAMES: would making fewer operations out of this make it faster? */ for(x=0; xmessages[x].hash=H5O_HASH_UNDEF; + list->messages[x].ref_count=0; } list->header = header; @@ -564,6 +632,134 @@ done: /*------------------------------------------------------------------------- + * Function: H5SM_convert_list_to_btree + * + * Purpose: Given a list index, turns it into a B-tree index. This is + * done when too many messages are added to the list. + * + * Requires that *_LIST be a valid list and currently protected + * in the cache. Unprotects (and expunges) *_LIST from the cache. + * + * _LIST needs to be a double pointer so that the calling function + * knows if it is released from the cache if this function exits + * in error. Trying to free it again will trigger an assert. + * + * Return: Non-negative on success + * Negative on failure + * + * Programmer: James Laird + * Thursday, January 4, 2006 + * + *------------------------------------------------------------------------- + */ +herr_t +H5SM_convert_list_to_btree(H5F_t * f, H5SM_index_header_t * header, + H5SM_list_t **_list, hid_t dxpl_id) +{ + H5SM_index_header_t temp_header; + H5SM_list_t *list; + haddr_t tree_addr; + size_t x; + herr_t ret_value = SUCCEED; + + FUNC_ENTER_NOAPI(H5SM_convert_list_to_btree, FAIL) + + HDassert(_list && *_list); + HDassert(header); + + list = *_list; + /* Copy the old index header */ + HDmemcpy(&temp_header, header, sizeof(H5SM_index_header_t)); + + if(H5B2_create(f, dxpl_id, H5SM_INDEX, (size_t)H5SM_B2_NODE_SIZE, + (size_t)H5SM_SOHM_ENTRY_SIZE(f), H5SM_B2_SPLIT_PERCENT, + H5SM_B2_MERGE_PERCENT, &tree_addr) <0) + HGOTO_ERROR(H5E_BTREE, H5E_CANTCREATE, FAIL, "B-tree creation failed for SOHM index") + + /* Insert each record into the new B-tree */ + for(x = 0; x < header->list_max; x++) + { + if(list->messages[x].ref_count > 0) + { + if(H5B2_insert(f, dxpl_id, H5SM_INDEX, tree_addr, &(list->messages[x])) < 0) + HGOTO_ERROR(H5E_BTREE, H5E_CANTINSERT, FAIL, "couldn't add SOHM to B-tree") + HDassert(list->messages[x].ref_count > 0); + } + } + + /* Unprotect list in cache and release heap */ + if(H5AC_unprotect(f, dxpl_id, H5AC_SOHM_LIST, header->index_addr, list, H5AC__DELETED_FLAG) < 0) + HGOTO_ERROR(H5E_CACHE, H5E_CANTUNPROTECT, FAIL, "unable to release SOHM list") + *_list = list = NULL; + + /* Delete the old list index (but not its heap, which the new index is + * still using!) + */ + HDmemcpy(&temp_header, header, sizeof(H5SM_index_header_t)); + if(H5SM_delete_index(f, &temp_header, dxpl_id, FALSE) < 0) + HGOTO_ERROR(H5E_SOHM, H5E_CANTDELETE, FAIL, "can't free list index"); + + header->index_addr = tree_addr; + header->index_type = H5SM_BTREE; + +done: + FUNC_LEAVE_NOAPI(ret_value) +} + +/*------------------------------------------------------------------------- + * Function: H5SM_convert_btree_to_list + * + * Purpose: Given a B-tree index, turns it into a list index. This is + * done when too many messages are deleted from the B-tree. + * + * Return: Non-negative on success + * Negative on failure + * + * Programmer: James Laird + * Thursday, January 4, 2006 + * + *------------------------------------------------------------------------- + */ +static herr_t +H5SM_convert_btree_to_list(H5F_t * f, H5SM_index_header_t * header, hid_t dxpl_id) +{ + H5SM_list_t *list = NULL; + haddr_t btree_addr; + herr_t ret_value = SUCCEED; + + FUNC_ENTER_NOAPI(H5SM_convert_btree_to_list, FAIL) + + /* Remember the address of the old B-tree, but change the header over to be + * a list.. + */ + btree_addr = header->index_addr; + + header->num_messages = 0; + header->index_type = H5SM_LIST; + + /* Create a new list index */ + if(HADDR_UNDEF == (header->index_addr = H5SM_create_list(f, header, dxpl_id))) + HGOTO_ERROR(H5E_SOHM, H5E_CANTINIT, FAIL, "unable to create shared message list") + + if(NULL == (list = (H5SM_list_t *)H5AC_protect(f, dxpl_id, H5AC_SOHM_LIST, header->index_addr, NULL, header, H5AC_WRITE))) + HGOTO_ERROR(H5E_SOHM, H5E_CANTPROTECT, FAIL, "unable to load SOHM list index") + + /* Delete the B-tree and have messages copy themselves to the + * list as they're deleted + */ + if(H5B2_delete(f, dxpl_id, H5SM_INDEX, btree_addr, H5SM_convert_to_list_op, list) < 0) + HGOTO_ERROR(H5E_BTREE, H5E_CANTDELETE, FAIL, "unable to delete B-tree") + +done: + /* Release the SOHM list from the cache */ + if(list && H5AC_unprotect(f, dxpl_id, H5AC_SOHM_LIST, header->index_addr, list, H5AC__DIRTIED_FLAG) < 0) + HDONE_ERROR(H5E_CACHE, H5E_CANTUNPROTECT, FAIL, "unable to unprotect SOHM index") + + FUNC_LEAVE_NOAPI(ret_value) +} + + +/*------------------------------------------------------------------------- * Function: H5SM_try_share * * Purpose: Attempts to share an object header message. If the message @@ -784,46 +980,10 @@ H5SM_write_mesg(H5F_t *f, hid_t dxpl_id, H5SM_index_header_t *header, HGOTO_ERROR(H5E_HEAP, H5E_CANTINSERT, FAIL, "unable to insert message into fractal heap") /* Check whether the list has grown enough that it needs to become a B-tree */ - /* JAMES: make this a separate function */ - if(header->index_type == H5SM_LIST && header->num_messages >= header->list_to_btree) + if(header->index_type == H5SM_LIST && header->num_messages >= header->list_max) { - hsize_t list_size; /* Size of list on disk */ - haddr_t tree_addr; - - if(H5B2_create(f, dxpl_id, H5SM_INDEX, (size_t)H5SM_B2_NODE_SIZE, - (size_t)H5SM_SOHM_ENTRY_SIZE(f), H5SM_B2_SPLIT_PERCENT, - H5SM_B2_MERGE_PERCENT, &tree_addr) <0) - HGOTO_ERROR(H5E_BTREE, H5E_CANTCREATE, FAIL, "B-tree creation failed for SOHM index") - - /* Insert each record into the new B-tree */ - for(x = 0; x < header->list_to_btree; x++) - { - /* JAMES: I'd like to stop relying on H5O_HASH_UNDEF */ - if(list->messages[x].hash != H5O_HASH_UNDEF) - { - if(H5B2_insert(f, dxpl_id, H5SM_INDEX, tree_addr, &(list->messages[x])) < 0) - HGOTO_ERROR(H5E_BTREE, H5E_CANTINSERT, FAIL, "couldn't add SOHM to B-tree") - } - } - - /* Delete the old list */ - HDassert(list); - if(H5AC_unprotect(f, dxpl_id, H5AC_SOHM_LIST, header->index_addr, list, H5AC__DELETED_FLAG) < 0) - HGOTO_ERROR(H5E_CACHE, H5E_CANTUNPROTECT, FAIL, "unable to close SOHM index") - list = NULL; - - /* JAMES: same as list deletion in try_delete? */ - /* Remove the list from the cache */ - if(H5AC_expunge_entry(f, dxpl_id, H5AC_SOHM_LIST, header->index_addr) < 0) - HGOTO_ERROR(H5E_HEAP, H5E_CANTREMOVE, FAIL, "unable to remove list index from cache") - - /* Free the list's space on disk */ - list_size = H5SM_LIST_SIZE(f, header->list_to_btree); - if(H5MF_xfree(f, H5FD_MEM_SOHM_INDEX, dxpl_id, header->index_addr, list_size) < 0) - HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, FAIL, "unable to free shared message list") - - header->index_addr = tree_addr; - header->index_type = H5SM_BTREE; + if(H5SM_convert_list_to_btree(f, header, &list, dxpl_id) < 0) + HGOTO_ERROR(H5E_SOHM, H5E_CANTDELETE, FAIL, "unable to convert list to B-tree") } @@ -832,9 +992,9 @@ H5SM_write_mesg(H5F_t *f, hid_t dxpl_id, H5SM_index_header_t *header, /* Insert the new message into the SOHM index */ if(header->index_type == H5SM_LIST) { - for(x = 0; x < header->list_to_btree; x++) + for(x = 0; x < header->list_max; x++) { - if(list->messages[x].hash == H5O_HASH_UNDEF) /* JAMES: is this a valid test? */ + if(list->messages[x].ref_count == 0) { list->messages[x].fheap_id = shared.u.heap_id; list->messages[x].hash = key.hash; @@ -970,7 +1130,7 @@ done: * * Purpose: Find a message's location in a list * - * Return: Number of messages remaining in the index on success + * Return: Message's position in the list on success * UFAIL if message couldn't be found * * Programmer: James Laird @@ -989,9 +1149,9 @@ H5SM_find_in_list(H5SM_list_t *list, const H5SM_mesg_key_t *key) HDassert(list); HDassert(key); - for(x = 0; x < list->header->list_to_btree; x++) + for(x = 0; x < list->header->list_max; x++) { - if(0 == H5SM_message_compare(key, &(list->messages[x]))) + if((list->messages[x].ref_count > 0 )&& 0 == H5SM_message_compare(key, &(list->messages[x]))) { ret_value = x; break; @@ -1139,9 +1299,7 @@ H5SM_delete_from_index(H5F_t *f, hid_t dxpl_id, H5SM_index_header_t *header, if(header->index_type == H5SM_LIST) { - list->messages[list_pos].hash = H5O_HASH_UNDEF; - list->messages[list_pos].fheap_id = 0; - list->messages[list_pos].ref_count = 0; /* Just in case */ + list->messages[list_pos].ref_count = 0; } else { @@ -1154,79 +1312,31 @@ H5SM_delete_from_index(H5F_t *f, hid_t dxpl_id, H5SM_index_header_t *header, *cache_flags |= H5AC__DIRTIED_FLAG; /* If there are no messages left in the index, delete it - * JAMES: make this a separate function */ if(header->num_messages <=0) { - if(header->index_type == H5SM_LIST) { - hsize_t list_size; /* Size of list on disk */ - - /* Remove the list from the cache */ - HDassert(list); - if(H5AC_unprotect(f, dxpl_id, H5AC_SOHM_LIST, header->index_addr, list, H5AC__DELETED_FLAG) < 0) - HGOTO_ERROR(H5E_CACHE, H5E_CANTUNPROTECT, FAIL, "unable to close SOHM index") - list = NULL; - if(H5AC_expunge_entry(f, dxpl_id, H5AC_SOHM_LIST, header->index_addr) < 0) - HGOTO_ERROR(H5E_HEAP, H5E_CANTREMOVE, FAIL, "unable to remove list index from cache") - - /* Free the file space used */ - list_size = H5SM_LIST_SIZE(f, header->list_to_btree); - if(H5MF_xfree(f, H5FD_MEM_SOHM_INDEX, dxpl_id, header->index_addr, list_size) < 0) - HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, FAIL, "unable to free shared message list") - - } else { - HDassert(header->index_type == H5SM_BTREE); - - if(H5B2_delete(f, dxpl_id, H5SM_INDEX, header->index_addr, NULL, NULL) < 0) - HGOTO_ERROR(H5E_BTREE, H5E_CANTDELETE, FAIL, "unable to delete B-tree") - } + /* Unprotect cache and release heap */ + if(list && H5AC_unprotect(f, dxpl_id, H5AC_SOHM_LIST, header->index_addr, list, H5AC__DELETED_FLAG) < 0) + HGOTO_ERROR(H5E_CACHE, H5E_CANTUNPROTECT, FAIL, "unable to release SOHM list") + list = NULL; - /* Free the fractal heap */ - /* Release the fractal heap if we opened it */ HDassert(fheap); if(H5HF_close(fheap, dxpl_id) < 0) - HDONE_ERROR(H5E_HEAP, H5E_CLOSEERROR, FAIL, "can't close fractal heap") + HGOTO_ERROR(H5E_HEAP, H5E_CLOSEERROR, FAIL, "can't close fractal heap") fheap = NULL; - if(H5HF_delete(f, dxpl_id, header->heap_addr) < 0) - HGOTO_ERROR(H5E_SOHM, H5E_CANTDELETE, FAIL, "unable to delete fractal heap") - header->index_addr = HADDR_UNDEF; - header->heap_addr = HADDR_UNDEF; + /* Delete the index and its heap */ + if(H5SM_delete_index(f, header, dxpl_id, TRUE) < 0) + HGOTO_ERROR(H5E_SOHM, H5E_CANTDELETE, FAIL, "can't delete empty index") - } else if(header->index_type == H5SM_BTREE && header->num_messages < header->btree_to_list) + } else if(header->index_type == H5SM_BTREE && header->num_messages < header->btree_min) /* JAMES: there's an off-by-one error here? */ - /* JAMES: make this a separate function */ { /* Otherwise, if we've just passed the btree-to-list cutoff, convert * this B-tree into a list */ - /* Remember the btree address for this index; we'll overwrite the - * address in the index header - */ - H5SM_index_header_t temp_header; - - /* The protect callback expects a header corresponding to the list - * index. Create a "temporary" header to hold the old B-tree - * index and reset values in the "real" header to point to an - * empty list index. - */ - HDmemcpy(&temp_header, header, sizeof(H5SM_index_header_t)); - header->num_messages = 0; - header->index_type = H5SM_LIST; - - /* Create a new list index */ - if(HADDR_UNDEF == (header->index_addr = H5SM_create_list(f, header, dxpl_id))) - HGOTO_ERROR(H5E_SOHM, H5E_CANTINIT, FAIL, "unable to create shared message list") - - HDassert(NULL == list); - if(NULL == (list = (H5SM_list_t *)H5AC_protect(f, dxpl_id, H5AC_SOHM_LIST, header->index_addr, NULL, header, H5AC_WRITE))) - HGOTO_ERROR(H5E_SOHM, H5E_CANTPROTECT, FAIL, "unable to load SOHM index") - - /* Delete the B-tree and have messages copy themselves to the - * list as they're deleted - */ - if(H5B2_delete(f, dxpl_id, H5SM_INDEX, temp_header.index_addr, H5SM_convert_to_list_op, list) < 0) - HGOTO_ERROR(H5E_BTREE, H5E_CANTDELETE, FAIL, "unable to delete B-tree") + if(H5SM_convert_btree_to_list(f, header, dxpl_id) < 0) + HGOTO_ERROR(H5E_SOHM, H5E_CANTINIT, FAIL, "unable to convert btree to list") } /* end if */ } /* end if */ @@ -1261,7 +1371,7 @@ done: */ herr_t H5SM_get_info(H5F_t *f, unsigned *index_flags, unsigned *minsizes, - unsigned *list_to_btree, unsigned *btree_to_list, hid_t dxpl_id) + unsigned *list_max, unsigned *btree_min, hid_t dxpl_id) { H5SM_master_table_t *table = NULL; haddr_t table_addr; @@ -1282,8 +1392,8 @@ H5SM_get_info(H5F_t *f, unsigned *index_flags, unsigned *minsizes, HGOTO_ERROR(H5E_CACHE, H5E_CANTPROTECT, FAIL, "unable to load SOHM master table") /* Return info */ - *list_to_btree = table->indexes[0].list_to_btree; - *btree_to_list = table->indexes[0].btree_to_list; + *list_max = table->indexes[0].list_max; + *btree_min = table->indexes[0].btree_min; /* Get information about the individual SOHM indexes */ for(i=0; inum_indexes; ++i) { diff --git a/src/H5SMbtree2.c b/src/H5SMbtree2.c index adfcaba..1b929e7 100755 --- a/src/H5SMbtree2.c +++ b/src/H5SMbtree2.c @@ -93,6 +93,12 @@ H5SM_message_compare(const void *rec1, const void *rec2) FUNC_ENTER_NOAPI_NOINIT_NOFUNC(H5SM_message_compare) + /* JAMES: might be able to spare a sentinel byte instead of worrying about + * refcounts. Here, we need to find a deleted message in a B-tree to + * actually delete it. + */ + /* JAMES HDassert(mesg->ref_count > 0); */ + hash_diff = key->hash; hash_diff -= mesg->hash; @@ -383,11 +389,12 @@ H5SM_convert_to_list_op(const void * record, void *op_data) HDassert(op_data); /* Insert this message into the list */ - for(x=0; xheader->list_to_btree; x++) + for(x=0; xheader->list_max; x++) { - if(list->messages[x].hash == H5O_HASH_UNDEF) /* JAMES: is this a valid test? */ + if(list->messages[x].ref_count == 0) { HDmemcpy(&(list->messages[x]), message, sizeof(H5SM_sohm_t)); + HDassert(list->messages[x].ref_count > 0); break; } } diff --git a/src/H5SMcache.c b/src/H5SMcache.c index bd1eb2b..d4323b0 100644 --- a/src/H5SMcache.c +++ b/src/H5SMcache.c @@ -151,8 +151,8 @@ H5SM_flush_table(H5F_t *f, hid_t dxpl_id, hbool_t destroy, haddr_t addr, H5SM_ma UINT16ENCODE(p, table->indexes[x].mesg_types); /* Type of messages in the index */ UINT32ENCODE(p, table->indexes[x].min_mesg_size); /* Minimum size of message to share */ - UINT16ENCODE(p, table->indexes[x].list_to_btree); /* List cutoff; fewer than this number and index becomes a list */ - UINT16ENCODE(p, table->indexes[x].btree_to_list); /* B-tree cutoff; more than this number and index becomes a B-tree */ + UINT16ENCODE(p, table->indexes[x].list_max); /* List cutoff; fewer than this number and index becomes a list */ + UINT16ENCODE(p, table->indexes[x].btree_min); /* B-tree cutoff; more than this number and index becomes a B-tree */ UINT16ENCODE(p, table->indexes[x].num_messages); /* Number of messages shared */ H5F_addr_encode(f, &p, table->indexes[x].index_addr); /* Address of the actual index */ H5F_addr_encode(f, &p, table->indexes[x].heap_addr); /* Address of the index's heap */ @@ -258,8 +258,8 @@ H5SM_load_table(H5F_t *f, hid_t dxpl_id, haddr_t addr, const void UNUSED *udata1 UINT16DECODE(p, table->indexes[x].mesg_types); UINT32DECODE(p, table->indexes[x].min_mesg_size); - UINT16DECODE(p, table->indexes[x].list_to_btree); - UINT16DECODE(p, table->indexes[x].btree_to_list); + UINT16DECODE(p, table->indexes[x].list_max); + UINT16DECODE(p, table->indexes[x].btree_min); UINT16DECODE(p, table->indexes[x].num_messages); H5F_addr_decode(f, &p, &(table->indexes[x].index_addr)); H5F_addr_decode(f, &p, &(table->indexes[x].heap_addr)); @@ -435,8 +435,8 @@ H5SM_flush_list(H5F_t *f, hid_t dxpl_id, hbool_t destroy, haddr_t addr, H5SM_lis /* Write messages from the messages array to disk */ /* JAMES: we have to search the whole array. not the best way to do it; could go until we've written * num_messages */ - for(x=0; xheader->list_to_btree; x++) { - if(list->messages[x].fheap_id != 0 && list->messages[x].hash != H5O_HASH_UNDEF) { + for(x=0; xheader->list_max; x++) { + if(list->messages[x].ref_count > 0) { /* JAMES: use H5SM_message_encode here */ UINT32ENCODE(p, list->messages[x].hash); /* Read the hash value for this message */ UINT32ENCODE(p, list->messages[x].ref_count); /* Read the reference count for this message */ @@ -501,7 +501,7 @@ H5SM_load_list(H5F_t *f, hid_t dxpl_id, haddr_t addr, const void UNUSED *udata1, HDmemset(&list->cache_info, 0, sizeof(H5AC_info_t)); /* Allocate list in memory as an array*/ - if((list->messages = H5FL_ARR_MALLOC(H5SM_sohm_t, header->list_to_btree)) == NULL) + if((list->messages = H5FL_ARR_MALLOC(H5SM_sohm_t, header->list_max)) == NULL) HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "file allocation failed for SOHM list") list->header = header; @@ -552,11 +552,9 @@ H5SM_load_list(H5F_t *f, hid_t dxpl_id, haddr_t addr, const void UNUSED *udata1, /* Initialize the rest of the array */ - for(x=header->num_messages; xlist_to_btree; x++) + for(x=header->num_messages; xlist_max; x++) { - list->messages[x].fheap_id = 0; /* JAMES: would making this one operation make it faster? */ list->messages[x].ref_count = 0; - list->messages[x].hash = H5O_HASH_UNDEF; } ret_value = list; @@ -666,7 +664,7 @@ H5SM_list_size(const H5F_t UNUSED *f, const H5SM_list_t *list, size_t *size_ptr) HDassert(size_ptr); /* Set size value */ - *size_ptr = H5SM_LIST_SIZE(f, list->header->list_to_btree); /* JAMES: might want to have variable-sized lists */ + *size_ptr = H5SM_LIST_SIZE(f, list->header->list_max); /* JAMES: might want to have variable-sized lists */ FUNC_LEAVE_NOAPI(SUCCEED) } /* end H5SM_list_size */ diff --git a/src/H5SMpkg.h b/src/H5SMpkg.h index 643b1fd..760f6a2 100755 --- a/src/H5SMpkg.h +++ b/src/H5SMpkg.h @@ -40,8 +40,8 @@ #define H5SM_MASTER_TABLE_VERSION 0 /* Version of the Shared Object Header Message Master Table*/ #define H5SM_SOHM_ENTRY_SIZE(f) (4 /* Hash value */ \ - + 4 /* reference count*/ \ - + 8) /* JAMES: size of heap ID on disk */ + + 4 /* reference count*/ \ + + 8) /* JAMES: size of heap ID on disk */ #define H5SM_TABLE_SIZE(f) ( H5SM_TABLE_SIZEOF_MAGIC \ + 1 /* Table version */ \ @@ -54,7 +54,6 @@ + H5F_SIZEOF_ADDR(f) /* Location of list or B-tree */ \ + H5F_SIZEOF_ADDR(f)) /* Address of heap */ -/* JAMES: add checksum? */ #define H5SM_LIST_SIZE(f, num_mesg) H5SM_LIST_SIZEOF_MAGIC \ + 1 /* List version */ \ + (H5SM_SOHM_ENTRY_SIZE(f) * num_mesg) \ @@ -98,10 +97,9 @@ /* Typedef for a SOHM index node */ typedef struct { - /* JAMES: I think I need message type here, and stored in file. */ uint32_t hash; /* Hash value for OHM */ H5SM_fheap_id_t fheap_id; /* ID of the OHM in the fractal heap */ - hsize_t ref_count; /* JAMES TODO: should this be hsize_t? */ + hsize_t ref_count; /* Number of times this message is used */ } H5SM_sohm_t; typedef enum { @@ -123,8 +121,8 @@ typedef struct { typedef struct { unsigned mesg_types; /* Bit flag vector of message types */ size_t min_mesg_size; /* number of messages being tracked */ - size_t list_to_btree; /* >= this many messages, index with a B-tree */ - size_t btree_to_list; /* <= this many messages, index with a list again */ + size_t list_max; /* >= this many messages, index with a B-tree */ + size_t btree_min; /* <= this many messages, index with a list again */ size_t num_messages; /* number of messages being tracked */ H5SM_index_type_t index_type; /* Is the index a list or a B-tree? */ haddr_t index_addr; /* Address of the actual index (list or B-tree) */ diff --git a/src/H5SMprivate.h b/src/H5SMprivate.h index 9cfad58..1a45fcc 100755 --- a/src/H5SMprivate.h +++ b/src/H5SMprivate.h @@ -37,7 +37,7 @@ H5_DLL htri_t H5SM_try_share(H5F_t *f, hid_t dxpl_id, unsigned type_id, void *mesg); H5_DLL herr_t H5SM_try_delete(H5F_t *f, hid_t dxpl_id, unsigned type_id, const H5O_shared_t *mesg); H5_DLL herr_t H5SM_get_info(H5F_t *f, unsigned *index_flags, unsigned *minsizes, - unsigned *list_to_btree, unsigned *btree_to_list, hid_t dxpl_id); + unsigned *list_max, unsigned *btree_min, hid_t dxpl_id); H5_DLL htri_t H5SM_type_shared(H5F_t *f, unsigned type_id, hid_t dxpl_id); H5_DLL haddr_t H5SM_get_fheap_addr(H5F_t *f, unsigned type_id, hid_t dxpl_id); H5_DLL herr_t H5SM_reconstitute(H5O_shared_t *sh_mesg, const uint8_t *heap_id); diff --git a/test/tattr.c b/test/tattr.c index c5f0dcc..92f0c56 100644 --- a/test/tattr.c +++ b/test/tattr.c @@ -20,9 +20,6 @@ * *************************************************************/ -/* JAMES: try writing a second value to an existing shared attribute. - * Does modifying attributes work? */ - #include "testhdf5.h" #include "h5test.h" #include "hdf5.h" diff --git a/test/tfile.c b/test/tfile.c index 1269ec3..834e261 100644 --- a/test/tfile.c +++ b/test/tfile.c @@ -1245,13 +1245,11 @@ test_file_freespace(void) /* Check that there is the right amount of free space in the file */ free_space = H5Fget_freespace(file); CHECK(free_space, FAIL, "H5Fget_freespace"); -#ifdef JAMES #ifdef H5_HAVE_LARGE_HSIZET VERIFY(free_space, 2368, "H5Fget_freespace"); #else /* H5_HAVE_LARGE_HSIZET */ VERIFY(free_space, 588, "H5Fget_freespace"); /* XXX: fix me */ #endif /* H5_HAVE_LARGE_HSIZET */ -#endif /* JAMES */ /* Delete datasets in file */ for(u = 0; u < 10; u++) { sprintf(name, "Dataset %u", u); @@ -1262,13 +1260,11 @@ test_file_freespace(void) /* Check that there is the right amount of free space in the file */ free_space = H5Fget_freespace(file); CHECK(free_space, FAIL, "H5Fget_freespace"); -#ifdef JAMES #ifdef H5_HAVE_LARGE_HSIZET VERIFY(free_space, 5512, "H5Fget_freespace"); #else /* H5_HAVE_LARGE_HSIZET */ VERIFY(free_space, 4592, "H5Fget_freespace"); /* XXX: fix me */ #endif /* H5_HAVE_LARGE_HSIZET */ -#endif /* JAMES */ /* Close file */ ret = H5Fclose(file); diff --git a/test/th5s.c b/test/th5s.c index 0c97a3f..359f023 100644 --- a/test/th5s.c +++ b/test/th5s.c @@ -954,10 +954,6 @@ test_h5s_compound_scalar_read(void) ret = H5Sclose(sid1); CHECK(ret, FAIL, "H5Sclose"); - /* Close datatype JAMES - ret = H5Tclose(type); - CHECK(ret, FAIL, "H5Tclose"); -*/ /* Close file */ ret = H5Fclose(fid1); CHECK(ret, FAIL, "H5Fclose"); diff --git a/test/tmisc.c b/test/tmisc.c index 0f71f8d..a4d54ae 100644 --- a/test/tmisc.c +++ b/test/tmisc.c @@ -1787,7 +1787,6 @@ test_misc11(void) unsigned stab; /* Symbol table entry version # */ unsigned shhdr; /* Shared object header version # */ unsigned nindexes; /* Shared message number of indexes */ -/* JAMES: add more SOHM properties here */ herr_t ret; /* Generic return value */ /* Output message about test being performed */ diff --git a/test/tsohm.c b/test/tsohm.c index a6e38ad..3bdb340 100644 --- a/test/tsohm.c +++ b/test/tsohm.c @@ -30,7 +30,6 @@ const unsigned def_minsizes[H5SM_MAX_NINDEXES] = {250,250,250,250,250,250}; #define DEF_B2L 40 /* Non-default SOHM values for testing */ -/* JAMES: make these defined in function */ #define TEST_NUM_INDEXES 4 const unsigned test_type_flags[H5SM_MAX_NINDEXES] = {H5O_MESG_FILL_FLAG, @@ -909,7 +908,8 @@ static void test_sohm_size1(void) /* Both sohm files should be bigger than a normal file when empty. * It's hard to say whether a B-tree with no nodes allocated should be * smaller than a list with SOHM_HELPER_NUM_DTYPES elements. - * JAMES: The sizes here shouldn't really be 1 + * The sizes here shouldn't really be 1; it's just used to ensure that the + * error code triggers. */ if(sohm_empty_filesize <= norm_empty_filesize) VERIFY(sohm_empty_filesize, 1, "H5Fclose"); @@ -1330,7 +1330,6 @@ size2_helper(hid_t fcpl_id, int test_file_closing) char fill2[DTYPE2_SIZE]; /* Create a file and get its size */ - /* JAMES: is fixname needed at all? */ file_id = H5Fcreate(FILENAME, H5F_ACC_TRUNC, fcpl_id, H5P_DEFAULT); CHECK_I(file_id, "H5Fcreate"); @@ -3068,6 +3067,7 @@ test_sohm(void) test_sohm_delete_revert(); /* Test that a file with SOHMs becomes an * empty file again when they are deleted. */ test_sohm_extlink(); /* Test SOHMs when external links are used */ + /* JAMES: try extending dataspaces, overwriting attributes */ } /* test_sohm() */ -- cgit v0.12