From b1c318eebe05605f5086fb79cfd7e88d4b949bcf Mon Sep 17 00:00:00 2001 From: James Laird Date: Fri, 22 Dec 2006 13:23:08 -0500 Subject: [svn-r13086] Added deletion test for shared messages. I'm not sure that this test is as complete as it could be, so I may add to it later. Fixed a bug in reference counting messages that are referenced by shared messages (attribute datatypes and dataspaces). Tested on mir, smirom, and Windows. --- src/H5Oattribute.c | 18 +++ src/H5Omessage.c | 40 +++++++ src/H5Oprivate.h | 1 + src/H5Oshared.c | 4 - src/H5SM.c | 70 ++++++++++-- test/tsohm.c | 324 ++++++++++++++++++++++++++++++++++++++++++++++++++++- 6 files changed, 444 insertions(+), 13 deletions(-) diff --git a/src/H5Oattribute.c b/src/H5Oattribute.c index a7aeb79..ce7a8e5 100644 --- a/src/H5Oattribute.c +++ b/src/H5Oattribute.c @@ -562,6 +562,24 @@ H5O_attr_write_cb(H5O_t UNUSED *oh, H5O_mesg_t *mesg/*in,out*/, if(HDstrcmp(shared_attr.name, udata->attr->name) == 0) { htri_t shared_mesg; /* Whether the message should be shared */ + /* Re-share attribute's datatype and dataspace to increment their + * reference count if they're shared. + * Otherwise they may be deleted when the old attribute + * message is deleted. + */ + if(H5SM_try_share(udata->f, udata->dxpl_id, H5O_DTYPE_ID, udata->attr->dt) < 0) + HGOTO_ERROR(H5E_SOHM, H5E_BADMESG, H5_ITER_ERROR, "error trying to re-share attribute datatype") + if(H5SM_try_share(udata->f, udata->dxpl_id, H5O_SDSPACE_ID, udata->attr->ds) < 0) + HGOTO_ERROR(H5E_SOHM, H5E_BADMESG, H5_ITER_ERROR, "error trying to re-share attribute datatype") + + /* Likewise, increment reference count if this attribute has a named datatype */ + /* JAMES: this is identical to the attr_link callback. */ + if(H5T_committed(udata->attr->dt)) { + /* Increment the reference count on the shared datatype */ + if(H5T_link(udata->attr->dt,1,udata->dxpl_id) < 0) + HGOTO_ERROR(H5E_OHDR, H5E_LINKCOUNT, FAIL, "unable to adjust shared datatype link count") + } /* end if */ + /* Store new version of message as a SOHM */ /* (should always work, since we're not changing the size of the attribute) */ if((shared_mesg = H5SM_try_share(udata->f, udata->dxpl_id, H5O_ATTR_ID, udata->attr)) == 0) diff --git a/src/H5Omessage.c b/src/H5Omessage.c index 3648d49..bedfcb2 100644 --- a/src/H5Omessage.c +++ b/src/H5Omessage.c @@ -2000,6 +2000,46 @@ done: /*------------------------------------------------------------------------- + * Function: H5O_msg_delete + * + * Purpose: Calls a message's delete callback. + * + * JAMES: this is mostly redundant with H5O_delete_mesg below, + * but H5O_delete_mesg only works on messages in object headers + * (i.e., not shared messages). + * + * Return: Success: Non-negative + * Failure: Negative + * + * Programmer: James Laird + * December 21, 2006 + * + *------------------------------------------------------------------------- + */ +herr_t +H5O_msg_delete(H5F_t *f, hid_t dxpl_id, unsigned type_id, const void *mesg) +{ + const H5O_msg_class_t *type; /* Actual H5O class type for the ID */ + herr_t ret_value = SUCCEED; /* Return value */ + + FUNC_ENTER_NOAPI(H5O_msg_delete, NULL) + + /* check args */ + HDassert(f); + HDassert(type_id < NELMTS(H5O_msg_class_g)); + type = H5O_msg_class_g[type_id]; /* map the type ID to the actual type object */ + HDassert(type); + + /* delete */ + if((type->del) && (type->del)(f, dxpl_id, mesg, 1) < 0) + HGOTO_ERROR(H5E_OHDR, H5E_CANTDELETE, FAIL, "unable to delete file space for object header message") + +done: + FUNC_LEAVE_NOAPI(ret_value) +} /* end H5O_msg_decode() */ + + +/*------------------------------------------------------------------------- * Function: H5O_delete_mesg * * Purpose: Internal function to: diff --git a/src/H5Oprivate.h b/src/H5Oprivate.h index 27e2bdf..615d646 100644 --- a/src/H5Oprivate.h +++ b/src/H5Oprivate.h @@ -432,6 +432,7 @@ H5_DLL herr_t H5O_msg_reset_share(unsigned type_id, void *mesg); H5_DLL herr_t H5O_msg_encode(H5F_t *f, unsigned type_id, unsigned char *buf, const void *obj); H5_DLL void* H5O_msg_decode(H5F_t *f, hid_t dxpl_id, unsigned type_id, const unsigned char *buf); +H5_DLL herr_t H5O_msg_delete(H5F_t *f, hid_t dxpl_id, unsigned type_id, const void *mesg); /* Object copying routines */ H5_DLL herr_t H5O_copy_header_map(const H5O_loc_t *oloc_src, H5O_loc_t *oloc_dst /*out */, diff --git a/src/H5Oshared.c b/src/H5Oshared.c index d1f6577..579e08f 100644 --- a/src/H5Oshared.c +++ b/src/H5Oshared.c @@ -531,10 +531,6 @@ H5O_shared_delete(H5F_t *f, hid_t dxpl_id, const void *_mesg, if(H5O_shared_link_adj(f, dxpl_id, shared, -1) < 0) HGOTO_ERROR(H5E_OHDR, H5E_LINKCOUNT, FAIL, "unable to adjust shared object link count") - /* JAMES */ -/* JAMES if((shared->flags & H5O_SHARED_IN_HEAP_FLAG) > 0) - H5O_loc_free(&(shared->oloc)); -*/ done: FUNC_LEAVE_NOAPI(ret_value) } /* end H5O_shared_delete() */ diff --git a/src/H5SM.c b/src/H5SM.c index 348e4b6..7b0d6ec 100755 --- a/src/H5SM.c +++ b/src/H5SM.c @@ -19,7 +19,6 @@ #define H5SM_PACKAGE /*suppress error about including H5SMpkg */ #define H5F_PACKAGE /*suppress error about including H5Fpkg */ - /***********/ /* Headers */ /***********/ @@ -32,7 +31,6 @@ #include "H5MMprivate.h" /* Memory management */ #include "H5SMpkg.h" /* Shared object header messages */ - /****************/ /* Local Macros */ /****************/ @@ -60,7 +58,7 @@ static herr_t H5SM_write_mesg(H5F_t *f, hid_t dxpl_id, H5SM_index_header_t *head unsigned type_id, void *mesg, unsigned *cache_flags_ptr); static herr_t H5SM_delete_from_index(H5F_t *f, hid_t dxpl_id, H5SM_index_header_t *header, unsigned type_id, const H5O_shared_t * mesg, - unsigned *cache_flags); + unsigned *cache_flags, void ** buf); static herr_t H5SM_type_to_flag(unsigned type_id, unsigned *type_flag); @@ -896,6 +894,8 @@ H5SM_try_delete(H5F_t *f, hid_t dxpl_id, unsigned type_id, H5SM_master_table_t *table = NULL; unsigned cache_flags = H5AC__NO_FLAGS_SET; ssize_t index_num; + void *mesg_buf = NULL; + void *native_mesg = NULL; herr_t ret_value = SUCCEED; FUNC_ENTER_NOAPI(H5SM_try_delete, FAIL) @@ -918,14 +918,43 @@ H5SM_try_delete(H5F_t *f, hid_t dxpl_id, unsigned type_id, HGOTO_ERROR(H5E_SOHM, H5E_NOTFOUND, FAIL, "unable to find correct SOHM index") /* JAMES: this triggers some warning on heping. "overflow in implicit constant conversion" */ - if(H5SM_delete_from_index(f, dxpl_id, &(table->indexes[index_num]), type_id, sh_mesg, &cache_flags) < 0) + /* If mesg_buf is not NULL, the message's reference count has reached + * zero and any file space it uses needs to be freed. mesg_buf holds the + * serialized form of the message. + */ + if(H5SM_delete_from_index(f, dxpl_id, &(table->indexes[index_num]), type_id, sh_mesg, &cache_flags, &mesg_buf) < 0) HGOTO_ERROR(H5E_SOHM, H5E_CANTDELETE, FAIL, "unable to delete mesage from SOHM index") -done: /* Release the master SOHM table */ + if(H5AC_unprotect(f, dxpl_id, H5AC_SOHM_TABLE, f->shared->sohm_addr, table, cache_flags) < 0) + HGOTO_ERROR(H5E_CACHE, H5E_CANTRELEASE, FAIL, "unable to close SOHM master table") + + table = NULL; + + /* If buf was allocated, delete the message it holds. This message may + * reference other shared messages that also need to be deleted, so the + * master table needs to be unprotected when we do this. + */ + if(mesg_buf) { + if(NULL == (native_mesg = H5O_msg_decode(f, dxpl_id, type_id, mesg_buf))) + HGOTO_ERROR(H5E_OHDR, H5E_CANTDECODE, FAIL, "can't decode shared message.") + + if(H5O_msg_delete(f, dxpl_id, type_id, native_mesg) < 0) + HGOTO_ERROR(H5E_OHDR, H5E_CANTFREE, FAIL, "can't delete shared message.") + } + +done: + /* Release the master SOHM table on error */ if(table && H5AC_unprotect(f, dxpl_id, H5AC_SOHM_TABLE, f->shared->sohm_addr, table, cache_flags) < 0) HDONE_ERROR(H5E_CACHE, H5E_CANTRELEASE, FAIL, "unable to close SOHM master table") + if(native_mesg) + H5O_msg_free(type_id, native_mesg); + + /* Free buf */ + if(mesg_buf) + H5MM_xfree(mesg_buf); + FUNC_LEAVE_NOAPI(ret_value) } /* end H5SM_try_delete() */ @@ -998,8 +1027,11 @@ H5SM_get_hash_fh_cb(const void *obj, size_t obj_len, void *_udata) /*------------------------------------------------------------------------- * Function: H5SM_delete_from_index * - * Purpose: Given a SOHM message, delete it from this index. - * JAMES: is the message necessarily in the index? Also, name this "dec ref count" or something. + * Purpose: Decrement the reference count for a particular message in this + * index. If the reference count reaches zero, allocate a buffer + * to hold the serialized form of this message so that any + * resources it uses can be freed. + * JAMES: rename buf * * Return: Non-negative on success * Negative on failure @@ -1011,7 +1043,7 @@ H5SM_get_hash_fh_cb(const void *obj, size_t obj_len, void *_udata) */ static herr_t H5SM_delete_from_index(H5F_t *f, hid_t dxpl_id, H5SM_index_header_t *header, - unsigned type_id, const H5O_shared_t * mesg, unsigned *cache_flags) + unsigned type_id, const H5O_shared_t * mesg, unsigned *cache_flags, void ** buf) { H5SM_list_t *list = NULL; H5SM_mesg_key_t key; @@ -1027,6 +1059,7 @@ H5SM_delete_from_index(H5F_t *f, hid_t dxpl_id, H5SM_index_header_t *header, HDassert(mesg); HDassert(cache_flags); HDassert(mesg->flags & H5O_SHARED_IN_HEAP_FLAG); + HDassert(*buf == NULL); /* Open the heap that this message is in */ if(NULL == (fheap = H5HF_open(f, dxpl_id, header->heap_addr))) @@ -1076,6 +1109,24 @@ H5SM_delete_from_index(H5F_t *f, hid_t dxpl_id, H5SM_index_header_t *header, /* If the ref count is zero, delete the message from the index */ if(message.ref_count <= 0) { + size_t buf_size; + + /* Close the message being deleted, since it may need to adjust + * other reference counts (e.g., an attribute needs to free its + * shared datatype). + */ + /* Get the size of the message in the heap */ + if(H5HF_get_obj_len(fheap, dxpl_id, &(message.fheap_id), &buf_size) < 0) + HGOTO_ERROR(H5E_OHDR, H5E_CANTGET, FAIL, "can't get message size from fractal heap.") + + /* Allocate a buffer to hold the message */ + if(NULL == (*buf = H5MM_malloc(buf_size))) + HGOTO_ERROR(H5E_OHDR, H5E_NOSPACE, FAIL, "memory allocation failed") + + /* Retrieve the message from the heap */ + if(H5HF_read(fheap, dxpl_id, &(message.fheap_id), *buf) < 0) + HGOTO_ERROR(H5E_OHDR, H5E_CANTLOAD, FAIL, "can't read message from fractal heap.") + /* Remove the message from the heap */ if(H5HF_remove(fheap, dxpl_id, &(message.fheap_id)) < 0) HGOTO_ERROR(H5E_SOHM, H5E_CANTREMOVE, FAIL, "unable to remove message from heap") @@ -1142,6 +1193,9 @@ done: if(fheap && H5HF_close(fheap, dxpl_id) < 0) HDONE_ERROR(H5E_HEAP, H5E_CLOSEERROR, FAIL, "can't close fractal heap") + /* Free the serialized message buffer on error */ + if(ret_value < 0 && *buf) + H5MM_xfree(*buf); FUNC_LEAVE_NOAPI(ret_value) } /* end H5SM_delete_from_index() */ diff --git a/test/tsohm.c b/test/tsohm.c index 9274ed4..6585f0e 100644 --- a/test/tsohm.c +++ b/test/tsohm.c @@ -142,6 +142,11 @@ typedef struct size2_helper_struct { h5_stat_size_t attrs2; } size2_helper_struct; +/* Number of distinct messages for the sohm_delete test */ +#define DELETE_NUM_MESGS 7 +#define HALF_DELETE_NUM_MESGS 3 +#define DELETE_DIMS {1,1,1,1,1,1,1} + /**************************************************************** ** ** check_fcpl_values(): Helper function for test_sohm_fcpl. @@ -2429,6 +2434,320 @@ static void test_sohm_size2(int close_reopen) +/*------------------------------------------------------------------------- + * Function: delete_helper_write + * + * Purpose: Creates a dataset and attribute in file FILE_ID using value X + * in the DSPACE_ID and DCPL_ID arrays. + * + * Programmer: James Laird + * Tuesday, December 19, 2006 + * + * Modifications: + * + *------------------------------------------------------------------------- + */ +static void delete_helper_write(hid_t file_id, hid_t *dspace_id, hid_t *dcpl_id, int x) +{ + hid_t dset_id = -1; + hid_t attr_id = -1; + char wdata; + herr_t ret; + + /* Create dataset */ + dset_id = H5Dcreate(file_id, DSETNAME[x], H5T_NATIVE_CHAR, dspace_id[x], dcpl_id[x]); + CHECK_I(dset_id, "H5Dcreate"); + + /* Write data to dataset */ + wdata = x + 'a'; + ret = H5Dwrite(dset_id, H5T_NATIVE_CHAR, dspace_id[x], dspace_id[x], H5P_DEFAULT, &wdata); + CHECK_I(ret, "H5Dwrite"); + + /* Create an attribute on the dataset. */ + attr_id = H5Acreate(dset_id, "attr_name", H5T_NATIVE_CHAR, dspace_id[x], H5P_DEFAULT); + CHECK_I(attr_id, "H5Acreate"); + + /* Write to attribute */ + ret = H5Awrite(attr_id, H5T_NATIVE_CHAR, &wdata); + CHECK_I(ret, "H5Awrite"); + + ret = H5Aclose(attr_id); + CHECK_I(ret, "H5Aclose"); + ret = H5Dclose(dset_id); + CHECK_I(ret, "H5Dclose"); +} + +/*------------------------------------------------------------------------- + * Function: delete_helper_read + * + * Purpose: Checks the value of the dataset and attribute created by + * delete_helper_write. + * + * Programmer: James Laird + * Tuesday, December 19, 2006 + * + * Modifications: + * + *------------------------------------------------------------------------- + */ +static void delete_helper_read(hid_t file_id, hid_t *dspace_id, int x) +{ + hid_t dset_id = -1; + hid_t attr_id = -1; + char rdata; + herr_t ret; + + /* Open dataset */ + dset_id = H5Dopen(file_id, DSETNAME[x]); + CHECK_I(dset_id, "H5Dcreate"); + + /* Read */ + rdata = '\0'; + ret = H5Dread(dset_id, H5T_NATIVE_CHAR, dspace_id[x], dspace_id[x], H5P_DEFAULT, &rdata); + CHECK_I(ret, "H5Dread"); + VERIFY(rdata, (x + 'a'), "H5Dread"); + + /* Open attribute */ + attr_id = H5Aopen_name(dset_id, "attr_name"); + CHECK_I(attr_id, "H5Aopen"); + + /* Read */ + rdata = '\0'; + ret = H5Aread(attr_id, H5T_NATIVE_CHAR, &rdata); + CHECK_I(ret, "H5Dread"); + VERIFY(rdata, (x + 'a'), "H5Dread"); + + /* Cleanup */ + ret = H5Aclose(attr_id); + CHECK_I(ret, "H5Aclose"); + ret = H5Dclose(dset_id); + CHECK_I(ret, "H5Dclose"); +} + + +/*------------------------------------------------------------------------- + * Function: delete_helper + * + * Purpose: Creates some shared messages, deletes them, and creates some + * more messages. The second batch of messages should use the + * space freed by the first batch, so should be about the same + * size as a file that never had the first batch of messages + * created. + * + * FCPL_ID is the file creation property list to use. + * DSPACE_ID and DCPL_ID are arrays of different dataspaces + * and property lists with filter pipelines used to create the + * messages. + * + * Programmer: James Laird + * Tuesday, December 19, 2006 + * + * Modifications: + * + *------------------------------------------------------------------------- + */ +static void delete_helper(hid_t fcpl_id, hid_t *dspace_id, hid_t *dcpl_id) +{ + hid_t file_id=-1; + int x; + h5_stat_size_t norm_filesize; + h5_stat_size_t deleted_filesize; + herr_t ret; + + /* Get the size of a "normal" file with no deleted messages */ + file_id = H5Fcreate(FILENAME, H5F_ACC_TRUNC, fcpl_id, H5P_DEFAULT); + CHECK_I(file_id, "H5Fcreate"); + + /* Create batch of messages in the file starting at message 2 */ + for(x=HALF_DELETE_NUM_MESGS; x deleted_filesize * OVERHEAD_ALLOWED) + VERIFY(0, 1, "h5_get_file_size"); + if(deleted_filesize > norm_filesize * OVERHEAD_ALLOWED) + VERIFY(0, 1, "h5_get_file_size"); + +} + + + +/*------------------------------------------------------------------------- + * Function: test_delete + * + * Purpose: Tests shared object header message deletion. + * + * Creates lots of shared messages, then ensures that they + * can be deleted without corrupting the remaining messages. + * Also checks that indexes convert from B-trees back into + * lists. + * + * Programmer: James Laird + * Tuesday, December 19, 2006 + * + * Modifications: + * + *------------------------------------------------------------------------- + */ +static void test_sohm_delete() +{ + hid_t fcpl_id; + /* We'll use dataspaces, filter pipelines, and attributes for this + * test. Create a number of distinct messages of each type. + */ + hid_t dspace_id[DELETE_NUM_MESGS] = {0}; + hid_t dcpl_id[DELETE_NUM_MESGS] = {0}; + int x; + hsize_t dims[] = DELETE_DIMS; + herr_t ret; + + /* Create a number of different dataspaces. + * For simplicity, each dataspace has only one element. + */ + for(x=0; x=0; --x) { + ret = H5Sclose(dspace_id[x]); + CHECK_I(ret, "H5Sclose"); + ret = H5Pclose(dcpl_id[x]); + CHECK_I(ret, "H5Pclose"); + } +} + + /**************************************************************** ** ** test_sohm(): Main Shared Object Header Message testing routine. @@ -2444,7 +2763,10 @@ test_sohm(void) test_sohm_size1(); /* Tests the sizes of files with one SOHM */ test_sohm_attrs(); /* Tests shared messages in attributes */ test_sohm_size2(0); /* Tests the sizes of files with multiple SOHMs */ - test_sohm_size2(1); /* Tests the sizes of files with multiple SOHMs */ + test_sohm_size2(1); /* Tests the sizes of files with multiple + * SOHMs, closing and reopening file after + * each write. */ + test_sohm_delete(); /* Test deleting shared messages */ } /* test_sohm() */ -- cgit v0.12