Skip to content

Commit

Permalink
Use H5Treclaim for vlen cleanup
Browse files Browse the repository at this point in the history
  • Loading branch information
@mattjala
mattjala committed May 9, 2024
1 parent 4f340e7 commit 9b5540b
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 30 deletions.
38 changes: 27 additions & 11 deletions src/rest_vol.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3656,6 +3656,7 @@ RV_curl_multi_perform(CURL *curl_multi_handle, dataset_transfer_info *transfer_i
int maxfd = -1;
long timeout_ms = 0;
struct timeval timeout;
hid_t vlen_buf_space = H5I_INVALID_HID;

if ((failed_handles_to_retry = calloc(count, sizeof(CURL *))) == NULL)
FUNC_GOTO_ERROR(H5E_DATASET, H5E_CANTALLOC, FAIL,
Expand Down Expand Up @@ -3797,12 +3798,15 @@ RV_curl_multi_perform(CURL *curl_multi_handle, dataset_transfer_info *transfer_i

if (transfer_info[handle_index].transfer_type == WRITE) {
if (transfer_info[handle_index].tconv_buf) {
if ((dtype_class = H5Tget_class(transfer_info[handle_index].mem_type_id)) ==
H5T_NO_CLASS)
FUNC_DONE_ERROR(H5E_DATASET, H5E_CANTGET, FAIL, "can't get mem dtype class");
htri_t has_vlen = FALSE;

if ((has_vlen =
H5Tdetect_class(transfer_info[handle_index].mem_type_id, H5T_VLEN)) < 0)
FUNC_DONE_ERROR(H5E_DATASET, H5E_CANTGET, FAIL,
"can't check if dtype contains vlen");

/* Clean up memory allocated by type conversion of vlen types */
if (dtype_class == H5T_VLEN) {
if (has_vlen > 0) {
/* Buffer was gathered before type conversion, so we can manually free vlen
* memory by iteration */
hssize_t num_elems = 0;
Expand All @@ -3811,13 +3815,21 @@ RV_curl_multi_perform(CURL *curl_multi_handle, dataset_transfer_info *transfer_i
FUNC_DONE_ERROR(H5E_DATASET, H5E_CANTGET, FAIL,
"can't get number of elements in dataspace");

hvl_t *vlen_buf = (hvl_t *)transfer_info[handle_index].tconv_buf;
for (size_t i = 0; i < (size_t)num_elems; i++) {
if (vlen_buf[i].p) {
RV_free(vlen_buf[i].p);
vlen_buf[i].p = NULL;
}
}
/* Vlen buffer is packed, so generate a 1D dataspace to describe its layout */
if ((vlen_buf_space = H5Screate_simple(1, &num_elems, NULL)) < 0)
FUNC_DONE_ERROR(H5E_DATASPACE, H5E_CANTCREATE, FAIL,
"can't create dataspace for vlen buffer");

if ((H5Treclaim(transfer_info[handle_index].mem_type_id, vlen_buf_space,
H5P_DEFAULT, transfer_info[handle_index].tconv_buf)) < 0)
FUNC_DONE_ERROR(H5E_DATASET, H5E_CANTFREE, FAIL,
"can't free vlen data from buffer");

if (H5Sclose(vlen_buf_space) < 0)
FUNC_DONE_ERROR(H5E_DATASPACE, H5E_CANTCLOSEOBJ, FAIL,
"can't close dataspace for vlen buffer");

vlen_buf_space = H5I_INVALID_HID;
}
}

Expand Down Expand Up @@ -3889,6 +3901,10 @@ RV_curl_multi_perform(CURL *curl_multi_handle, dataset_transfer_info *transfer_i
done:
RV_free(failed_handles_to_retry);

if (vlen_buf_space != H5I_INVALID_HID)
if (H5Sclose(vlen_buf_space) < 0)
FUNC_DONE_ERROR(H5E_DATASPACE, H5E_CANTCLOSEOBJ, FAIL, "can't close dataspace for vlen buffer");

return ret_value;
}

Expand Down
62 changes: 44 additions & 18 deletions src/rest_vol_dataset.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1351,17 +1351,32 @@ RV_dataset_write(size_t count, void *dset[], hid_t mem_type_id[], hid_t _mem_spa

if (transfer_info[i].tconv_buf) {
/* Clean up memory allocated by type conversion of vlen types */
if ((dtype_class = H5Tget_class(transfer_info[i].mem_type_id)) == H5T_NO_CLASS)
FUNC_DONE_ERROR(H5E_DATASET, H5E_CANTGET, FAIL, "can't get mem dtype class");
htri_t contains_vlen = FALSE;

if (dtype_class == H5T_VLEN) {
/* Type conversion buffer is packed, so free allocated buffers manually */
for (size_t j = 0; j < (size_t)file_select_npoints; j++) {
hvl_t vl = ((hvl_t *)transfer_info[i].tconv_buf)[j];
if (contains_vlen = H5Tdetect_class(transfer_info[i].file_type_id, H5T_VLEN) < 0)
FUNC_DONE_ERROR(H5E_DATASET, H5E_CANTGET, FAIL,
"can't determine if datatype contains Vlen type");

if (vl.p)
RV_free(vl.p);
}
if (contains_vlen > 0) {
hid_t type_conv_space = H5I_INVALID_HID;

/* Type conversion buffer is packed, so create a 1D dataspace to describe its layout */
if (mem_select_npoints = H5Sget_select_npoints(transfer_info[i].mem_space_id) < 0)
FUNC_DONE_ERROR(H5E_DATASPACE, H5E_CANTGET, FAIL,
"can't get number of elements in file space");

if ((type_conv_space = H5Screate_simple(1, &mem_select_npoints, NULL)) < 0)
FUNC_DONE_ERROR(H5E_DATASPACE, H5E_CANTCREATE, FAIL,
"can't create simple dataspace for type conversion buffer");

if ((H5Treclaim(transfer_info[i].mem_type_id, type_conv_space, H5P_DEFAULT,
transfer_info[i].tconv_buf)) < 0)
FUNC_DONE_ERROR(H5E_DATASET, H5E_CANTFREE, FAIL,
"can't free type conversion buffer for vlen type");

if ((H5Sclose(type_conv_space)) < 0)
FUNC_DONE_ERROR(H5E_DATASPACE, H5E_CANTCLOSEOBJ, FAIL,
"can't close dataspace for type conversion buffer");
}

RV_free(transfer_info[i].tconv_buf);
Expand Down Expand Up @@ -4753,15 +4768,26 @@ RV_dataset_read_cb(hid_t mem_type_id, hid_t mem_space_id, hid_t file_type_id, hi
RV_free(obj_ref_buf);

if (tconv_buf) {
if (H5T_VLEN == dtype_class && vlen_buf) {
/* Returned buffer is densely packed and not laid out according to the filespace; free vlen data
* through iteration */
for (size_t i = 0; i < file_select_npoints; i++) {
hvl_t vl = ((hvl_t *)vlen_buf)[i];

if (vl.p)
RV_free(vl.p);
}
htri_t contains_vlen = FALSE;

if ((contains_vlen = H5Tdetect_class(mem_type_id, H5T_VLEN)) < 0)
FUNC_DONE_ERROR(H5E_DATASET, H5E_CANTGET, FAIL, "can't determine if datatype contains Vlen type");

if (contains_vlen > 0) {
hid_t type_conv_space = H5I_INVALID_HID;

/* Vlen buffer is packed, so create a 1D dataspace to describe its layout */
if ((type_conv_space = H5Screate_simple(1, &file_select_npoints, NULL)) < 0)
FUNC_DONE_ERROR(H5E_DATASPACE, H5E_CANTCREATE, FAIL,
"can't create simple dataspace for unpacked vlen buffer");

if ((H5Treclaim(mem_type_id, type_conv_space, H5P_DEFAULT, vlen_buf)) < 0)
FUNC_DONE_ERROR(H5E_DATASET, H5E_CANTFREE, FAIL,
"can't free type conversion buffer for vlen type");

if (H5Sclose(type_conv_space) < 0)
FUNC_DONE_ERROR(H5E_DATASPACE, H5E_CANTCLOSEOBJ, FAIL,
"can't close dataspace for type conversion buffer");
}
RV_free(tconv_buf);
}
Expand Down
2 changes: 1 addition & 1 deletion src/rest_vol_datatype.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2122,7 +2122,7 @@ RV_convert_JSON_to_datatype(const char *type)
printf("-> Variable-length Datatype\n");
#endif

// allocate temporary buffer for parent substring
/* Allocate temporary buffer for parent substring */
tmp_vlen_type_buffer_size = DATATYPE_BODY_DEFAULT_SIZE;

if (NULL == (tmp_vlen_type_buffer = (char *)RV_malloc(tmp_vlen_type_buffer_size)))
Expand Down

0 comments on commit 9b5540b

Please sign in to comment.