From 3a0a5df0d2c92306e863abac1bd544a44f57da77 Mon Sep 17 00:00:00 2001 From: Quincey Koziol Date: Fri, 19 Nov 2004 09:57:30 -0500 Subject: [svn-r9553] Purpose: Code optimization Description: Avoid making as many copies of attribute information. Also, be smarter about which properties we've seen before when copying and closing property lists. Fix memory leak of attribute data structures. Platforms tested: FreeBSD 4.10 (sleipnir) w/parallel Solaris 2.7 (arabica) Too minor to require h5committest --- src/H5A.c | 35 +++++++++++++++-------------------- src/H5Oattr.c | 38 ++++++++++++++++++++++++++++++-------- src/H5P.c | 52 +++++++++++++++++++++++++++++++++++----------------- 3 files changed, 80 insertions(+), 45 deletions(-) diff --git a/src/H5A.c b/src/H5A.c index 668935d..abeb148 100644 --- a/src/H5A.c +++ b/src/H5A.c @@ -364,8 +364,7 @@ H5A_get_index(H5G_entry_t *ent, const char *name, hid_t dxpl_id) if(HDstrcmp(found_attr.name,name)==0) { if(H5O_reset (H5O_ATTR_ID, &found_attr)<0) HGOTO_ERROR(H5E_ATTR, H5E_CANTFREE, FAIL, "can't release attribute info") - ret_value = i; - break; + HGOTO_DONE(i); } if(H5O_reset (H5O_ATTR_ID, &found_attr)<0) HGOTO_ERROR(H5E_ATTR, H5E_CANTFREE, FAIL, "can't release attribute info") @@ -524,7 +523,7 @@ H5A_open(H5G_entry_t *ent, unsigned idx, hid_t dxpl_id) /* Read in attribute with H5O_read() */ H5_CHECK_OVERFLOW(idx,unsigned,int); - if (NULL==(attr=H5O_read(ent, H5O_ATTR_ID, (int)idx, attr, dxpl_id))) + if (NULL==(attr=H5O_read(ent, H5O_ATTR_ID, (int)idx, NULL, dxpl_id))) HGOTO_ERROR(H5E_ATTR, H5E_CANTINIT, FAIL, "unable to load attribute info from dataset header") attr->initialized=1; @@ -1230,7 +1229,7 @@ static herr_t H5A_rename(H5G_entry_t *ent, const char *old_name, const char *new_name, hid_t dxpl_id) { int seq, idx=FAIL; /* Index of attribute being querried */ - H5A_t *found_attr; /* Attribute with OLD_NAME */ + H5A_t found_attr; /* Attribute with OLD_NAME */ herr_t ret_value=SUCCEED; /* Return value */ FUNC_ENTER_NOAPI_NOINIT(H5A_rename) @@ -1240,45 +1239,41 @@ H5A_rename(H5G_entry_t *ent, const char *old_name, const char *new_name, hid_t d assert(old_name); assert(new_name); - /* Build the attribute information */ - if((found_attr = H5FL_CALLOC(H5A_t))==NULL) - HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, FAIL, "memory allocation failed for attribute info") - /* Read in the existing attributes to check for duplicates */ seq=0; - while(H5O_read(ent, H5O_ATTR_ID, seq, found_attr, dxpl_id)!=NULL) { + while(H5O_read(ent, H5O_ATTR_ID, seq, &found_attr, dxpl_id)!=NULL) { /* * Compare found attribute name. */ - if(HDstrcmp(found_attr->name,old_name)==0) { + if(HDstrcmp(found_attr.name,old_name)==0) { idx = seq; break; } - if(H5O_reset (H5O_ATTR_ID, found_attr)<0) + if(H5O_reset (H5O_ATTR_ID, &found_attr)<0) HGOTO_ERROR(H5E_ATTR, H5E_CANTFREE, FAIL, "can't release attribute info") seq++; } - + H5E_clear (); if(idx<0) HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "attribute cannot be found") /* Copy the attribute name. */ - if(found_attr->name) - HDfree(found_attr->name); - found_attr->name = HDstrdup(new_name); - if(!found_attr->name) + if(found_attr.name) + HDfree(found_attr.name); + found_attr.name = HDstrdup(new_name); + if(!found_attr.name) HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, FAIL, "String copy failed") /* Indicate entry is not opened and the attribute doesn't need fill-values. */ - found_attr->ent_opened=FALSE; - found_attr->initialized=TRUE; + found_attr.ent_opened=FALSE; + found_attr.initialized=TRUE; /* Modify the attribute message */ - if (H5O_modify(ent, H5O_ATTR_ID, idx, 0, 1, found_attr, dxpl_id) < 0) + if (H5O_modify(ent, H5O_ATTR_ID, idx, 0, 1, &found_attr, dxpl_id) < 0) HGOTO_ERROR(H5E_ATTR, H5E_CANTINIT, FAIL, "unable to update attribute header messages") /* Close the attribute */ - if(H5A_close(found_attr)<0) + if(H5A_free(&found_attr)<0) HGOTO_ERROR(H5E_ATTR, H5E_CANTRELEASE, FAIL, "unable to close renamed attribute") done: diff --git a/src/H5Oattr.c b/src/H5Oattr.c index e647743..bb22f70 100644 --- a/src/H5Oattr.c +++ b/src/H5Oattr.c @@ -35,6 +35,7 @@ static void *H5O_attr_decode (H5F_t *f, hid_t dxpl_id, const uint8_t *p, H5O_sha static void *H5O_attr_copy (const void *_mesg, void *_dest); static size_t H5O_attr_size (H5F_t *f, const void *_mesg); static herr_t H5O_attr_reset (void *_mesg); +static herr_t H5O_attr_free (void *mesg); static herr_t H5O_attr_delete (H5F_t *f, hid_t dxpl_id, const void *_mesg); static herr_t H5O_attr_link(H5F_t *f, hid_t dxpl_id, const void *_mesg); static herr_t H5O_attr_debug (H5F_t *f, hid_t dxpl_id, const void *_mesg, @@ -50,7 +51,7 @@ const H5O_class_t H5O_ATTR[1] = {{ H5O_attr_copy, /* copy the native value */ H5O_attr_size, /* size of raw message */ H5O_attr_reset, /* reset method */ - NULL, /* free method */ + H5O_attr_free, /* free method */ H5O_attr_delete, /* file delete method */ H5O_attr_link, /* link method */ NULL, /* get share method */ @@ -375,7 +376,6 @@ static void * H5O_attr_copy(const void *_src, void *_dst) { const H5A_t *src = (const H5A_t *) _src; - H5A_t *dst = NULL; void *ret_value; /* Return value */ FUNC_ENTER_NOAPI_NOINIT(H5O_attr_copy); @@ -384,12 +384,9 @@ H5O_attr_copy(const void *_src, void *_dst) assert(src); /* copy */ - if (NULL == (dst = H5A_copy(_dst,src))) + if (NULL == (ret_value = H5A_copy(_dst,src))) HGOTO_ERROR(H5E_ATTR, H5E_CANTINIT, NULL, "can't copy attribute"); - /* Set return value */ - ret_value=dst; - done: FUNC_LEAVE_NOAPI(ret_value); } @@ -486,7 +483,6 @@ static herr_t H5O_attr_reset(void *_mesg) { H5A_t *attr = (H5A_t *) _mesg; - H5A_t *tmp = NULL; herr_t ret_value=SUCCEED; /* Return value */ FUNC_ENTER_NOAPI_NOINIT(H5O_attr_reset); @@ -494,12 +490,38 @@ H5O_attr_reset(void *_mesg) if (attr) H5A_free(attr); -done: FUNC_LEAVE_NOAPI(ret_value); } /*------------------------------------------------------------------------- + * Function: H5O_attr_free + * + * Purpose: Free's the message + * + * Return: Non-negative on success/Negative on failure + * + * Programmer: Quincey Koziol + * Thursday, November 18, 2004 + * + * Modifications: + * + *------------------------------------------------------------------------- + */ +static herr_t +H5O_attr_free (void *mesg) +{ + FUNC_ENTER_NOAPI_NOINIT_NOFUNC(H5O_attr_free); + + assert (mesg); + + H5FL_FREE(H5A_t,mesg); + + FUNC_LEAVE_NOAPI(SUCCEED); +} /* end H5O_attr_free() */ + + +/*------------------------------------------------------------------------- * Function: H5O_attr_delete * * Purpose: Free file space referenced by message diff --git a/src/H5P.c b/src/H5P.c index a56e37b..f5a5ccd 100644 --- a/src/H5P.c +++ b/src/H5P.c @@ -19,6 +19,10 @@ #define H5P_PACKAGE /*suppress error about including H5Ppkg */ +/* Pablo information */ +/* (Put before include files to avoid problems with inline functions) */ +#define PABLO_MASK H5P_mask + /* Private header files */ #include "H5private.h" /* Generic Functions */ #include "H5Dprivate.h" /* Datasets */ @@ -29,9 +33,6 @@ #include "H5MMprivate.h" /* Memory management */ #include "H5Ppkg.h" /* Property lists */ -/* Pablo mask */ -#define PABLO_MASK H5P_mask - /* Interface initialization */ static int interface_initialize_g = 0; #define INTERFACE_INIT H5P_init_interface @@ -213,10 +214,6 @@ H5P_init_interface(void) FUNC_ENTER_NOAPI_NOINIT(H5P_init_interface); - /* Make certain IDs are initialized */ - if (ret_value < 0) - HGOTO_ERROR(H5E_ATOM, H5E_CANTINIT, FAIL, "unable to initialize atom group"); - /* * Initialize the Generic Property class & object groups. */ @@ -472,6 +469,8 @@ H5P_copy_plist(H5P_genplist_t *old_plist) hid_t new_plist_id; /* Property list ID of new list created */ H5TB_NODE *curr_node; /* Current node in TBBT */ H5TB_TREE *seen=NULL; /* TBBT containing properties already seen */ + size_t nseen; /* Number of items 'seen' */ + hbool_t has_parent_class; /* Flag to indicate that this property list's class has a parent */ hid_t ret_value=FAIL; /* return value */ FUNC_ENTER_NOAPI(H5P_copy_plist, FAIL); @@ -506,6 +505,7 @@ H5P_copy_plist(H5P_genplist_t *old_plist) */ if((seen=H5TB_fast_dmake(H5TB_FAST_STR_COMPARE))==NULL) HGOTO_ERROR(H5E_PLIST,H5E_CANTMAKETREE,FAIL,"can't create TBBT for seen properties"); + nseen=0; /* Cycle through the deleted properties & copy them into the new list's deleted section */ if(old_plist->del->root) { @@ -524,6 +524,7 @@ H5P_copy_plist(H5P_genplist_t *old_plist) /* Add property name to "seen" list */ if(H5TB_dins(seen,new_name,new_name)==NULL) HGOTO_ERROR(H5E_PLIST,H5E_CANTINSERT,FAIL,"can't insert property into seen TBBT"); + nseen++; /* Get the next property node in the TBBT */ curr_node=H5TB_next(curr_node); @@ -558,6 +559,7 @@ H5P_copy_plist(H5P_genplist_t *old_plist) /* Add property name to "seen" list */ if(H5TB_dins(seen,new_prop->name,new_prop->name)==NULL) HGOTO_ERROR(H5E_PLIST,H5E_CANTINSERT,FAIL,"can't insert property into seen TBBT"); + nseen++; /* Increment the number of properties in list */ new_plist->nprops++; @@ -572,6 +574,7 @@ H5P_copy_plist(H5P_genplist_t *old_plist) * initialize each with default value & make property 'copy' callback. */ tclass=old_plist->pclass; + has_parent_class=(tclass!=NULL && tclass->parent!=NULL && tclass->parent->nprops>0); while(tclass!=NULL) { if(tclass->nprops>0) { /* Walk through the properties in the old class */ @@ -581,7 +584,7 @@ H5P_copy_plist(H5P_genplist_t *old_plist) tmp=curr_node->data; /* Only "copy" properties we haven't seen before */ - if(H5TB_dfind(seen,tmp->name,NULL)==NULL) { + if(nseen==0 || H5TB_dfind(seen,tmp->name,NULL)==NULL) { /* Call property creation callback, if it exists */ if(tmp->copy) { /* Call the callback & insert changed value into tree (if necessary) */ @@ -589,9 +592,12 @@ H5P_copy_plist(H5P_genplist_t *old_plist) HGOTO_ERROR (H5E_PLIST, H5E_CANTCOPY, FAIL,"Can't create property"); } /* end if */ - /* Add property name to "seen" list */ - if(H5TB_dins(seen,tmp->name,tmp->name)==NULL) - HGOTO_ERROR(H5E_PLIST,H5E_CANTINSERT,FAIL,"can't insert property into seen TBBT"); + /* Add property name to "seen" list, if we have other classes to work on */ + if(has_parent_class) { + if(H5TB_dins(seen,tmp->name,tmp->name)==NULL) + HGOTO_ERROR(H5E_PLIST,H5E_CANTINSERT,FAIL,"can't insert property into seen TBBT"); + nseen++; + } /* end if */ /* Increment the number of properties in list */ new_plist->nprops++; @@ -5122,6 +5128,9 @@ H5P_close(void *_plist) H5P_genclass_t *tclass; /* Temporary class pointer */ H5P_genplist_t *plist=(H5P_genplist_t *)_plist; H5TB_TREE *seen=NULL; /* TBBT to hold names of properties already seen */ + size_t nseen; /* Number of items 'seen' */ + hbool_t has_parent_class; /* Flag to indicate that this property list's class has a parent */ + ssize_t ndel; /* Number of items deleted */ H5TB_NODE *curr_node; /* Current node in TBBT */ H5P_genprop_t *tmp; /* Temporary pointer to properties */ herr_t ret_value=SUCCEED; /* return value */ @@ -5143,6 +5152,7 @@ H5P_close(void *_plist) */ if((seen=H5TB_fast_dmake(H5TB_FAST_STR_COMPARE))==NULL) HGOTO_ERROR(H5E_PLIST,H5E_CANTMAKETREE,FAIL,"can't create TBBT for seen properties"); + nseen=0; /* Walk through the changed properties in the list */ if(plist->props->root) { @@ -5160,17 +5170,23 @@ H5P_close(void *_plist) /* Add property name to "seen" list */ if(H5TB_dins(seen,tmp->name,tmp->name)==NULL) HGOTO_ERROR(H5E_PLIST,H5E_CANTINSERT,FAIL,"can't insert property into seen TBBT"); + nseen++; /* Get the next property node in the TBBT */ curr_node=H5TB_next(curr_node); } /* end while */ } /* end if */ + /* Determine number of deleted items from property list */ + if((ndel=H5TB_count(plist->del))<0) + HGOTO_ERROR(H5E_PLIST,H5E_CANTCOUNT,FAIL,"can't deterimine # of items in TBBT"); + /* * Check if we should remove class properties (up through list of parent classes also), * initialize each with default value & make property 'remove' callback. */ tclass=plist->pclass; + has_parent_class=(tclass!=NULL && tclass->parent!=NULL && tclass->parent->nprops>0); while(tclass!=NULL) { if(tclass->nprops>0) { /* Walk through the properties in the class */ @@ -5182,8 +5198,8 @@ H5P_close(void *_plist) /* Only "delete" properties we haven't seen before * and that haven't already been deleted */ - if(H5TB_dfind(seen,tmp->name,NULL)==NULL && - H5TB_dfind(plist->del,tmp->name,NULL)==NULL) { + if((nseen==0 || H5TB_dfind(seen,tmp->name,NULL)==NULL) && + (ndel==0 || H5TB_dfind(plist->del,tmp->name,NULL)==NULL)) { /* Call property close callback, if it exists */ if(tmp->close) { @@ -5201,9 +5217,12 @@ H5P_close(void *_plist) H5MM_xfree(tmp_value); } /* end if */ - /* Add property name to "seen" list */ - if(H5TB_dins(seen,tmp->name,tmp->name)==NULL) - HGOTO_ERROR(H5E_PLIST,H5E_CANTINSERT,FAIL,"can't insert property into seen TBBT"); + /* Add property name to "seen" list, if we have other classes to work on */ + if(has_parent_class) { + if(H5TB_dins(seen,tmp->name,tmp->name)==NULL) + HGOTO_ERROR(H5E_PLIST,H5E_CANTINSERT,FAIL,"can't insert property into seen TBBT"); + nseen++; + } /* end if */ } /* end if */ /* Get the next property node in the TBBT */ @@ -5669,4 +5688,3 @@ H5Pclose_class(hid_t cls_id) done: FUNC_LEAVE_API(ret_value); } /* H5Pclose_class() */ - -- cgit v0.12