Skip to content

Delay dataset layout copy to DCPL #5537

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from
Open

Delay dataset layout copy to DCPL #5537

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0df33c5
Delay dataset layout copy after open
mattjala May 14, 2025
aca6d0d
Test delayed DCPL layout copy
mattjala May 14, 2025
668ca57
Committing clang-format changes
github-actions[bot] May 14, 2025
6f82d62
Flush layout to DCPL before filter routines
mattjala May 14, 2025
da3b16f
Move definition of layout flush helper
mattjala May 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/H5Dchunk.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -764,9 +764,6 @@ H5D__chunk_set_sizes(H5D_t *dset)
/* Sanity checks */
assert(dset);

/* Increment # of chunk dimensions, to account for datatype size as last element */
dset->shared->layout.u.chunk.ndims++;

/* Set the last dimension of the chunk size to the size of the datatype */
dset->shared->layout.u.chunk.dim[dset->shared->layout.u.chunk.ndims - 1] =
(uint32_t)H5T_GET_SIZE(dset->shared->type);
Expand Down Expand Up @@ -830,6 +827,9 @@ H5D__chunk_construct(H5F_t H5_ATTR_UNUSED *f, H5D_t *dset)
if (dset->shared->layout.u.chunk.ndims != dset->shared->ndims)
HGOTO_ERROR(H5E_DATASET, H5E_BADVALUE, FAIL, "dimensionality of chunks doesn't match the dataspace");

/* Increment # of chunk dimensions, to account for datatype size as last element */
dset->shared->layout.u.chunk.ndims++;

/* Set chunk sizes */
if (H5D__chunk_set_sizes(dset) < 0)
HGOTO_ERROR(H5E_DATASET, H5E_BADVALUE, FAIL, "unable to set chunk sizes");
Expand Down
74 changes: 74 additions & 0 deletions src/H5Dint.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,8 @@ H5D__init_package(void)
H5D_def_dset.type_id = H5I_INVALID_HID;
H5D_def_dset.dapl_id = H5I_INVALID_HID;
H5D_def_dset.dcpl_id = H5I_INVALID_HID;
/* By default, do not copy layout immediately */
H5D_def_dset.layout_copied_to_dcpl = false;

/* Get the default dataset creation property list values and initialize the
* default dataset with them.
Expand Down Expand Up @@ -486,13 +488,19 @@ H5D__new(hid_t dcpl_id, hid_t dapl_id, bool creating, bool vl_type)
if (H5I_inc_ref(dcpl_id, false) < 0)
HGOTO_ERROR(H5E_DATASET, H5E_CANTINC, NULL, "can't increment default DCPL ID");
new_dset->dcpl_id = dcpl_id;

new_dset->layout_copied_to_dcpl = true;
} /* end if */
else {
/* Get the property list */
if (NULL == (plist = (H5P_genplist_t *)H5I_object(dcpl_id)))
HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, NULL, "not a property list");

new_dset->dcpl_id = H5P_copy_plist(plist, false);

/* If a specific DCPL was provided, then the dset's internal DCPL now has an accurate layout */
if (creating)
new_dset->layout_copied_to_dcpl = true;
} /* end else */

