From 6e40802f06d0d4f12a710522fd977531d36fafe9 Mon Sep 17 00:00:00 2001 From: James Laird Date: Tue, 9 Jan 2007 10:11:16 -0500 Subject: [svn-r13126] More cleanup. Shared messages now use in-heap callbacks when searching for a matching message, which should improve performance. Tested on Windows, kagiso, and smirom. --- src/H5SM.c | 2 +- src/H5SMbtree2.c | 120 ++++++++++++++++++++++++++++++++++++++----------------- src/H5SMcache.c | 21 +++------- src/H5SMpkg.h | 2 +- test/tsohm.c | 3 +- 5 files changed, 91 insertions(+), 57 deletions(-) diff --git a/src/H5SM.c b/src/H5SM.c index 49052eb..7942eb7 100755 --- a/src/H5SM.c +++ b/src/H5SM.c @@ -753,7 +753,7 @@ H5SM_convert_btree_to_list(H5F_t * f, H5SM_index_header_t * header, hid_t dxpl_i /* 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) + if(H5B2_delete(f, dxpl_id, H5SM_INDEX, btree_addr, H5SM_btree_convert_to_list_op, list) < 0) HGOTO_ERROR(H5E_BTREE, H5E_CANTDELETE, FAIL, "unable to delete B-tree") done: diff --git a/src/H5SMbtree2.c b/src/H5SMbtree2.c index 1b929e7..0c07e9f 100755 --- a/src/H5SMbtree2.c +++ b/src/H5SMbtree2.c @@ -35,15 +35,21 @@ /* Local Typedefs */ /******************/ +/* Udata struct for call to H5SM_btree_compare_cb */ +typedef struct H5SM_compare_udata { + H5SM_mesg_key_t *key; /* Key; compare this against record in heap */ + herr_t ret; /* Return value; set this to result of memcmp */ +} H5SM_compare_udata; /********************/ /* Local Prototypes */ /********************/ /* JAMES: name these as "H5SM_btree_store", etc? */ -static herr_t H5SM_message_store(void *native, const void *udata); -static herr_t H5SM_message_retrieve(void *udata, const void *native); -static herr_t H5SM_message_debug(FILE *stream, const H5F_t *f, hid_t dxpl_id, +static herr_t H5SM_btree_compare_cb(const void *obj, size_t obj_len, void *_udata); +static herr_t H5SM_btree_store(void *native, const void *udata); +static herr_t H5SM_btree_retrieve(void *udata, const void *native); +static herr_t H5SM_btree_debug(FILE *stream, const H5F_t *f, hid_t dxpl_id, int indent, int fwidth, const void *record, const void *_udata); @@ -51,15 +57,15 @@ static herr_t H5SM_message_debug(FILE *stream, const H5F_t *f, hid_t dxpl_id, /* Library Private Variables */ /*****************************/ /* v2 B-tree class for SOHM indexes*/ -const H5B2_class_t H5SM_INDEX[1]={{ /* B-tree class information */ - H5B2_SOHM_INDEX_ID, /* Type of B-tree */ - sizeof(H5SM_sohm_t), /* Size of native record */ - H5SM_message_store, /* Record storage callback */ - H5SM_message_retrieve, /* Record retrieval callback */ - H5SM_message_compare, /* Record comparison callback */ - H5SM_message_encode, /* Record encoding callback */ - H5SM_message_decode, /* Record decoding callback */ - H5SM_message_debug /* Record debugging callback */ +const H5B2_class_t H5SM_INDEX[1]={{ /* B-tree class information */ + H5B2_SOHM_INDEX_ID, /* Type of B-tree */ + sizeof(H5SM_sohm_t), /* Size of native record */ + H5SM_btree_store, /* Record storage callback */ + H5SM_btree_retrieve, /* Record retrieval callback */ + H5SM_message_compare, /* Record comparison callback */ + H5SM_message_encode, /* Record encoding callback */ + H5SM_message_decode, /* Record decoding callback */ + H5SM_btree_debug /* Record debugging callback */ }}; /*******************/ @@ -68,6 +74,44 @@ const H5B2_class_t H5SM_INDEX[1]={{ /* B-tree class information */ /*------------------------------------------------------------------------- + * Function: H5SM_btree_compare_cb + * + * Purpose: Callback for H5HF_op, used in H5SM_btree_compare_cb below. + * Determines whether the search key passed in in _UDATA is + * equal to OBJ or not. + * + * Passes back the result in _UDATA->RET + * + * Return: Negative on error, non-negative on success + * + * Programmer: James Laird + * Monday, January 8, 2007 + * + *------------------------------------------------------------------------- + */ +static herr_t +H5SM_btree_compare_cb(const void *obj, size_t obj_len, void *_udata) +{ + H5SM_compare_udata *udata = (H5SM_compare_udata *)_udata; + herr_t ret_value = SUCCEED; + + FUNC_ENTER_NOAPI_NOINIT_NOFUNC(H5SM_btree_compare_cb) + + /* If the encoding sizes are different, it's not the same object */ + if(udata->key->encoding_size != obj_len) { + if(udata->key->encoding_size > obj_len) + udata->ret = 1; + else + udata->ret = -1; + } else { + /* Sizes are the same. Return result of memcmp */ + udata->ret = HDmemcmp(udata->key->encoding, obj, obj_len); + } + + FUNC_LEAVE_NOAPI(ret_value) +} + +/*------------------------------------------------------------------------- * Function: H5SM_message_compare * * Purpose: Determine whether the search key rec1 represents a shared @@ -91,7 +135,7 @@ H5SM_message_compare(const void *rec1, const void *rec2) int64_t hash_diff; /* Has to be able to hold two 32-bit values */ herr_t ret_value = 0; - FUNC_ENTER_NOAPI_NOINIT_NOFUNC(H5SM_message_compare) + FUNC_ENTER_NOAPI_NOINIT(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 @@ -114,19 +158,21 @@ H5SM_message_compare(const void *rec1, const void *rec2) } else { - unsigned char buf2[H5O_MESG_MAX_SIZE]; + /* Hash values match, but we don't have a heap ID for the key. + * Compare the encoded message with the one in the heap. + */ + /* JAMES: can we hold off encoding until now? */ + H5SM_compare_udata udata; herr_t ret; - /* We need to see if this message is in fact the message stored - * in the heap. Read it from the heap and compare the two. - */ - HDmemset(buf2, 0, (size_t)H5O_MESG_MAX_SIZE); + /* Casting away const OK. -JML */ + udata.key = key; - ret = H5HF_read(key->fheap, H5AC_dxpl_id, &(mesg->fheap_id), &buf2); + /* Call heap op routine with comparison callback */ + ret = H5HF_op(key->fheap, H5AC_dxpl_id, &(mesg->fheap_id), H5SM_btree_compare_cb, &udata); HDassert(ret >= 0); - /* JAMES: I think I want to use in-heap callback here. */ - ret_value = HDmemcmp(key->encoding, buf2, key->encoding_size); + ret_value = udata.ret; } } else { @@ -142,7 +188,7 @@ H5SM_message_compare(const void *rec1, const void *rec2) /*------------------------------------------------------------------------- - * Function: H5SM_message_store + * Function: H5SM_btree_store * * Purpose: Store a H5SM_sohm_t SOHM message in the B-tree by copying it * from UDATA to NATIVE. @@ -156,19 +202,19 @@ H5SM_message_compare(const void *rec1, const void *rec2) *------------------------------------------------------------------------- */ static herr_t -H5SM_message_store(void *native, const void *udata) +H5SM_btree_store(void *native, const void *udata) { - FUNC_ENTER_NOAPI_NOINIT_NOFUNC(H5SM_message_store) + FUNC_ENTER_NOAPI_NOINIT_NOFUNC(H5SM_btree_store) /* Copy the source message to the B-tree */ *(H5SM_sohm_t *)native = *(const H5SM_sohm_t *)udata; FUNC_LEAVE_NOAPI(SUCCEED) -} /* end H5SM_message_store */ +} /* end H5SM_btree_store */ /*------------------------------------------------------------------------- - * Function: H5SM_message_retrieve + * Function: H5SM_btree_retrieve * * Purpose: Retrieve a H5SM_sohm_t SOHM message from the B-tree by * copying it from NATIVE to UDATA. @@ -184,15 +230,15 @@ H5SM_message_store(void *native, const void *udata) *------------------------------------------------------------------------- */ static herr_t -H5SM_message_retrieve(void *udata, const void *native) +H5SM_btree_retrieve(void *udata, const void *native) { - FUNC_ENTER_NOAPI_NOINIT_NOFUNC(H5SM_message_retrieve) + FUNC_ENTER_NOAPI_NOINIT_NOFUNC(H5SM_btree_retrieve) /* Copy the B-tree's native message to the udata buffer */ *(H5SM_sohm_t *)udata = *(const H5SM_sohm_t *)native; FUNC_LEAVE_NOAPI(SUCCEED) -} /* end H5SM_message_retrieve */ +} /* end H5SM_btree_retrieve */ /*------------------------------------------------------------------------- @@ -237,7 +283,7 @@ H5SM_message_encode(const H5F_t UNUSED *f, uint8_t *raw, const void *_nrecord) * *------------------------------------------------------------------------- */ -/* JAMES: can we combine this with H5SMcache functions? */ +/* JAMES: move to H5SM.c or something */ herr_t H5SM_message_decode(const H5F_t UNUSED *f, const uint8_t *raw, void *_nrecord) { @@ -255,7 +301,7 @@ H5SM_message_decode(const H5F_t UNUSED *f, const uint8_t *raw, void *_nrecord) /*------------------------------------------------------------------------- - * Function: H5SM_message_debug + * Function: H5SM_btree_debug * * Purpose: Print debugging information for a H5SM_sohm_t. * @@ -268,18 +314,18 @@ H5SM_message_decode(const H5F_t UNUSED *f, const uint8_t *raw, void *_nrecord) *------------------------------------------------------------------------- */ static herr_t -H5SM_message_debug(FILE *stream, const H5F_t UNUSED *f, hid_t UNUSED dxpl_id, +H5SM_btree_debug(FILE *stream, const H5F_t UNUSED *f, hid_t UNUSED dxpl_id, int indent, int fwidth, const void *record, const void UNUSED *_udata) { const H5SM_sohm_t *sohm = (const H5SM_sohm_t *)record; - FUNC_ENTER_NOAPI_NOINIT_NOFUNC(H5SM_message_debug) + FUNC_ENTER_NOAPI_NOINIT_NOFUNC(H5SM_btree_debug) HDfprintf(stream, "%*s%-*s {%a, %lo, %Hu}\n", indent, "", fwidth, "Record:", sohm->fheap_id, sohm->hash, sohm->ref_count); FUNC_LEAVE_NOAPI(SUCCEED) -} /* end H5SM_message_debug */ +} /* end H5SM_btree_debug */ /*------------------------------------------------------------------------- @@ -361,7 +407,7 @@ H5SM_decr_ref(void *record, void *op_data, hbool_t *changed) /*------------------------------------------------------------------------- - * Function: H5SM_convert_to_list_op + * Function: H5SM_btree_convert_to_list_op * * Purpose: An H5B2_remove_t callback function to convert a SOHM * B-tree index to a list. @@ -377,13 +423,13 @@ H5SM_decr_ref(void *record, void *op_data, hbool_t *changed) *------------------------------------------------------------------------- */ herr_t -H5SM_convert_to_list_op(const void * record, void *op_data) +H5SM_btree_convert_to_list_op(const void * record, void *op_data) { const H5SM_sohm_t *message = (const H5SM_sohm_t *) record; const H5SM_list_t *list = (const H5SM_list_t *) op_data; hsize_t x; - FUNC_ENTER_NOAPI_NOINIT_NOFUNC(H5SM_convert_to_list_op) + FUNC_ENTER_NOAPI_NOINIT_NOFUNC(H5SM_btree_convert_to_list_op) HDassert(record); HDassert(op_data); diff --git a/src/H5SMcache.c b/src/H5SMcache.c index 70f6177..7f1c282 100644 --- a/src/H5SMcache.c +++ b/src/H5SMcache.c @@ -22,28 +22,19 @@ /***********/ /* Headers */ /***********/ -/* JAMES: these need to go first or else FILE isn't defined in H5Fpkg.h */ -/* JAMES: which of these are really needed? H5Fpkg.h, even? */ -#include "H5private.h" /* Generic Functions */ -#include "H5Aprivate.h" /* Attributes */ -#include "H5ACprivate.h" /* Metadata cache */ -#include "H5Dprivate.h" /* Datasets */ #include "H5Eprivate.h" /* Error handling */ - -#include "H5Fpkg.h" /* File access */ #include "H5FLprivate.h" /* Free Lists */ -#include "H5FOprivate.h" /* File objects */ -#include "H5HLprivate.h" /* Local heaps */ -#include "H5MFprivate.h" /* File memory management */ #include "H5MMprivate.h" /* Memory management */ -#include "H5Vprivate.h" /* Vectors and arrays */ + +#include "H5Fpkg.h" /* File access */ #include "H5SMpkg.h" /* Shared object header messages */ -#include "H5FDprivate.h" /* File drivers */ /****************/ /* Local Macros */ /****************/ -/* JAMES: should this change according to address size? */ +/* JAMES: should this change according to address size? + Answer: shouldn't use this ever anyway. + */ #define H5F_LISTBUF_SIZE H5SM_LIST_SIZEOF_MAGIC + H5O_SHMESG_MAX_LIST_SIZE * 16 #define H5SM_LIST_VERSION 0 /* Verion of Shared Object Header Message List Indexes */ @@ -641,8 +632,6 @@ H5SM_dest_list(H5F_t UNUSED *f, H5SM_list_t* list) } /* end H5SM_dest_list */ -/* JAMES: should this number be constant, or should it increase and decrease as - * messages are added and removed? */ /*------------------------------------------------------------------------- * Function: H5SM_list_size * diff --git a/src/H5SMpkg.h b/src/H5SMpkg.h index e2e839b..4a89235 100755 --- a/src/H5SMpkg.h +++ b/src/H5SMpkg.h @@ -200,7 +200,7 @@ H5_DLL herr_t H5SM_incr_ref(void *record, void *op_data, hbool_t *changed); H5_DLL herr_t H5SM_decr_ref(void *record, void *op_data, hbool_t *changed); /* H5B2_remove_t callback to add messages to a list index */ -H5_DLL herr_t H5SM_convert_to_list_op(const void * record, void *op_data); +H5_DLL herr_t H5SM_btree_convert_to_list_op(const void * record, void *op_data); /* Fractal heap 'op' callback to compute hash value for message "in place" */ H5_DLL herr_t H5SM_get_hash_fh_cb(const void *obj, size_t obj_len, void *_udata); diff --git a/test/tsohm.c b/test/tsohm.c index 23a2975..9fcbf41 100644 --- a/test/tsohm.c +++ b/test/tsohm.c @@ -356,7 +356,7 @@ static void test_sohm_fcpl(void) * have corrupted the fcpl, although we do need to reset the * second index that we changed above. */ - ret = H5Pset_shared_mesg_index(fcpl_id, 1, test_type_flags[1], 15 /* JAMES */); + ret = H5Pset_shared_mesg_index(fcpl_id, 1, test_type_flags[1], 15); CHECK_I(ret, "H5Pset_shared_mesg_index"); ret = H5Pset_shared_mesg_phase_change(fcpl_id, 10, 11); CHECK_I(ret, "H5Pset_shared_mesg_phase_change"); @@ -1981,7 +1981,6 @@ static void test_sohm_size2(int close_reopen) fcpl_id = H5Pcreate(H5P_FILE_CREATE); CHECK_I(fcpl_id, "H5Pcreate"); - /* JAMES: should be zero-indexed? */ ret = H5Pset_shared_mesg_nindexes(fcpl_id, 3); CHECK_I(ret, "H5Pset_shared_mesg_nindexes"); ret = H5Pset_shared_mesg_index(fcpl_id, 0, H5O_MESG_SDSPACE_FLAG | H5O_MESG_DTYPE_FLAG, 20); -- cgit v0.12