From 5032b2e7df430fffd5941508cfc6276474f96718 Mon Sep 17 00:00:00 2001 From: Quincey Koziol Date: Mon, 23 Oct 2006 19:16:49 -0500 Subject: [svn-r12806] Description: Reduce memory usage for common cases of I/O pipeline filter memory usage. Also, clean up some more code... Tested on: Linux/32 2.6 (chicago) Linux/64 2.6 (chicago2) --- src/H5HFtest.c | 2 +- src/H5Opline.c | 97 +++++++++++++++++++++++++-------------- src/H5Z.c | 136 ++++++++++++++++++++++++++++++++++--------------------- src/H5Zprivate.h | 40 +++++++++++++--- src/H5Zpublic.h | 8 +++- test/dsets.c | 124 +++++++++++++++++++++++++------------------------- 6 files changed, 251 insertions(+), 156 deletions(-) diff --git a/src/H5HFtest.c b/src/H5HFtest.c index 61ad00d..d7f6f5e 100644 --- a/src/H5HFtest.c +++ b/src/H5HFtest.c @@ -205,7 +205,7 @@ HDfprintf(stderr, "%s: cparam2->pline.filter[%Zu].name = %s\n", "H5HF_cmp_cparam done: FUNC_LEAVE_NOAPI(ret_value) -} /* H5HF_get_cparam_test() */ +} /* H5HF_cmp_cparam_test() */ /*------------------------------------------------------------------------- diff --git a/src/H5Opline.c b/src/H5Opline.c index 86c8dd7..fefaa78 100644 --- a/src/H5Opline.c +++ b/src/H5Opline.c @@ -152,9 +152,22 @@ H5O_pline_decode(H5F_t UNUSED *f, hid_t UNUSED dxpl_id, const uint8_t *p) /* Filter name, if there is one */ if(name_length) { - filter->name = H5MM_malloc(name_length); - HDmemcpy(filter->name, p, name_length); - HDassert(filter->name[name_length - 1] == '\0'); + size_t actual_name_length; /* Actual length of name */ + + /* Determine actual name length (without padding, but with null terminator) */ + actual_name_length = HDstrlen((const char *)p) + 1; + HDassert(actual_name_length <= name_length); + + /* Allocate space for the filter name, or use the internal buffer */ + if(actual_name_length > H5Z_COMMON_NAME_LEN) { + filter->name = H5MM_malloc(actual_name_length); + if(NULL == filter->name) + HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed for filter name") + } /* end if */ + else + filter->name = filter->_name; + + HDstrcpy(filter->name, (const char *)p); p += name_length; } /* end if */ @@ -162,12 +175,18 @@ H5O_pline_decode(H5F_t UNUSED *f, hid_t UNUSED dxpl_id, const uint8_t *p) if(filter->cd_nelmts) { size_t j; /* Local index variable */ + /* Allocate space for the client data elements, or use the internal buffer */ + if(filter->cd_nelmts > H5Z_COMMON_CD_VALUES) { + filter->cd_values = H5MM_malloc(filter->cd_nelmts * sizeof(unsigned)); + if(NULL == filter->cd_values) + HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed for client data") + } /* end if */ + else + filter->cd_values = filter->_cd_values; + /* * Read the client data values and the padding */ - filter->cd_values = H5MM_malloc(filter->cd_nelmts * sizeof(unsigned)); - if(NULL == filter->cd_values) - HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed for client data") for(j = 0; j < filter->cd_nelmts; j++) UINT32DECODE(p, filter->cd_values[j]); if(version == H5O_PLINE_VERSION_1) @@ -181,14 +200,8 @@ H5O_pline_decode(H5F_t UNUSED *f, hid_t UNUSED dxpl_id, const uint8_t *p) done: if(NULL == ret_value && pline) { - if(pline->filter) { - for(i = 0; i < pline->nused; i++) { - H5MM_xfree(pline->filter[i].name); - H5MM_xfree(pline->filter[i].cd_values); - } /* end for */ - H5MM_xfree(pline->filter); - } /* end if */ - H5FL_FREE(H5O_pline_t,pline); + H5O_pline_reset(pline); + H5O_pline_free(pline); } /* end if */ FUNC_LEAVE_NOAPI(ret_value) @@ -346,16 +359,36 @@ H5O_pline_copy(const void *_src, void *_dst/*out*/, unsigned UNUSED update_flags dst->filter[i] = src->filter[i]; /* Filter name */ - if(src->filter[i].name) - if(NULL == (dst->filter[i].name = H5MM_xstrdup(src->filter[i].name))) - HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed") + if(src->filter[i].name) { + size_t namelen; /* Length of source filter name, including null terminator */ + + namelen = HDstrlen(src->filter[i].name) + 1; + + /* Allocate space for the filter name, or use the internal buffer */ + if(namelen > H5Z_COMMON_NAME_LEN) { + dst->filter[i].name = H5MM_malloc(namelen); + if(NULL == dst->filter[i].name) + HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed for filter name") + + /* Copy name */ + HDstrcpy(dst->filter[i].name, src->filter[i].name); + } /* end if */ + else + dst->filter[i].name = dst->filter[i]._name; + } /* end if */ /* Filter parameters */ if(src->filter[i].cd_nelmts > 0) { - if(NULL == (dst->filter[i].cd_values = H5MM_malloc(src->filter[i].cd_nelmts* sizeof(unsigned)))) - HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed") - HDmemcpy(dst->filter[i].cd_values, src->filter[i].cd_values, - src->filter[i].cd_nelmts * sizeof(unsigned)); + /* Allocate space for the client data elements, or use the internal buffer */ + if(src->filter[i].cd_nelmts > H5Z_COMMON_CD_VALUES) { + if(NULL == (dst->filter[i].cd_values = H5MM_malloc(src->filter[i].cd_nelmts* sizeof(unsigned)))) + HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed") + + HDmemcpy(dst->filter[i].cd_values, src->filter[i].cd_values, + src->filter[i].cd_nelmts * sizeof(unsigned)); + } /* end if */ + else + dst->filter[i].cd_values = dst->filter[i]._cd_values; } /* end if */ } /* end for */ } /* end if */ @@ -367,15 +400,9 @@ H5O_pline_copy(const void *_src, void *_dst/*out*/, unsigned UNUSED update_flags done: if(!ret_value && dst) { - if(dst->filter) { - for(i = 0; i < dst->nused; i++) { - H5MM_xfree(dst->filter[i].name); - H5MM_xfree(dst->filter[i].cd_values); - } /* end for */ - H5MM_xfree(dst->filter); - } /* end if */ + H5O_pline_reset(dst); if(!_dst) - H5FL_FREE(H5O_pline_t,dst); + H5O_pline_free(dst); } /* end if */ FUNC_LEAVE_NOAPI(ret_value) @@ -475,8 +502,14 @@ H5O_pline_reset(void *mesg) /* Free information for each filter */ for(i = 0; i < pline->nused; i++) { - H5MM_xfree(pline->filter[i].name); - H5MM_xfree(pline->filter[i].cd_values); + if(pline->filter[i].name && pline->filter[i].name != pline->filter[i]._name) + HDassert((HDstrlen(pline->filter[i].name) + 1) > H5Z_COMMON_NAME_LEN); + if(pline->filter[i].name != pline->filter[i]._name) + pline->filter[i].name = H5MM_xfree(pline->filter[i].name); + if(pline->filter[i].cd_values && pline->filter[i].cd_values != pline->filter[i]._cd_values) + HDassert(pline->filter[i].cd_nelmts > H5Z_COMMON_CD_VALUES); + if(pline->filter[i].cd_values != pline->filter[i]._cd_values) + pline->filter[i].cd_values = H5MM_xfree(pline->filter[i].cd_values); } /* end for */ /* Free filter array */ @@ -500,8 +533,6 @@ H5O_pline_reset(void *mesg) * Programmer: Quincey Koziol * Saturday, March 11, 2000 * - * Modifications: - * *------------------------------------------------------------------------- */ static herr_t diff --git a/src/H5Z.c b/src/H5Z.c index f11756b..b83377d 100644 --- a/src/H5Z.c +++ b/src/H5Z.c @@ -698,23 +698,22 @@ H5Z_modify(const H5O_pline_t *pline, H5Z_filter_t filter, unsigned flags, size_t cd_nelmts, const unsigned int cd_values[/*cd_nelmts*/]) { size_t idx; /* Index of filter in pipeline */ - size_t i; /* Local index variable */ - herr_t ret_value=SUCCEED; /* Return value */ + herr_t ret_value = SUCCEED; /* Return value */ FUNC_ENTER_NOAPI(H5Z_modify, FAIL) - assert(pline); - assert(filter>=0 && filter<=H5Z_FILTER_MAX); - assert(0==(flags & ~((unsigned)H5Z_FLAG_DEFMASK))); - assert(0==cd_nelmts || cd_values); + HDassert(pline); + HDassert(filter >= 0 && filter <= H5Z_FILTER_MAX); + HDassert(0 == (flags & ~((unsigned)H5Z_FLAG_DEFMASK))); + HDassert(0 == cd_nelmts || cd_values); /* Locate the filter in the pipeline */ - for(idx=0; idxnused; idx++) - if(pline->filter[idx].id==filter) + for(idx = 0; idx < pline->nused; idx++) + if(pline->filter[idx].id == filter) break; /* Check if the filter was not already in the pipeline */ - if(idx>pline->nused) + if(idx > pline->nused) HGOTO_ERROR(H5E_PLINE, H5E_NOTFOUND, FAIL, "filter not in pipeline") /* Change parameters for filter */ @@ -722,15 +721,24 @@ H5Z_modify(const H5O_pline_t *pline, H5Z_filter_t filter, unsigned flags, pline->filter[idx].cd_nelmts = cd_nelmts; /* Free any existing parameters */ - if(pline->filter[idx].cd_values!=NULL) + if(pline->filter[idx].cd_values != NULL && pline->filter[idx].cd_values != pline->filter[idx]._cd_values) H5MM_xfree(pline->filter[idx].cd_values); /* Set parameters */ - if (cd_nelmts>0) { - pline->filter[idx].cd_values = H5MM_malloc(cd_nelmts*sizeof(unsigned)); - if (NULL==pline->filter[idx].cd_values) - HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, FAIL, "memory allocation failed for filter parameters") - for (i=0; i 0) { + size_t i; /* Local index variable */ + + /* Allocate memory or point at internal buffer */ + if(cd_nelmts > H5Z_COMMON_CD_VALUES) { + pline->filter[idx].cd_values = H5MM_malloc(cd_nelmts * sizeof(unsigned)); + if(NULL == pline->filter[idx].cd_values) + HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, FAIL, "memory allocation failed for filter parameters") + } /* end if */ + else + pline->filter[idx].cd_values = pline->filter[idx]._cd_values; + + /* Copy client data values */ + for(i = 0; i < cd_nelmts; i++) pline->filter[idx].cd_values[i] = cd_values[i]; } /* end if */ else @@ -759,33 +767,34 @@ herr_t H5Z_append(H5O_pline_t *pline, H5Z_filter_t filter, unsigned flags, size_t cd_nelmts, const unsigned int cd_values[/*cd_nelmts*/]) { - size_t idx, i; - herr_t ret_value=SUCCEED; /* Return value */ + size_t idx; + herr_t ret_value = SUCCEED; /* Return value */ FUNC_ENTER_NOAPI(H5Z_append, FAIL) - assert(pline); - assert(filter>=0 && filter<=H5Z_FILTER_MAX); - assert(0==(flags & ~((unsigned)H5Z_FLAG_DEFMASK))); - assert(0==cd_nelmts || cd_values); + HDassert(pline); + HDassert(filter >= 0 && filter <= H5Z_FILTER_MAX); + HDassert(0 == (flags & ~((unsigned)H5Z_FLAG_DEFMASK))); + HDassert(0 == cd_nelmts || cd_values); /* * Check filter limit. We do it here for early warnings although we may * decide to relax this restriction in the future. */ - if (pline->nused>=H5Z_MAX_NFILTERS) + if(pline->nused >= H5Z_MAX_NFILTERS) HGOTO_ERROR(H5E_PLINE, H5E_CANTINIT, FAIL, "too many filters in pipeline") /* Allocate additional space in the pipeline if it's full */ - if (pline->nused>=pline->nalloc) { + if(pline->nused >= pline->nalloc) { H5O_pline_t x; - x.nalloc = MAX(H5Z_MAX_NFILTERS, 2*pline->nalloc); + + x.nalloc = MAX(H5Z_MAX_NFILTERS, 2 * pline->nalloc); x.filter = H5MM_realloc(pline->filter, x.nalloc*sizeof(x.filter[0])); - if (NULL==x.filter) + if(NULL == x.filter) HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, FAIL, "memory allocation failed for filter pipeline") pline->nalloc = x.nalloc; pline->filter = x.filter; - } + } /* end if */ /* Add the new filter to the pipeline */ idx = pline->nused; @@ -793,20 +802,30 @@ H5Z_append(H5O_pline_t *pline, H5Z_filter_t filter, unsigned flags, pline->filter[idx].flags = flags; pline->filter[idx].name = NULL; /*we'll pick it up later*/ pline->filter[idx].cd_nelmts = cd_nelmts; - if (cd_nelmts>0) { - pline->filter[idx].cd_values = H5MM_malloc(cd_nelmts*sizeof(unsigned)); - if (NULL==pline->filter[idx].cd_values) - HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, FAIL, "memory allocation failed for filter") - for (i=0; i 0) { + size_t i; /* Local index variable */ + + /* Allocate memory or point at internal buffer */ + if(cd_nelmts > H5Z_COMMON_CD_VALUES) { + pline->filter[idx].cd_values = H5MM_malloc(cd_nelmts * sizeof(unsigned)); + if(NULL == pline->filter[idx].cd_values) + HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, FAIL, "memory allocation failed for filter") + } /* end if */ + else + pline->filter[idx].cd_values = pline->filter[idx]._cd_values; + + /* Copy client data values */ + for(i = 0; i < cd_nelmts; i++) pline->filter[idx].cd_values[i] = cd_values[i]; - } else { + } /* end if */ + else pline->filter[idx].cd_values = NULL; - } + pline->nused++; done: FUNC_LEAVE_NOAPI(ret_value) -} +} /* end H5Z_append() */ /*------------------------------------------------------------------------- @@ -1129,58 +1148,71 @@ done: herr_t H5Z_delete(H5O_pline_t *pline, H5Z_filter_t filter) { - herr_t ret_value=SUCCEED; /* Return value */ + herr_t ret_value = SUCCEED; /* Return value */ FUNC_ENTER_NOAPI(H5Z_delete, FAIL) /* Check args */ - assert(pline); - assert(filter>=0 && filter<=H5Z_FILTER_MAX); + HDassert(pline); + HDassert(filter >= 0 && filter <= H5Z_FILTER_MAX); /* if the pipeline has no filters, just return */ if(pline->nused==0) HGOTO_DONE(SUCCEED) /* Delete all filters */ - if (H5Z_FILTER_ALL==filter) { - if(H5O_reset(H5O_PLINE_ID, pline)<0) + if(H5Z_FILTER_ALL == filter) { + if(H5O_reset(H5O_PLINE_ID, pline) < 0) HGOTO_ERROR(H5E_PLINE, H5E_CANTFREE, FAIL, "can't release pipeline info") } /* end if */ /* Delete filter */ else { size_t idx; /* Index of filter in pipeline */ - unsigned found=0; /* Indicate filter was found in pipeline */ + hbool_t found = FALSE; /* Indicate filter was found in pipeline */ /* Locate the filter in the pipeline */ - for(idx=0; idxnused; idx++) - if(pline->filter[idx].id==filter) { - found=1; + for(idx = 0; idx < pline->nused; idx++) + if(pline->filter[idx].id == filter) { + found = TRUE; break; - } + } /* end if */ /* filter was not found in the pipeline */ - if (!found) + if(!found) HGOTO_ERROR(H5E_PLINE, H5E_NOTFOUND, FAIL, "filter not in pipeline") /* Free information for deleted filter */ - H5MM_xfree(pline->filter[idx].name); - H5MM_xfree(pline->filter[idx].cd_values); + if(pline->filter[idx].name && pline->filter[idx].name != pline->filter[idx]._name) + HDassert((HDstrlen(pline->filter[idx].name) + 1) > H5Z_COMMON_NAME_LEN); + if(pline->filter[idx].name != pline->filter[idx]._name) + pline->filter[idx].name = H5MM_xfree(pline->filter[idx].name); + if(pline->filter[idx].cd_values && pline->filter[idx].cd_values != pline->filter[idx]._cd_values) + HDassert(pline->filter[idx].cd_nelmts > H5Z_COMMON_CD_VALUES); + if(pline->filter[idx].cd_values != pline->filter[idx]._cd_values) + pline->filter[idx].cd_values = H5MM_xfree(pline->filter[idx].cd_values); /* Remove filter from pipeline array */ - if((idx+1)nused) - HDmemcpy(&pline->filter[idx], &pline->filter[idx+1], - sizeof (H5Z_filter_info_t)*(pline->nused-(idx+1))); + if((idx + 1) < pline->nused) { + /* Copy filters down & fix up any client data value arrays using internal storage */ + for(; (idx + 1) < pline->nused; idx++) { + pline->filter[idx] = pline->filter[idx + 1]; + if(pline->filter[idx].name && (HDstrlen(pline->filter[idx].name) + 1) <= H5Z_COMMON_NAME_LEN) + pline->filter[idx].name = pline->filter[idx]._name; + if(pline->filter[idx].cd_nelmts <= H5Z_COMMON_CD_VALUES) + pline->filter[idx].cd_values = pline->filter[idx]._cd_values; + } /* end for */ + } /* end if */ /* Decrement number of used filters */ pline->nused--; /* Reset information for previous last filter in pipeline */ - HDmemset(&pline->filter[pline->nused], 0, sizeof (H5Z_filter_info_t)); + HDmemset(&pline->filter[pline->nused], 0, sizeof(H5Z_filter_info_t)); } /* end else */ done: FUNC_LEAVE_NOAPI(ret_value) -} +} /* end H5Z_delete() */ /*------------------------------------------------------------------------- * Function: H5Zget_filter_info diff --git a/src/H5Zprivate.h b/src/H5Zprivate.h index e13876a..822e95d 100644 --- a/src/H5Zprivate.h +++ b/src/H5Zprivate.h @@ -25,23 +25,49 @@ /* Private headers needed by this file */ #include "H5Tprivate.h" /* Datatypes */ +/**************************/ +/* Library Private Macros */ +/**************************/ + +/* Special parameters for szip compression */ +/* [These are aliases for the similar definitions in szlib.h, which we can't + * include directly due to the duplication of various symbols with the zlib.h + * header file] */ +#define H5_SZIP_LSB_OPTION_MASK 8 +#define H5_SZIP_MSB_OPTION_MASK 16 +#define H5_SZIP_RAW_OPTION_MASK 128 + +/* Common # of 'client data values' for filters */ +/* (avoids dynamic memory allocation in most cases) */ +#define H5Z_COMMON_CD_VALUES 4 + +/* Common size of filter name */ +/* (avoids dynamic memory allocation in most cases) */ +#define H5Z_COMMON_NAME_LEN 12 + +/****************************/ +/* Library Private Typedefs */ +/****************************/ + /* Structure to store information about each filter's parameters */ typedef struct { H5Z_filter_t id; /*filter identification number */ unsigned flags; /*defn and invocation flags */ + char _name[H5Z_COMMON_NAME_LEN]; /*internal filter name */ char *name; /*optional filter name */ size_t cd_nelmts; /*number of elements in cd_values[] */ + unsigned _cd_values[H5Z_COMMON_CD_VALUES]; /*internal client data values */ unsigned *cd_values; /*client data values */ } H5Z_filter_info_t; -/* Special parameters for szip compression */ -/* [These are aliases for the similar definitions in szlib.h, which we can't - * include directly due to the duplication of various symbols with the zlib.h - * header file] */ -#define H5_SZIP_LSB_OPTION_MASK 8 -#define H5_SZIP_MSB_OPTION_MASK 16 -#define H5_SZIP_RAW_OPTION_MASK 128 +/*****************************/ +/* Library-private Variables */ +/*****************************/ + +/***************************************/ +/* Library-private Function Prototypes */ +/***************************************/ struct H5O_pline_t; /*forward decl*/ /* Internal API routines */ diff --git a/src/H5Zpublic.h b/src/H5Zpublic.h index c632c65..9b2fb3e 100644 --- a/src/H5Zpublic.h +++ b/src/H5Zpublic.h @@ -42,7 +42,13 @@ typedef int H5Z_filter_t; /* General macros */ #define H5Z_FILTER_ALL 0 /* Symbol to remove all filters in H5Premove_filter */ -#define H5Z_MAX_NFILTERS 32 /* Maximum number of filters allowed in a pipeline (should probably be allowed to be an unlimited amount) */ +#define H5Z_MAX_NFILTERS 32 /* Maximum number of filters allowed in a pipeline */ + /* (should probably be allowed to be an + * unlimited amount, but currently each + * filter uses a bit in a 32-bit field, + * so the format would have to be + * changed to accomodate that) + */ /* Flags for filter definition (stored) */ #define H5Z_FLAG_DEFMASK 0x00ff /*definition flag mask */ diff --git a/test/dsets.c b/test/dsets.c index 8508b37..3dae7f0 100644 --- a/test/dsets.c +++ b/test/dsets.c @@ -5620,72 +5620,72 @@ error: static herr_t auxread_fdata(hid_t fid, const char *name) { - hid_t dset_id=-1; /* dataset ID */ - hid_t dcpl_id=-1; /* dataset creation property list ID */ - hid_t space_id=-1; /* space ID */ - hid_t ftype_id=-1; /* file data type ID */ - hid_t mtype_id=-1; /* memory data type ID */ - size_t msize; /* memory size of memory type */ - void *buf=NULL; /* data buffer */ - hsize_t nelmts; /* number of elements in dataset */ - int rank; /* rank of dataset */ - hsize_t dims[H5S_MAX_RANK];/* dimensions of dataset */ - int i; - - if ((dset_id=H5Dopen(fid,name))<0) - goto error; - if ((space_id=H5Dget_space(dset_id))<0) - goto error; - if ((ftype_id=H5Dget_type (dset_id))<0) - goto error; - if ((dcpl_id=H5Dget_create_plist(dset_id))<0) - goto error; - if ( (rank=H5Sget_simple_extent_ndims(space_id))<0) - goto error; - HDmemset(dims, 0, sizeof dims); - if ( H5Sget_simple_extent_dims(space_id,dims,NULL)<0) - goto error; - nelmts=1; - for (i=0; i