if (!vl_type && creating && dapl_id == H5P_DATASET_ACCESS_DEFAULT) {
Expand Down Expand Up @@ -1262,6 +1270,10 @@ H5D__create(H5F_t *file, hid_t type_id, const H5S_t *space, hid_t dcpl_id, hid_t
HGOTO_ERROR(H5E_ARGS, H5E_CANTINIT, NULL, "H5Z_has_optional_filter() failed");

if (false == ignore_filters) {
/* Flush layout to DCPL before reading */
if (H5D_flush_layout_to_dcpl(new_dset) < 0)
HGOTO_ERROR(H5E_DATASET, H5E_CANTSET, NULL, "unable to flush layout");

/* Check if the filters in the DCPL can be applied to this dataset */
if (H5Z_can_apply(new_dset->shared->dcpl_id, new_dset->shared->type_id) < 0)
HGOTO_ERROR(H5E_ARGS, H5E_CANTINIT, NULL, "I/O filters can't operate on this dataset");
Expand Down Expand Up @@ -3021,6 +3033,10 @@ H5D__check_filters(H5D_t *dataset)
if (fill_status == H5D_FILL_VALUE_DEFAULT || fill_status == H5D_FILL_VALUE_USER_DEFINED) {
if (fill->fill_time == H5D_FILL_TIME_ALLOC ||
(fill->fill_time == H5D_FILL_TIME_IFSET && fill_status == H5D_FILL_VALUE_USER_DEFINED)) {
/* Flush layout to DCPL before reading */
if (H5D_flush_layout_to_dcpl(dataset) < 0)
HGOTO_ERROR(H5E_DATASET, H5E_CANTSET, FAIL, "unable to flush layout");

/* Filters must have encoding enabled. Ensure that all filters can be applied */
if (H5Z_can_apply(dataset->shared->dcpl_id, dataset->shared->type_id) < 0)
HGOTO_ERROR(H5E_PLINE, H5E_CANAPPLY, FAIL, "can't apply filters");
Expand Down Expand Up @@ -3632,6 +3648,11 @@ H5D_get_create_plist(const H5D_t *dset)
if (NULL == (dcpl_plist = (H5P_genplist_t *)H5I_object(dset->shared->dcpl_id)))
HGOTO_ERROR(H5E_DATASET, H5E_BADTYPE, FAIL, "can't get property list");

/* If necessary, flush virtual layout changes to the DCPL before copying */
if (H5D_flush_layout_to_dcpl(dset) < 0) {
HGOTO_ERROR(H5E_DATASET, H5E_CANTSET, FAIL, "can't flush layout to DCPL");
}

/* Copy the creation property list */
if ((new_dcpl_id = H5P_copy_plist(dcpl_plist, true)) < 0)
HGOTO_ERROR(H5E_DATASET, H5E_CANTGET, FAIL, "unable to copy the creation property list");
Expand Down Expand Up @@ -4058,3 +4079,56 @@ H5D_get_dcpl_id(const H5D_obj_create_t *d)

FUNC_LEAVE_NOAPI(d->dcpl_id);
} /* end H5D_get_dcpl_id() */

/*-------------------------------------------------------------------------
* Function: H5D_flush_layout_to_dcpl
*
* Purpose: Copy the dataset's creation-time layout to the internal DCPL,
* if this has not yet been done.
*
* Return: Success: non-negative
*
* Failure: negative
*-------------------------------------------------------------------------
*/
herr_t
H5D_flush_layout_to_dcpl(const H5D_t *dset)
{
herr_t ret_value = SUCCEED;
H5P_genplist_t *dcpl = NULL;
bool ndims_modified = false;

FUNC_ENTER_NOAPI(FAIL)

if ((dcpl = H5P_object_verify(dset->shared->dcpl_id, H5P_DATASET_CREATE, true)) == NULL) {
HGOTO_ERROR(H5E_DATASET, H5E_BADID, FAIL, "invalid DCPL ID");
}

if (!dset->shared->layout_copied_to_dcpl) {
/* Don't modify default DCPL; short-circuit success */
if (H5P_is_default_plist(dset->shared->dcpl_id)) {
HGOTO_DONE(ret_value);
}

/* Adjust chunk dimensions to omit datatype size (in last dimension) for creation property */
if (H5D_CHUNKED == dset->shared->layout.type) {
dset->shared->layout.u.chunk.ndims--;
ndims_modified = true;
}

/* Copy layout property to DCPL from dataset */
if (H5P_set(dcpl, H5D_CRT_LAYOUT_NAME, (void *)&dset->shared->layout) < 0) {

HGOTO_ERROR(H5E_DATASET, H5E_CANTSET, FAIL, "can't set VDS layout property");
}
}

done:
if (ret_value == SUCCEED)
dset->shared->layout_copied_to_dcpl = true;

if (ndims_modified)
dset->shared->layout.u.chunk.ndims++;

FUNC_LEAVE_NOAPI(ret_value);
} /* end H5D_flush_layout_to_dcpl() */
4 changes: 4 additions & 0 deletions src/H5Dio.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -568,6 +568,10 @@ H5D__write(size_t count, H5D_dset_io_info_t *dset_info)

/* All filters in the DCPL must have encoding enabled. */
if (!dset_info[i].dset->shared->checked_filters) {
/* Flush layout to DCPL before readaing */
if (H5D_flush_layout_to_dcpl(dset_info[i].dset) < 0)
HGOTO_ERROR(H5E_DATASET, H5E_CANTSET, FAIL, "unable to flush layout");

if (H5Z_can_apply(dset_info[i].dset->shared->dcpl_id, dset_info[i].dset->shared->type_id) < 0)
HGOTO_ERROR(H5E_PLINE, H5E_CANAPPLY, FAIL, "can't apply filters");

Expand Down
25 changes: 9 additions & 16 deletions src/H5Dlayout.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -570,11 +570,11 @@ H5D__layout_oh_create(H5F_t *file, H5O_t *oh, H5D_t *dset, hid_t dapl_id)
herr_t
H5D__layout_oh_read(H5D_t *dataset, hid_t dapl_id, H5P_genplist_t *plist)
{
htri_t msg_exists; /* Whether a particular type of message exists */
bool pline_copied = false; /* Flag to indicate that dcpl_cache.pline's message was copied */
bool layout_copied = false; /* Flag to indicate that layout message was copied */
bool efl_copied = false; /* Flag to indicate that the EFL message was copied */
herr_t ret_value = SUCCEED; /* Return value */
htri_t msg_exists; /* Whether a particular type of message exists */
bool pline_copied = false; /* Flag to indicate that dcpl_cache.pline's message was copied */
bool layout_copied_to_dset = false; /* Flag to indicate that layout message was copied */
bool efl_copied = false; /* Flag to indicate that the EFL message was copied */
herr_t ret_value = SUCCEED; /* Return value */

FUNC_ENTER_PACKAGE

Expand Down Expand Up @@ -603,7 +603,7 @@ H5D__layout_oh_read(H5D_t *dataset, hid_t dapl_id, H5P_genplist_t *plist)
*/
if (NULL == H5O_msg_read(&(dataset->oloc), H5O_LAYOUT_ID, &(dataset->shared->layout)))
HGOTO_ERROR(H5E_DATASET, H5E_CANTINIT, FAIL, "unable to read data layout message");
layout_copied = true;
layout_copied_to_dset = true;

/* Check for external file list message (which might not exist) */
if ((msg_exists = H5O_msg_exists(&(dataset->oloc), H5O_EFL_ID)) < 0)
Expand All @@ -630,25 +630,18 @@ H5D__layout_oh_read(H5D_t *dataset, hid_t dapl_id, H5P_genplist_t *plist)
(dataset->shared->layout.ops->init)(dataset->oloc.file, dataset, dapl_id) < 0)
HGOTO_ERROR(H5E_DATASET, H5E_CANTINIT, FAIL, "unable to initialize layout information");

/* Adjust chunk dimensions to omit datatype size (in last dimension) for creation property */
if (H5D_CHUNKED == dataset->shared->layout.type)
dataset->shared->layout.u.chunk.ndims--;

/* Copy layout to the DCPL */
if (H5P_set(plist, H5D_CRT_LAYOUT_NAME, &dataset->shared->layout) < 0)
HGOTO_ERROR(H5E_DATASET, H5E_CANTSET, FAIL, "can't set layout");

/* Set chunk sizes */
if (H5D_CHUNKED == dataset->shared->layout.type)
if (H5D_CHUNKED == dataset->shared->layout.type) {
if (H5D__chunk_set_sizes(dataset) < 0)
HGOTO_ERROR(H5E_DATASET, H5E_BADVALUE, FAIL, "unable to set chunk sizes");
}

done:
if (ret_value < 0) {
if (pline_copied)
if (H5O_msg_reset(H5O_PLINE_ID, &dataset->shared->dcpl_cache.pline) < 0)
HDONE_ERROR(H5E_DATASET, H5E_CANTRESET, FAIL, "unable to reset pipeline info");
if (layout_copied)
if (layout_copied_to_dset)
if (H5O_msg_reset(H5O_LAYOUT_ID, &dataset->shared->layout) < 0)
HDONE_ERROR(H5E_DATASET, H5E_CANTRESET, FAIL, "unable to reset layout info");
if (efl_copied)
Expand Down
21 changes: 11 additions & 10 deletions src/H5Dpkg.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -531,16 +531,17 @@ typedef struct H5D_rdcdc_t {
* there will be two IDs and two H5D_t structs, both sharing one H5D_shared_t.
*/
struct H5D_shared_t {
size_t fo_count; /* Reference count */
bool closing; /* Flag to indicate dataset is closing */
hid_t type_id; /* ID for dataset's datatype */
H5T_t *type; /* Datatype for this dataset */
H5S_t *space; /* Dataspace of this dataset */
hid_t dcpl_id; /* Dataset creation property id */
hid_t dapl_id; /* Dataset access property id */
H5D_dcpl_cache_t dcpl_cache; /* Cached DCPL values */
H5O_layout_t layout; /* Data layout */
bool checked_filters; /* true if dataset passes can_apply check */
size_t fo_count; /* Reference count */
bool closing; /* Flag to indicate dataset is closing */
hid_t type_id; /* ID for dataset's datatype */
H5T_t *type; /* Datatype for this dataset */
H5S_t *space; /* Dataspace of this dataset */
hid_t dcpl_id; /* Dataset creation property id */
hid_t dapl_id; /* Dataset access property id */
H5D_dcpl_cache_t dcpl_cache; /* Cached DCPL values */
H5O_layout_t layout; /* Data layout */
bool layout_copied_to_dcpl; /* Whether the layout has change not present in the DCPL */
bool checked_filters; /* true if dataset passes can_apply check */

/* Cached dataspace info */
unsigned ndims; /* The dataset's dataspace rank */
Expand Down
1 change: 1 addition & 0 deletions src/H5Dprivate.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,7 @@ H5_DLL herr_t H5D_flush_all(H5F_t *f);
H5_DLL hid_t H5D_get_create_plist(const H5D_t *dset);
H5_DLL hid_t H5D_get_access_plist(const H5D_t *dset);
H5_DLL hid_t H5D_get_dcpl_id(const H5D_obj_create_t *d);
H5_DLL herr_t H5D_flush_layout_to_dcpl(const H5D_t *dset);

/* Functions that operate on chunked storage */
H5_DLL herr_t H5D_chunk_idx_reset(H5O_storage_chunk_t *storage, bool reset_addr);
Expand Down
Loading
Loading