From 8fa9daa17424a76c715f63afc35cfdcd0c3add32 Mon Sep 17 00:00:00 2001 From: James Laird Date: Tue, 16 Jan 2007 12:19:11 -0500 Subject: [svn-r13143] Refactoring. Added can_share callback for OH messages. This determines whether the message is allowed to be shared in the heap (committed and immutable datatypes can't be). Fixed a bug in the dense attribute storage that tried to open the shared message heap when it hadn't been created yet. Made the test to extend shared dataspace messages a bit more robust. Refactored the code the searches a shared message list index to be a little more efficient. A few other minor changes. Tested on smirom, kagiso, Windows, and juniper. --- src/H5Adense.c | 43 ++++++++++------ src/H5Oattr.c | 1 + src/H5Ocont.c | 1 + src/H5Odtype.c | 49 +++++++++++++++++- src/H5Oefl.c | 1 + src/H5Ofill.c | 2 + src/H5Oginfo.c | 1 + src/H5Olayout.c | 1 + src/H5Olinfo.c | 1 + src/H5Olink.c | 1 + src/H5Omessage.c | 52 +++++++++++++++++++ src/H5Omtime.c | 2 + src/H5Oname.c | 1 + src/H5Opkg.h | 1 + src/H5Opline.c | 1 + src/H5Oprivate.h | 1 + src/H5Osdspace.c | 1 + src/H5Oshared.c | 4 +- src/H5Ostab.c | 1 + src/H5SM.c | 96 +++++++++++++++++------------------ src/H5SMbtree2.c | 3 +- src/H5SMcache.c | 40 +++++++-------- src/H5SMpkg.h | 3 +- src/H5Tcommit.c | 15 +++--- test/tsohm.c | 149 +++++++++++++++++++++++++++++++++++++++++++------------ 25 files changed, 344 insertions(+), 127 deletions(-) diff --git a/src/H5Adense.c b/src/H5Adense.c index c492932..e9eeafa 100644 --- a/src/H5Adense.c +++ b/src/H5Adense.c @@ -340,8 +340,10 @@ H5A_dense_open(H5F_t *f, hid_t dxpl_id, const H5O_t *oh, const char *name) HGOTO_ERROR(H5E_ATTR, H5E_CANTGET, NULL, "can't get shared message heap address") /* Open the fractal heap for shared header messages */ - if(NULL == (shared_fheap = H5HF_open(f, dxpl_id, shared_fheap_addr))) - HGOTO_ERROR(H5E_ATTR, H5E_CANTOPENOBJ, NULL, "unable to open fractal heap") + if(H5F_addr_defined(shared_fheap_addr)) { + if(NULL == (shared_fheap = H5HF_open(f, dxpl_id, shared_fheap_addr))) + HGOTO_ERROR(H5E_ATTR, H5E_CANTOPENOBJ, NULL, "unable to open fractal heap") + } } /* end if */ /* Create the "udata" information for v2 B-tree record modify */ @@ -412,6 +414,7 @@ H5A_dense_insert(H5F_t *f, hid_t dxpl_id, const H5O_t *oh, unsigned mesg_flags, HGOTO_ERROR(H5E_ATTR, H5E_CANTGET, FAIL, "can't determine if attributes are shared") /* Get handle for shared message heap, if attributes are sharable */ + /* JAMES: does this work if only very large attributes are shared? */ if(attr_sharable) { haddr_t shared_fheap_addr; /* Address of fractal heap to use */ @@ -420,8 +423,10 @@ H5A_dense_insert(H5F_t *f, hid_t dxpl_id, const H5O_t *oh, unsigned mesg_flags, HGOTO_ERROR(H5E_ATTR, H5E_CANTGET, FAIL, "can't get shared message heap address") /* Open the fractal heap for shared header messages */ - if(NULL == (shared_fheap = H5HF_open(f, dxpl_id, shared_fheap_addr))) - HGOTO_ERROR(H5E_ATTR, H5E_CANTOPENOBJ, FAIL, "unable to open fractal heap") + if(H5F_addr_defined(shared_fheap_addr)) { + if(NULL == (shared_fheap = H5HF_open(f, dxpl_id, shared_fheap_addr))) + HGOTO_ERROR(H5E_ATTR, H5E_CANTOPENOBJ, FAIL, "unable to open fractal heap") + } } /* end if */ /* Open the fractal heap */ @@ -638,8 +643,10 @@ H5A_dense_write(H5F_t *f, hid_t dxpl_id, const H5O_t *oh, H5A_t *attr) HGOTO_ERROR(H5E_ATTR, H5E_CANTGET, FAIL, "can't get shared message heap address") /* Open the fractal heap for shared header messages */ - if(NULL == (shared_fheap = H5HF_open(f, dxpl_id, shared_fheap_addr))) - HGOTO_ERROR(H5E_ATTR, H5E_CANTOPENOBJ, FAIL, "unable to open fractal heap") + if(H5F_addr_defined(shared_fheap_addr)) { + if(NULL == (shared_fheap = H5HF_open(f, dxpl_id, shared_fheap_addr))) + HGOTO_ERROR(H5E_ATTR, H5E_CANTOPENOBJ, FAIL, "unable to open fractal heap") + } } /* end if */ /* Open the fractal heap */ @@ -770,8 +777,10 @@ H5A_dense_rename(H5F_t *f, hid_t dxpl_id, const H5O_t *oh, const char *old_name, HGOTO_ERROR(H5E_ATTR, H5E_CANTGET, FAIL, "can't get shared message heap address") /* Open the fractal heap for shared header messages */ - if(NULL == (shared_fheap = H5HF_open(f, dxpl_id, shared_fheap_addr))) - HGOTO_ERROR(H5E_ATTR, H5E_CANTOPENOBJ, FAIL, "unable to open fractal heap") + if(H5F_addr_defined(shared_fheap_addr)) { + if(NULL == (shared_fheap = H5HF_open(f, dxpl_id, shared_fheap_addr))) + HGOTO_ERROR(H5E_ATTR, H5E_CANTOPENOBJ, FAIL, "unable to open fractal heap") + } } /* end if */ /* Open the fractal heap */ @@ -999,8 +1008,10 @@ H5A_dense_iterate(H5F_t *f, hid_t dxpl_id, hid_t loc_id, haddr_t attr_fheap_addr HGOTO_ERROR(H5E_ATTR, H5E_CANTGET, FAIL, "can't get shared message heap address") /* Open the fractal heap for shared header messages */ - if(NULL == (shared_fheap = H5HF_open(f, dxpl_id, shared_fheap_addr))) - HGOTO_ERROR(H5E_ATTR, H5E_CANTOPENOBJ, FAIL, "unable to open fractal heap") + if(H5F_addr_defined(shared_fheap_addr)) { + if(NULL == (shared_fheap = H5HF_open(f, dxpl_id, shared_fheap_addr))) + HGOTO_ERROR(H5E_ATTR, H5E_CANTOPENOBJ, FAIL, "unable to open fractal heap") + } } /* end if */ /* Construct the user data for v2 B-tree iterator callback */ @@ -1141,8 +1152,10 @@ H5A_dense_remove(H5F_t *f, hid_t dxpl_id, const H5O_t *oh, const char *name) HGOTO_ERROR(H5E_ATTR, H5E_CANTGET, FAIL, "can't get shared message heap address") /* Open the fractal heap for shared header messages */ - if(NULL == (shared_fheap = H5HF_open(f, dxpl_id, shared_fheap_addr))) - HGOTO_ERROR(H5E_ATTR, H5E_CANTOPENOBJ, FAIL, "unable to open fractal heap") + if(H5F_addr_defined(shared_fheap_addr)) { + if(NULL == (shared_fheap = H5HF_open(f, dxpl_id, shared_fheap_addr))) + HGOTO_ERROR(H5E_ATTR, H5E_CANTOPENOBJ, FAIL, "unable to open fractal heap") + } } /* end if */ /* Set up the user data for the v2 B-tree 'record remove' callback */ @@ -1223,8 +1236,10 @@ H5A_dense_exists(H5F_t *f, hid_t dxpl_id, const H5O_t *oh, const char *name) HGOTO_ERROR(H5E_ATTR, H5E_CANTGET, FAIL, "can't get shared message heap address") /* Open the fractal heap for shared header messages */ - if(NULL == (shared_fheap = H5HF_open(f, dxpl_id, shared_fheap_addr))) - HGOTO_ERROR(H5E_ATTR, H5E_CANTOPENOBJ, FAIL, "unable to open fractal heap") + if(H5F_addr_defined(shared_fheap_addr)) { + if(NULL == (shared_fheap = H5HF_open(f, dxpl_id, shared_fheap_addr))) + HGOTO_ERROR(H5E_ATTR, H5E_CANTOPENOBJ, FAIL, "unable to open fractal heap") + } } /* end if */ /* Create the "udata" information for v2 B-tree record 'find' */ diff --git a/src/H5Oattr.c b/src/H5Oattr.c index 037239f..c29c19e 100644 --- a/src/H5Oattr.c +++ b/src/H5Oattr.c @@ -57,6 +57,7 @@ const H5O_msg_class_t H5O_MSG_ATTR[1] = {{ NULL /* H5O_attr_link */, /* link method */ H5O_attr_get_share, /* get share method */ H5O_attr_set_share, /* set share method */ + NULL, /*can share method */ H5O_attr_is_shared, /*is shared method */ H5O_attr_pre_copy_file, /* pre copy native value to file */ H5O_attr_copy_file, /* copy native value to file */ diff --git a/src/H5Ocont.c b/src/H5Ocont.c index eab8749..e5000bd 100644 --- a/src/H5Ocont.c +++ b/src/H5Ocont.c @@ -58,6 +58,7 @@ const H5O_msg_class_t H5O_MSG_CONT[1] = {{ H5O_cont_delete, /* file delete method */ NULL, /* link method */ NULL, /*get share method */ + NULL, /*can share method */ NULL, /*set share method */ NULL, /*is shared method */ NULL, /* pre copy native value to file */ diff --git a/src/H5Odtype.c b/src/H5Odtype.c index 429db74..dd7a938 100644 --- a/src/H5Odtype.c +++ b/src/H5Odtype.c @@ -35,7 +35,8 @@ static herr_t H5O_dtype_reset(void *_mesg); static herr_t H5O_dtype_free(void *_mesg); static void *H5O_dtype_get_share(const void *_mesg, H5O_shared_t *sh); static herr_t H5O_dtype_set_share(void *_mesg, const H5O_shared_t *sh); -static herr_t H5O_dtype_is_shared(const void *_mesg); +static htri_t H5O_dtype_can_share(const void *_mesg); +static htri_t H5O_dtype_is_shared(const void *_mesg); static herr_t H5O_dtype_pre_copy_file(H5F_t *file_src, const H5O_msg_class_t *type, const void *mesg_src, hbool_t *deleted, const H5O_copy_t *cpy_info, void *_udata); static void *H5O_dtype_copy_file(H5F_t *file_src, const H5O_msg_class_t *mesg_type, @@ -59,6 +60,7 @@ const H5O_msg_class_t H5O_MSG_DTYPE[1] = {{ NULL, /* link method */ H5O_dtype_get_share, /* get share method */ H5O_dtype_set_share, /* set share method */ + H5O_dtype_can_share, /* can share method */ H5O_dtype_is_shared, /* is shared method */ H5O_dtype_pre_copy_file, /* pre copy native value to file */ H5O_dtype_copy_file, /* copy native value to file */ @@ -1346,6 +1348,51 @@ H5O_dtype_set_share(void *_mesg/*in,out*/, const H5O_shared_t *sh) /*------------------------------------------------------------------------- + * Function: H5O_dtype_can_share + * + * Purpose: Determines if this datatype is allowed to be shared or + * not. Immutable datatypes or datatypes that are already + * shared cannot be shared (again). + * + * Return: TRUE if datatype can be shared + * FALSE if datatype may not shared + * Negative on failure + * + * Programmer: James Laird + * Monday, October 16, 2006 + * + *------------------------------------------------------------------------- + */ +static htri_t +H5O_dtype_can_share(const void *_mesg) +{ + const H5T_t *mesg = (const H5T_t *)_mesg; + htri_t tri_ret; + htri_t ret_value = TRUE; + + FUNC_ENTER_NOAPI_NOINIT(H5O_dtype_can_share) + + HDassert(mesg); + + /* Don't share immutable datatypes */ + if((tri_ret = H5T_is_immutable(mesg)) > 0) + HGOTO_DONE(FALSE) + else if(tri_ret < 0) + HGOTO_ERROR(H5E_OHDR, H5E_BADTYPE, FAIL, "can't tell if datatype is immutable") + + /* Don't share committed datatypes */ + if((tri_ret = H5T_committed(mesg)) > 0) + HGOTO_DONE(FALSE) + else if(tri_ret < 0) + HGOTO_ERROR(H5E_OHDR, H5E_BADTYPE, FAIL, "can't tell if datatype is shared") + +done: + FUNC_LEAVE_NOAPI(ret_value) +} /* end H5O_dtype_can_share() */ + + + +/*------------------------------------------------------------------------- * Function: H5O_dtype_is_shared * * Purpose: Determines if this datatype is shared (committed or a SOHM) diff --git a/src/H5Oefl.c b/src/H5Oefl.c index eb6e6cf..7564e7f 100644 --- a/src/H5Oefl.c +++ b/src/H5Oefl.c @@ -53,6 +53,7 @@ const H5O_msg_class_t H5O_MSG_EFL[1] = {{ NULL, /* link method */ NULL, /*get share method */ NULL, /*set share method */ + NULL, /*can share method */ NULL, /*is shared method */ NULL, /* pre copy native value to file */ H5O_efl_copy_file, /* copy native value to file */ diff --git a/src/H5Ofill.c b/src/H5Ofill.c index 73732b2..ea08489 100644 --- a/src/H5Ofill.c +++ b/src/H5Ofill.c @@ -66,6 +66,7 @@ const H5O_msg_class_t H5O_MSG_FILL[1] = {{ NULL, /* link method */ H5O_fill_new_get_share, /* get share method */ H5O_fill_new_set_share, /* set share method */ + NULL, /*can share method */ H5O_fill_new_is_shared, /* is shared method */ NULL, /* pre copy native value to file */ NULL, /* copy native value to file */ @@ -88,6 +89,7 @@ const H5O_msg_class_t H5O_MSG_FILL_NEW[1] = {{ NULL, /* link method */ H5O_fill_new_get_share, /* get share method */ H5O_fill_new_set_share, /* set share method */ + NULL, /*can share method */ H5O_fill_new_is_shared, /* is shared method */ NULL, /* pre copy native value to file */ NULL, /* copy native value to file */ diff --git a/src/H5Oginfo.c b/src/H5Oginfo.c index f448bf4..d8a75c1 100644 --- a/src/H5Oginfo.c +++ b/src/H5Oginfo.c @@ -57,6 +57,7 @@ const H5O_msg_class_t H5O_MSG_GINFO[1] = {{ NULL, /* link method */ NULL, /*get share method */ NULL, /*set share method */ + NULL, /*can share method */ NULL, /*is shared method */ NULL, /* pre copy native value to file */ NULL, /* copy native value to file */ diff --git a/src/H5Olayout.c b/src/H5Olayout.c index 305ab17..e161d3c 100644 --- a/src/H5Olayout.c +++ b/src/H5Olayout.c @@ -60,6 +60,7 @@ const H5O_msg_class_t H5O_MSG_LAYOUT[1] = {{ NULL, /* link method */ NULL, /*get share method */ NULL, /*set share method */ + NULL, /*can share method */ NULL, /*is shared method */ NULL, /* pre copy native value to file */ H5O_layout_copy_file, /* copy native value to file */ diff --git a/src/H5Olinfo.c b/src/H5Olinfo.c index c5cede0..d6267b0 100644 --- a/src/H5Olinfo.c +++ b/src/H5Olinfo.c @@ -65,6 +65,7 @@ const H5O_msg_class_t H5O_MSG_LINFO[1] = {{ NULL, /* link method */ NULL, /*get share method */ NULL, /*set share method */ + NULL, /*can share method */ NULL, /*is shared method */ NULL, /* pre copy native value to file */ H5O_linfo_copy_file, /* copy native value to file */ diff --git a/src/H5Olink.c b/src/H5Olink.c index 8de560f..5a58db3 100644 --- a/src/H5Olink.c +++ b/src/H5Olink.c @@ -69,6 +69,7 @@ const H5O_msg_class_t H5O_MSG_LINK[1] = {{ NULL, /* link method */ NULL, /*get share method */ NULL, /*set share method */ + NULL, /*can share method */ NULL, /*is shared method */ H5O_link_pre_copy_file, /* pre copy native value to file */ H5O_link_copy_file, /* copy native value to file */ diff --git a/src/H5Omessage.c b/src/H5Omessage.c index e1ae146..54bbdf4 100644 --- a/src/H5Omessage.c +++ b/src/H5Omessage.c @@ -1556,6 +1556,58 @@ done: /*------------------------------------------------------------------------- + * Function: H5O_msg_can_share + * + * Purpose: Call the 'can share' method for a + * particular class of object header. This returns TRUE + * if the message is allowed to be put in the shared message + * heap and false otherwise (e.g., for committed or immutable + * datatypes). + * + * Return: Object can be shared: TRUE + * Object cannot be shared: FALSE + * + * Programmer: James Laird + * January 12 2007 + * + *------------------------------------------------------------------------- + */ +htri_t +H5O_msg_can_share(unsigned type_id, const void *mesg) +{ + const H5O_msg_class_t *type; /* Actual H5O class type for the ID */ + htri_t ret_value = FALSE; + + FUNC_ENTER_NOAPI_NOFUNC(H5O_msg_can_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(mesg); + + /* If the message doesn't have an is_shared callback, it's not sharable + * and thus can't be shared. + */ + /* If there is a can_share callback, use it */ + if((type->can_share)) + ret_value = (type->can_share)(mesg); + else { + /* Otherwise, the message can be shared if messages of this type are + * shareable in general; i.e., if they have a set_share callback + */ + if(type->set_share) + ret_value = TRUE; + else + ret_value = FALSE; + } + + FUNC_LEAVE_NOAPI(ret_value) +} /* end H5O_msg_can_share() */ + + +/*------------------------------------------------------------------------- * Function: H5O_msg_is_shared * * Purpose: Call the 'is_shared' method for a diff --git a/src/H5Omtime.c b/src/H5Omtime.c index 7b9c4fd..ccff078 100644 --- a/src/H5Omtime.c +++ b/src/H5Omtime.c @@ -60,6 +60,7 @@ const H5O_msg_class_t H5O_MSG_MTIME[1] = {{ NULL, /* link method */ NULL, /*get share method */ NULL, /*set share method */ + NULL, /*can share method */ NULL, /*is shared method */ NULL, /* pre copy native value to file */ NULL, /* copy native value to file */ @@ -83,6 +84,7 @@ const H5O_msg_class_t H5O_MSG_MTIME_NEW[1] = {{ NULL, /* link method */ NULL, /*get share method */ NULL, /*set share method */ + NULL, /*can share method */ NULL, /*is shared method */ NULL, /* pre copy native value to file */ NULL, /* copy native value to file */ diff --git a/src/H5Oname.c b/src/H5Oname.c index fc1aa21..2314682 100644 --- a/src/H5Oname.c +++ b/src/H5Oname.c @@ -57,6 +57,7 @@ const H5O_msg_class_t H5O_MSG_NAME[1] = {{ NULL, /* link method */ NULL, /*get share method */ NULL, /*set share method */ + NULL, /*can share method */ NULL, /*is shared method */ NULL, /* pre copy native value to file */ NULL, /* copy native value to file */ diff --git a/src/H5Opkg.h b/src/H5Opkg.h index e9d9171..ddd7799 100644 --- a/src/H5Opkg.h +++ b/src/H5Opkg.h @@ -167,6 +167,7 @@ struct H5O_msg_class_t { herr_t (*link)(H5F_t *, hid_t, const void *); /* Increment any links in file reference by this message */ void *(*get_share)(const void*, struct H5O_shared_t*); /* Get shared information */ herr_t (*set_share)(void*, const struct H5O_shared_t*); /* Set shared information */ + htri_t (*can_share)(const void*); /* Is message allowed to be shared? */ htri_t (*is_shared)(const void*); /* Is message shared? */ herr_t (*pre_copy_file)(H5F_t *, const H5O_msg_class_t *, const void *, hbool_t *, const H5O_copy_t *, void *); /*"pre copy" action when copying native value to file */ void *(*copy_file)(H5F_t *, const H5O_msg_class_t *, void *, H5F_t *, hid_t, H5O_copy_t *, void *); /*copy native value to file */ diff --git a/src/H5Opline.c b/src/H5Opline.c index abd7471..7ec6e9f 100644 --- a/src/H5Opline.c +++ b/src/H5Opline.c @@ -58,6 +58,7 @@ const H5O_msg_class_t H5O_MSG_PLINE[1] = {{ NULL, /* link method */ H5O_pline_get_share, /* get share method */ H5O_pline_set_share, /* set share method */ + NULL, /*can share method */ H5O_pline_is_shared, /* is shared method */ H5O_pline_pre_copy_file, /* pre copy native value to file */ NULL, /* copy native value to file */ diff --git a/src/H5Oprivate.h b/src/H5Oprivate.h index f40d28e..e1a08ce 100644 --- a/src/H5Oprivate.h +++ b/src/H5Oprivate.h @@ -419,6 +419,7 @@ H5_DLL size_t H5O_msg_mesg_size(const H5F_t *f, unsigned type_id, const void *me size_t extra_raw); H5_DLL void *H5O_msg_get_share(unsigned type_id, const void *mesg, H5O_shared_t *share); H5_DLL htri_t H5O_msg_is_shared(unsigned type_id, const void *mesg); +H5_DLL htri_t H5O_msg_can_share(unsigned type_id, const void *mesg); H5_DLL herr_t H5O_msg_set_share(unsigned type_id, H5O_shared_t *share, void *mesg); 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); diff --git a/src/H5Osdspace.c b/src/H5Osdspace.c index e5af3a6..d90ee3f 100644 --- a/src/H5Osdspace.c +++ b/src/H5Osdspace.c @@ -54,6 +54,7 @@ const H5O_msg_class_t H5O_MSG_SDSPACE[1] = {{ NULL, /* link method */ H5O_sdspace_get_share, /* get share method */ H5O_sdspace_set_share, /* set share method */ + NULL, /*can share method */ H5O_sdspace_is_shared, /* is shared method */ H5O_sdspace_pre_copy_file, /* pre copy native value to file */ NULL, /* copy native value to file */ diff --git a/src/H5Oshared.c b/src/H5Oshared.c index d665742..ab341d6 100644 --- a/src/H5Oshared.c +++ b/src/H5Oshared.c @@ -67,6 +67,7 @@ const H5O_msg_class_t H5O_MSG_SHARED[1] = {{ H5O_shared_link, /*link method */ NULL, /*get share method */ NULL, /*set share method */ + NULL, /*can share method */ NULL, /*is shared method */ H5O_shared_pre_copy_file, /* pre copy native value to file */ H5O_shared_copy_file, /* copy native value to file */ @@ -381,8 +382,7 @@ H5O_shared_encode(H5F_t *f, uint8_t *buf/*out*/, const void *_mesg) /* If this message is shared in the heap, we need to use version 3 of the * encoding and encode the SHARED_IN_HEAP flag. */ - /* JAMES: also use "use latest version" flag here */ - if(mesg->flags & H5O_SHARED_IN_HEAP_FLAG) { + if(mesg->flags & H5O_SHARED_IN_HEAP_FLAG || H5F_USE_LATEST_FORMAT(f)) { version = H5O_SHARED_VERSION; } else { diff --git a/src/H5Ostab.c b/src/H5Ostab.c index a9fe38d..5865dca 100644 --- a/src/H5Ostab.c +++ b/src/H5Ostab.c @@ -65,6 +65,7 @@ const H5O_msg_class_t H5O_MSG_STAB[1] = {{ NULL, /* link method */ NULL, /*get share method */ NULL, /*set share method */ + NULL, /*can share method */ NULL, /*is shared method */ NULL, /* pre copy native value to file */ H5O_stab_copy_file, /* copy native value to file */ diff --git a/src/H5SM.c b/src/H5SM.c index 94d1d67..43781f3 100755 --- a/src/H5SM.c +++ b/src/H5SM.c @@ -796,6 +796,7 @@ H5SM_try_share(H5F_t *f, hid_t dxpl_id, unsigned type_id, void *mesg) H5SM_master_table_t *table = NULL; unsigned cache_flags = H5AC__NO_FLAGS_SET; ssize_t index_num; + htri_t tri_ret; herr_t ret_value = TRUE; FUNC_ENTER_NOAPI(H5SM_try_share, FAIL) @@ -805,28 +806,12 @@ H5SM_try_share(H5F_t *f, hid_t dxpl_id, unsigned type_id, void *mesg) if(f->shared->sohm_addr == HADDR_UNDEF) HGOTO_DONE(FALSE); - /* Type-specific checks */ - /* JAMES: should this go here? Should there be a "can share" callback? */ - /* QAK: Yes, a "can share" callback would be very good here, this chunk of - * code is really violating the encapsulation of the datatype class - */ - if(type_id == H5O_DTYPE_ID) - { - htri_t tri_ret; - - /* Don't share immutable datatypes */ - if((tri_ret = H5T_is_immutable((H5T_t*) mesg)) > 0) - HGOTO_DONE(FALSE) - else if(tri_ret < 0) - HGOTO_ERROR(H5E_OHDR, H5E_BADTYPE, FAIL, "can't tell if datatype is immutable") - - /* Don't share committed datatypes */ - /* JAMES: Quincey says this check isn't working! */ - if((tri_ret = H5T_committed((H5T_t*) mesg)) > 0) - HGOTO_DONE(FALSE) - else if(tri_ret < 0) - HGOTO_ERROR(H5E_OHDR, H5E_BADTYPE, FAIL, "can't tell if datatype is comitted") - } /* end if */ + /* Type-specific check */ + if((tri_ret = H5O_msg_can_share(type_id, mesg)) < 0) + HGOTO_ERROR(H5E_OHDR, H5E_BADTYPE, FAIL, "can_share callback returned error") + + if(tri_ret == FALSE) + HGOTO_DONE(FALSE); /* Look up the master SOHM table */ if (NULL == (table = (H5SM_master_table_t *)H5AC_protect(f, dxpl_id, H5AC_SOHM_TABLE, f->shared->sohm_addr, NULL, NULL, H5AC_WRITE))) @@ -901,6 +886,7 @@ H5SM_write_mesg(H5F_t *f, hid_t dxpl_id, H5SM_index_header_t *header, H5HF_t *fheap = NULL; /* Fractal heap handle */ size_t buf_size; /* Size of the encoded message */ void * encoding_buf=NULL; /* Buffer for encoded message */ + size_t empty_pos=UFAIL; /* Empty entry in list */ herr_t ret_value = SUCCEED; FUNC_ENTER_NOAPI(H5SM_write_mesg, FAIL) @@ -947,9 +933,11 @@ H5SM_write_mesg(H5F_t *f, hid_t dxpl_id, H5SM_index_header_t *header, if (NULL == (list = (H5SM_list_t *)H5AC_protect(f, dxpl_id, H5AC_SOHM_LIST, header->index_addr, NULL, header, H5AC_WRITE))) HGOTO_ERROR(H5E_CACHE, H5E_CANTPROTECT, FAIL, "unable to load SOHM index") - /* JAMES: not very efficient (gets hash value twice, searches list twice). Refactor. */ - /* See if the message is already in the index and get its location */ - list_pos = H5SM_find_in_list(list, &key); + /* See if the message is already in the index and get its location. + * Also record the first empty list position we find in case we need it + * later. + */ + list_pos = H5SM_find_in_list(list, &key, &empty_pos); if(list_pos != UFAIL) { /* The message was in the index. Increment its reference count. */ @@ -973,10 +961,6 @@ H5SM_write_mesg(H5F_t *f, hid_t dxpl_id, H5SM_index_header_t *header, /* If the message isn't in the list, add it */ if(!found) { - hsize_t x; /* Counter variable */ - - /* JAMES: wrap this in a function call? */ - /* Put the message in the heap and record its new heap ID */ if(H5HF_insert(fheap, dxpl_id, key.encoding_size, key.encoding, &shared.u.heap_id) < 0) HGOTO_ERROR(H5E_HEAP, H5E_CANTINSERT, FAIL, "unable to insert message into fractal heap") @@ -991,20 +975,19 @@ H5SM_write_mesg(H5F_t *f, hid_t dxpl_id, H5SM_index_header_t *header, } - /* JAMES: should be H5SM_insert or something */ - /* Find an empty spot in the list for the message JAMES: combine this with the previous traversal */ /* Insert the new message into the SOHM index */ if(header->index_type == H5SM_LIST) { - for(x = 0; x < header->list_max; x++) - { - if(list->messages[x].ref_count == 0) - { - list->messages[x] = key.message; - HDassert(list->messages[x].ref_count > 0); - break; - } + /* Index is a list. Find an empty spot if we haven't already */ + if(empty_pos == UFAIL) { + if((H5SM_find_in_list(list, NULL, &empty_pos) == UFAIL) || empty_pos == UFAIL) + HGOTO_ERROR(H5E_SOHM, H5E_CANTINSERT, FAIL, "unable to find empty entry in list") } + + /* Insert message into list */ + HDassert(list->messages[empty_pos].ref_count == 0); + list->messages[empty_pos] = key.message; + HDassert(list->messages[empty_pos].ref_count > 0); } else /* Index is a B-tree */ { @@ -1082,7 +1065,6 @@ H5SM_try_delete(H5F_t *f, hid_t dxpl_id, unsigned type_id, if((index_num = H5SM_get_index(table, type_id)) < 0) 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 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. @@ -1126,10 +1108,18 @@ done: /*------------------------------------------------------------------------- * Function: H5SM_find_in_list * - * Purpose: Find a message's location in a list + * Purpose: Find a message's location in a list. Also find the first + * empty location in the list (since if we don't find the + * message, we may want to insert it into an open spot). + * + * If KEY is NULL, simply find the first empty location in the + * list. + * + * If EMPTY_POS is NULL, don't store anything in it. * * Return: Message's position in the list on success * UFAIL if message couldn't be found + * empty_pos set to position of empty message or UFAIL. * * Programmer: James Laird * Tuesday, May 2, 2006 @@ -1137,7 +1127,7 @@ done: *------------------------------------------------------------------------- */ size_t -H5SM_find_in_list(H5SM_list_t *list, const H5SM_mesg_key_t *key) +H5SM_find_in_list(H5SM_list_t *list, const H5SM_mesg_key_t *key, size_t *empty_pos) { size_t x; size_t ret_value; @@ -1145,11 +1135,23 @@ H5SM_find_in_list(H5SM_list_t *list, const H5SM_mesg_key_t *key) FUNC_ENTER_NOAPI(H5SM_find_in_list, UFAIL) HDassert(list); - HDassert(key); + /* Both key and empty_pos can be NULL, but not both! */ + HDassert(key || empty_pos); - for(x = 0; x < list->header->list_max; x++) - if((list->messages[x].ref_count > 0) && 0 == H5SM_message_compare(key, &(list->messages[x]))) + /* Initialize empty_pos to an invalid value */ + if(empty_pos) + *empty_pos = UFAIL; + + /* Find the first (only) message equal to the key passed in. + * Also record the first empty position we find. + */ + for(x = 0; x < list->header->list_max; x++) { + if(key && (list->messages[x].ref_count > 0) && + (0 == H5SM_message_compare(key, &(list->messages[x])))) HGOTO_DONE(x) + else if(empty_pos && *empty_pos == UFAIL && list->messages[x].ref_count == 0) + *empty_pos = x; + } /* If we reached this point, we didn't find the message */ HGOTO_ERROR(H5E_SOHM, H5E_NOTFOUND, UFAIL, "message not in list") @@ -1254,7 +1256,7 @@ H5SM_delete_from_index(H5F_t *f, hid_t dxpl_id, H5SM_index_header_t *header, HGOTO_ERROR(H5E_SOHM, H5E_CANTPROTECT, FAIL, "unable to load SOHM index") /* Find the message in the list */ - if((list_pos = H5SM_find_in_list(list, &key)) == UFAIL) + if((list_pos = H5SM_find_in_list(list, &key, NULL)) == UFAIL) HGOTO_ERROR(H5E_SOHM, H5E_NOTFOUND, FAIL, "message not in index") --(list->messages[list_pos].ref_count); @@ -1606,7 +1608,7 @@ H5SM_get_refcount(H5F_t *f, hid_t dxpl_id, unsigned type_id, HGOTO_ERROR(H5E_SOHM, H5E_CANTPROTECT, FAIL, "unable to load SOHM index") /* Find the message in the list */ - if((list_pos = H5SM_find_in_list(list, &key)) == UFAIL) + if((list_pos = H5SM_find_in_list(list, &key, NULL)) == UFAIL) HGOTO_ERROR(H5E_SOHM, H5E_NOTFOUND, FAIL, "message not in index") /* Copy the message */ diff --git a/src/H5SMbtree2.c b/src/H5SMbtree2.c index 1b5cb98..b59304f 100755 --- a/src/H5SMbtree2.c +++ b/src/H5SMbtree2.c @@ -150,9 +150,9 @@ H5SM_message_compare(const void *rec1, const void *rec2) /* Compare either the heap_ids directly (if the key has one) * or the encoded buffers */ - /* JAMES: not a great test. Use a flag instead? */ if(key->encoding_size == 0) { + HDassert(key->encoding == NULL); ret_value = (herr_t) (key->message.fheap_id - mesg->fheap_id); } else @@ -160,7 +160,6 @@ H5SM_message_compare(const void *rec1, const void *rec2) /* 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; diff --git a/src/H5SMcache.c b/src/H5SMcache.c index 1031a5c..1447fc9 100644 --- a/src/H5SMcache.c +++ b/src/H5SMcache.c @@ -32,11 +32,6 @@ /****************/ /* Local Macros */ /****************/ -/* 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 */ /******************/ @@ -80,6 +75,9 @@ const H5AC_class_t H5AC_SOHM_LIST[1] = {{ (H5AC_size_func_t) H5SM_list_size, }}; +/* Declare a free list to manage data to/from disk */ +H5FL_BLK_DEFINE_STATIC(shared_mesg_cache); + /*****************************/ /* Library Private Variables */ /*****************************/ @@ -123,8 +121,8 @@ H5SM_flush_table(H5F_t *f, hid_t dxpl_id, hbool_t destroy, haddr_t addr, H5SM_ma /* Encode the master table and all of the index headers as one big blob */ size = H5SM_TABLE_SIZE(f) + (H5SM_INDEX_HEADER_SIZE(f) * table->num_indexes); - /* Allocate the buffer */ /* JAMES: use H5FL_BLK_MALLOC instead? */ - if(NULL == (buf = H5MM_malloc(size))) + /* Allocate the buffer */ + if(NULL == (buf = H5FL_BLK_MALLOC(shared_mesg_cache, size))) HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, FAIL, "memory allocation failed") /* Encode the master table */ @@ -167,7 +165,8 @@ H5SM_flush_table(H5F_t *f, hid_t dxpl_id, hbool_t destroy, haddr_t addr, H5SM_ma done: /* Free buffer if allocated */ - buf = H5MM_xfree(buf); + if(buf) + H5FL_BLK_FREE(shared_mesg_cache, buf); FUNC_LEAVE_NOAPI(ret_value) } /* end H5SM_flush_table */ @@ -217,8 +216,8 @@ H5SM_load_table(H5F_t *f, hid_t dxpl_id, haddr_t addr, const void UNUSED *udata1 */ table_size = H5SM_TABLE_SIZE(f) + (table->num_indexes * H5SM_INDEX_HEADER_SIZE(f)); - /* Allocate temporary buffer */ /* JAMES: FL_BLK? */ - if(NULL == (buf = H5MM_malloc(table_size))) + /* Allocate temporary buffer */ + if(NULL == (buf = H5FL_BLK_MALLOC(shared_mesg_cache, table_size))) HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed") /* Read header from disk */ @@ -273,7 +272,8 @@ H5SM_load_table(H5F_t *f, hid_t dxpl_id, haddr_t addr, const void UNUSED *udata1 done: /* Free buffer if allocated */ - buf = H5MM_xfree(buf); + if(buf) + H5FL_BLK_FREE(shared_mesg_cache, buf); FUNC_LEAVE_NOAPI(ret_value) } /* end H5SM_load_table */ @@ -411,9 +411,8 @@ H5SM_flush_list(H5F_t *f, hid_t dxpl_id, hbool_t destroy, haddr_t addr, H5SM_lis size = H5SM_LIST_SIZE(f, list->header->num_messages); /* Allocate temporary buffer */ - /* JAMES: is BLK_MALLOC somehow better for this? */ - if(NULL == (buf = H5MM_malloc(size))) - HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed") + if(NULL == (buf = H5FL_BLK_MALLOC(shared_mesg_cache, size))) + HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, FAIL, "memory allocation failed") /* Encode the list */ p = buf; @@ -454,7 +453,8 @@ H5SM_flush_list(H5F_t *f, hid_t dxpl_id, hbool_t destroy, haddr_t addr, H5SM_lis done: /* Free buffer if allocated */ - buf = H5MM_xfree(buf); + if(buf) + H5FL_BLK_FREE(shared_mesg_cache, buf); FUNC_LEAVE_NOAPI(ret_value) } /* end H5SM_flush_list */ @@ -490,7 +490,7 @@ H5SM_load_list(H5F_t *f, hid_t dxpl_id, haddr_t addr, const void UNUSED *udata1, HDassert(header); - /* Allocate space for the SOHM list data structure and initialize list JAMES don't need to initialize all of list */ + /* Allocate space for the SOHM list data structure */ if(NULL == (list = H5FL_MALLOC(H5SM_list_t))) HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed") HDmemset(&list->cache_info, 0, sizeof(H5AC_info_t)); @@ -505,8 +505,7 @@ H5SM_load_list(H5F_t *f, hid_t dxpl_id, haddr_t addr, const void UNUSED *udata1, size = H5SM_LIST_SIZE(f, header->num_messages); /* Allocate temporary buffer */ - /* JAMES: is BLK_MALLOC somehow better for this? */ - if(NULL == (buf = H5MM_malloc(size))) + if(NULL == (buf = H5FL_BLK_MALLOC(shared_mesg_cache, size))) HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed") /* Read list from disk */ @@ -551,7 +550,8 @@ H5SM_load_list(H5F_t *f, hid_t dxpl_id, haddr_t addr, const void UNUSED *udata1, ret_value = list; done: /* Free buffer if allocated */ - buf = H5MM_xfree(buf); + if(buf) + H5FL_BLK_FREE(shared_mesg_cache, buf); if(ret_value == NULL) { if(list) { @@ -653,7 +653,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_max); /* JAMES: might want to have variable-sized lists */ + *size_ptr = H5SM_LIST_SIZE(f, list->header->list_max); FUNC_LEAVE_NOAPI(SUCCEED) } /* end H5SM_list_size */ diff --git a/src/H5SMpkg.h b/src/H5SMpkg.h index 37a40a1..d17f6c5 100755 --- a/src/H5SMpkg.h +++ b/src/H5SMpkg.h @@ -177,7 +177,8 @@ H5_DLLVAR const H5B2_class_t H5SM_INDEX[1]; /* General routines */ H5_DLL ssize_t H5SM_get_index(const H5SM_master_table_t *table, unsigned type_id); -H5_DLL size_t H5SM_find_in_list(H5SM_list_t *list, const H5SM_mesg_key_t *key); +H5_DLL size_t H5SM_find_in_list(H5SM_list_t *list, + const H5SM_mesg_key_t *key, size_t *empty_pos); /* Encode and decode routines, used for B-tree and cache encoding/decoding */ H5_DLL herr_t H5SM_message_encode(const H5F_t *f, uint8_t *raw, diff --git a/src/H5Tcommit.c b/src/H5Tcommit.c index d8fbe3b..bc213ae 100644 --- a/src/H5Tcommit.c +++ b/src/H5Tcommit.c @@ -134,25 +134,22 @@ done: /* If the datatype was committed but couldn't be linked, we need to return it to the state it was in * before it was committed. */ if(TRUE == uncommit) { -#ifdef JAMES - /* JAMES: I'm not convinced that this really works anyway */ - if(type->shared->state == H5T_STATE_OPEN && type->sh_loc.flags & H5O_COMMITTED_FLAG)) { + if(type->shared->state == H5T_STATE_OPEN && type->sh_loc.flags & H5O_COMMITTED_FLAG) { /* Remove the datatype from the list of opened objects in the file */ - if(H5FO_top_decr(type->oloc.file, type->oloc.addr) < 0) + if(H5FO_top_decr(type->sh_loc.u.oloc.file, type->sh_loc.u.oloc.addr) < 0) HDONE_ERROR(H5E_DATASET, H5E_CANTRELEASE, FAIL, "can't decrement count for object") - if(H5FO_delete(type->oloc.file, H5AC_dxpl_id, type->oloc.addr) < 0) + if(H5FO_delete(type->sh_loc.u.oloc.file, H5AC_dxpl_id, type->sh_loc.u.oloc.addr) < 0) HDONE_ERROR(H5E_DATASET, H5E_CANTRELEASE, FAIL, "can't remove dataset from list of open objects") - if(H5O_close(&(type->oloc)) < 0) + if(H5O_close(&(type->sh_loc.u.oloc)) < 0) HDONE_ERROR(H5E_DATATYPE, H5E_CLOSEERROR, FAIL, "unable to release object header") - if(H5O_delete(file, H5AC_dxpl_id, type->oloc.addr) < 0) + if(H5O_delete(file, H5AC_dxpl_id, type->sh_loc.u.oloc.addr) < 0) HDONE_ERROR(H5E_DATATYPE, H5E_CANTDELETE, FAIL, "unable to delete object header") /* Mark datatype as being back in memory */ if(H5T_set_loc(type, file, H5T_LOC_MEMORY)) HDONE_ERROR(H5E_DATATYPE, H5E_CANTDELETE, FAIL, "unable to return datatype to memory") - type->oloc.addr = HADDR_UNDEF; + type->sh_loc.flags = H5O_NOT_SHARED; type->shared->state = old_state; } /* end if */ -#endif /* JAMES */ } /* end if */ FUNC_LEAVE_API(ret_value) diff --git a/test/tsohm.c b/test/tsohm.c index 4fe2d8b..e1a976c 100644 --- a/test/tsohm.c +++ b/test/tsohm.c @@ -142,6 +142,9 @@ typedef struct size2_helper_struct { #define HALF_DELETE_NUM_MESGS 3 #define DELETE_DIMS {1,1,1,1,1,1,1} +/* Number of dimensions in extend_dset test */ +#define EXTEND_NDIMS 2 + /* Helper function prototypes */ static hid_t make_dtype_1(void); static hid_t make_dtype_2(void); @@ -3053,13 +3056,17 @@ test_sohm_extlink(void) static void test_sohm_extend_dset_helper(hid_t fcpl_id) { hid_t file_id = -1; - hid_t space_id = -1; + hid_t orig_space_id = -1; + hid_t space1_id, space2_id; hid_t dcpl_id = -1; - hid_t dset_id = -1; + hid_t dset1_id, dset2_id; hsize_t dims1[] = {1, 2}; - hsize_t max_dims1[] = {H5S_UNLIMITED, 2}; + hsize_t max_dims[] = {H5S_UNLIMITED, 2}; hsize_t dims2[] = {5, 2}; + hsize_t out_dims[2]; + hsize_t out_maxdims[2]; long data[10] = {0}; + int x; herr_t ret; /* Create file */ @@ -3073,62 +3080,142 @@ static void test_sohm_extend_dset_helper(hid_t fcpl_id) CHECK_I(ret, "H5Pset_chunk"); /* Create a dataspace and a dataset*/ - space_id = H5Screate_simple(2, dims1, max_dims1); - CHECK_I(space_id, "H5Screate_simple"); - dset_id = H5Dcreate(file_id, "dataset", H5T_NATIVE_LONG, space_id, dcpl_id); - CHECK_I(dset_id, "H5Dcreate"); + orig_space_id = H5Screate_simple(EXTEND_NDIMS, dims1, max_dims); + CHECK_I(orig_space_id, "H5Screate_simple"); + dset1_id = H5Dcreate(file_id, "dataset", H5T_NATIVE_LONG, orig_space_id, dcpl_id); + CHECK_I(dset1_id, "H5Dcreate"); + + /* Create another dataset with the same dataspace */ + dset2_id = H5Dcreate(file_id, "dataset2", H5T_NATIVE_LONG, orig_space_id, dcpl_id); + CHECK_I(dset2_id, "H5Dcreate"); + + /* Extend the first dataset */ + ret = H5Dextend(dset1_id, dims2); + CHECK_I(ret, "H5Dextend"); + + /* Get the dataspaces from the datasets */ + space1_id = H5Dget_space(dset1_id); + CHECK_I(space1_id, "H5Dget_space"); + space2_id = H5Dget_space(dset2_id); + CHECK_I(space2_id, "H5Dget_space"); + + /* Verify the dataspaces */ + ret = H5Sget_simple_extent_dims(space1_id, out_dims, out_maxdims); + CHECK_I(ret, "H5Sget_simple_extent_dims"); + + for(x=0; x