Skip to content

Commit

Permalink
Remove some H5T_copy calls that are now unnecessary (HDFGroup#4164)
Browse files Browse the repository at this point in the history 
Removes some datatype copying calls that are now unnecessary after
refactoring the datatype conversion code to use pointers internally
rather than IDs

Rewrites the enum conversion function so that it uses cached copies
of the source and destination datatypes in order to avoid modifying
the datatypes passed in

Adds a 'recursive' field to the datatype conversion context which
allows the conversion functions for members of a container datatype
to skip unnecessary repetitive conversion setup code

Changes internal datatype conversion callback functions so that the
source and destination datatype structure pointers are const

Removes some unused and unnecessary internal IDs registered with
H5I_register
  • Loading branch information
@jhendersonHDF @qkoziol
jhendersonHDF authored and qkoziol committed Mar 19, 2024
1 parent f5b174a commit a97708f
Show file tree
Hide file tree
Showing 23 changed files with 1,260 additions and 1,038 deletions.
46 changes: 9 additions & 37 deletions src/H5Aint.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -680,9 +680,7 @@ H5A__read(const H5A_t *attr, const H5T_t *mem_type, void *buf)
uint8_t *bkg_buf = NULL; /* background buffer */
hssize_t snelmts; /* elements in attribute */
size_t nelmts; /* elements in attribute*/
H5T_path_t *tpath = NULL; /* type conversion info */
H5T_t *src_type = NULL; /* temporary datatype */
H5T_t *dst_type = NULL; /* temporary datatype */
H5T_path_t *tpath = NULL; /* type conversion info */
size_t src_type_size; /* size of source type */
size_t dst_type_size; /* size of destination type */
size_t buf_size; /* desired buffer size */
Expand Down Expand Up @@ -722,11 +720,6 @@ H5A__read(const H5A_t *attr, const H5T_t *mem_type, void *buf)
if (!H5T_path_noop(tpath)) {
H5T_bkg_t need_bkg; /* Background buffer type */

if (NULL == (src_type = H5T_copy(attr->shared->dt, H5T_COPY_ALL)))
HGOTO_ERROR(H5E_ATTR, H5E_CANTCOPY, FAIL, "unable to copy attribute datatype");
if (NULL == (dst_type = H5T_copy(mem_type, H5T_COPY_ALL)))
HGOTO_ERROR(H5E_ATTR, H5E_CANTCOPY, FAIL, "unable to copy memory datatype");

/* Get the maximum buffer size needed and allocate it */
buf_size = nelmts * MAX(src_type_size, dst_type_size);
if (NULL == (tconv_buf = H5FL_BLK_MALLOC(attr_buf, buf_size)))
Expand All @@ -751,8 +744,8 @@ H5A__read(const H5A_t *attr, const H5T_t *mem_type, void *buf)
}

/* Perform datatype conversion. */
if (H5T_convert(tpath, src_type, dst_type, nelmts, (size_t)0, (size_t)0, tconv_buf, bkg_buf) <
0)
if (H5T_convert(tpath, attr->shared->dt, mem_type, nelmts, (size_t)0, (size_t)0, tconv_buf,
bkg_buf) < 0)
HGOTO_ERROR(H5E_ATTR, H5E_CANTCONVERT, FAIL, "datatype conversion failed");

/* Copy the converted data into the user's buffer */
Expand All @@ -770,10 +763,6 @@ H5A__read(const H5A_t *attr, const H5T_t *mem_type, void *buf)

done:
/* Release resources */
if (src_type && H5T_close(src_type) < 0)
HDONE_ERROR(H5E_ATTR, H5E_CANTCLOSEOBJ, FAIL, "unable to close temporary datatype");
if (dst_type && H5T_close(dst_type) < 0)
HDONE_ERROR(H5E_ATTR, H5E_CANTCLOSEOBJ, FAIL, "unable to close temporary datatype");
if (tconv_buf)
tconv_buf = H5FL_BLK_FREE(attr_buf, tconv_buf);
if (bkg_buf)
Expand Down Expand Up @@ -804,9 +793,7 @@ H5A__write(H5A_t *attr, const H5T_t *mem_type, const void *buf)
uint8_t *bkg_buf = NULL; /* temp conversion buffer */
hssize_t snelmts; /* elements in attribute */
size_t nelmts; /* elements in attribute */
H5T_path_t *tpath = NULL; /* conversion information*/
H5T_t *src_type = NULL; /* temporary datatype */
H5T_t *dst_type = NULL; /* temporary datatype */
H5T_path_t *tpath = NULL; /* conversion information*/
size_t src_type_size; /* size of source type */
size_t dst_type_size; /* size of destination type*/
size_t buf_size; /* desired buffer size */
Expand Down Expand Up @@ -842,11 +829,6 @@ H5A__write(H5A_t *attr, const H5T_t *mem_type, const void *buf)
if (!H5T_path_noop(tpath)) {
H5T_bkg_t need_bkg; /* Background buffer type */

if (NULL == (src_type = H5T_copy(mem_type, H5T_COPY_ALL)))
HGOTO_ERROR(H5E_ATTR, H5E_CANTCOPY, FAIL, "unable to copy memory datatype");
if (NULL == (dst_type = H5T_copy(attr->shared->dt, H5T_COPY_ALL)))
HGOTO_ERROR(H5E_ATTR, H5E_CANTCOPY, FAIL, "unable to copy attribute datatype");

/* Get the maximum buffer size needed and allocate it */
buf_size = nelmts * MAX(src_type_size, dst_type_size);
if (NULL == (tconv_buf = H5FL_BLK_MALLOC(attr_buf, buf_size)))
Expand Down Expand Up @@ -879,7 +861,8 @@ H5A__write(H5A_t *attr, const H5T_t *mem_type, const void *buf)
}

/* Perform datatype conversion */
if (H5T_convert(tpath, src_type, dst_type, nelmts, (size_t)0, (size_t)0, tconv_buf, bkg_buf) < 0)
if (H5T_convert(tpath, mem_type, attr->shared->dt, nelmts, (size_t)0, (size_t)0, tconv_buf,
bkg_buf) < 0)
HGOTO_ERROR(H5E_ATTR, H5E_CANTCONVERT, FAIL, "datatype conversion failed");

/* Free the previous attribute data buffer, if there is one */
Expand Down Expand Up @@ -910,10 +893,6 @@ H5A__write(H5A_t *attr, const H5T_t *mem_type, const void *buf)

done:
/* Release resources */
if (src_type && H5T_close(src_type) < 0)
HDONE_ERROR(H5E_ATTR, H5E_CANTCLOSEOBJ, FAIL, "unable to close temporary datatype");
if (dst_type && H5T_close(dst_type) < 0)
HDONE_ERROR(H5E_ATTR, H5E_CANTCLOSEOBJ, FAIL, "unable to close temporary datatype");
if (tconv_buf)
tconv_buf = H5FL_BLK_FREE(attr_buf, tconv_buf);
if (bkg_buf)
Expand Down Expand Up @@ -2083,10 +2062,10 @@ H5A__attr_copy_file(const H5A_t *attr_src, H5F_t *file_dst, bool *recompute_size
{
H5A_t *attr_dst = NULL; /* Destination attribute */
H5T_t *dt_mem = NULL; /* Memory datatype */
H5S_t *buf_space = NULL; /* Dataspace describing buffer */
void *buf = NULL; /* Buffer for copying data */
void *reclaim_buf = NULL; /* Buffer for reclaiming data */
void *bkg_buf = NULL; /* Background buffer */
hid_t buf_sid = -1; /* ID for buffer dataspace */
hssize_t sdst_nelmts; /* # of elements in destination attribute (signed) */
size_t dst_nelmts; /* # of elements in destination attribute */
size_t dst_dt_size; /* Size of destination attribute datatype */
Expand Down Expand Up @@ -2199,7 +2178,6 @@ H5A__attr_copy_file(const H5A_t *attr_src, H5F_t *file_dst, bool *recompute_size
size_t src_dt_size; /* Source datatype size */
size_t tmp_dt_size; /* Temp. datatype size */
size_t max_dt_size; /* Max atatype size */
H5S_t *buf_space; /* Dataspace describing buffer */
hsize_t buf_dim; /* Dimension for buffer */
size_t nelmts; /* Number of elements in buffer */
size_t buf_size; /* Size of copy buffer */
Expand Down Expand Up @@ -2240,12 +2218,6 @@ H5A__attr_copy_file(const H5A_t *attr_src, H5F_t *file_dst, bool *recompute_size
if (NULL == (buf_space = H5S_create_simple((unsigned)1, &buf_dim, NULL)))
HGOTO_ERROR(H5E_DATASPACE, H5E_CANTCREATE, NULL, "can't create simple dataspace");

/* Register */
if ((buf_sid = H5I_register(H5I_DATASPACE, buf_space, false)) < 0) {
H5S_close(buf_space);
HGOTO_ERROR(H5E_ID, H5E_CANTREGISTER, NULL, "unable to register dataspace ID");
} /* end if */

/* Allocate memory for recclaim buf */
if (NULL == (reclaim_buf = H5FL_BLK_MALLOC(attr_buf, buf_size)))
HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation NULLed for raw data chunk");
Expand Down Expand Up @@ -2303,10 +2275,10 @@ H5A__attr_copy_file(const H5A_t *attr_src, H5F_t *file_dst, bool *recompute_size
ret_value = attr_dst;

done:
if (buf_sid > 0 && H5I_dec_ref(buf_sid) < 0)
HDONE_ERROR(H5E_ATTR, H5E_CANTFREE, NULL, "can't decrement temporary dataspace ID");
if (dt_mem && (H5T_close(dt_mem) < 0))
HDONE_ERROR(H5E_ATTR, H5E_CANTCLOSEOBJ, NULL, "can't close temporary datatype");
if (buf_space && H5S_close(buf_space) < 0)
HDONE_ERROR(H5E_ATTR, H5E_CANTCLOSEOBJ, NULL, "can't close temporary dataspace");
if (buf)
buf = H5FL_BLK_FREE(attr_buf, buf);
if (reclaim_buf)
Expand Down
4 changes: 2 additions & 2 deletions src/H5D.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1853,7 +1853,7 @@ H5Dfill(const void *fill, hid_t fill_type_id, void *buf, hid_t buf_type_id, hid_
herr_t
H5Diterate(void *buf, hid_t type_id, hid_t space_id, H5D_operator_t op, void *operator_data)
{
H5T_t *type; /* Datatype */
const H5T_t *type; /* Datatype */
H5S_t *space; /* Dataspace for iteration */
H5S_sel_iter_op_t dset_op; /* Operator for iteration */
herr_t ret_value; /* Return value */
Expand All @@ -1868,7 +1868,7 @@ H5Diterate(void *buf, hid_t type_id, hid_t space_id, H5D_operator_t op, void *op
HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "invalid buffer");
if (H5I_DATATYPE != H5I_get_type(type_id))
HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "invalid datatype");
if (NULL == (type = (H5T_t *)H5I_object_verify(type_id, H5I_DATATYPE)))
if (NULL == (type = (const H5T_t *)H5I_object_verify(type_id, H5I_DATATYPE)))
HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "not an valid base datatype");
if (NULL == (space = (H5S_t *)H5I_object_verify(space_id, H5I_DATASPACE)))
HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "invalid dataspace");
Expand Down
Loading

0 comments on commit a97708f

Please sign in to comment.