Skip to content

Commit 68bf564

Browse files
mattjalamattjala
authored
Delay dataset layout copy to DCPL (#5537)
1 parent b89f178 commit 68bf564

File tree

7 files changed

+375
-26
lines changed

7 files changed

+375
-26
lines changed

‎src/H5Dchunk.c

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -771,9 +771,7 @@ H5D__chunk_set_sizes(H5D_t *dset)
771771

772772
/* Sanity checks */
773773
assert(dset);
774-
775-
/* Increment # of chunk dimensions, to account for datatype size as last element */
776-
dset->shared->layout.u.chunk.ndims++;
774+
assert(dset->shared->layout.u.chunk.ndims > 0);
777775

778776
/* Set the last dimension of the chunk size to the size of the datatype */
779777
dset->shared->layout.u.chunk.dim[dset->shared->layout.u.chunk.ndims - 1] =
@@ -838,6 +836,9 @@ H5D__chunk_construct(H5F_t H5_ATTR_UNUSED *f, H5D_t *dset)
838836
if (dset->shared->layout.u.chunk.ndims != dset->shared->ndims)
839837
HGOTO_ERROR(H5E_DATASET, H5E_BADVALUE, FAIL, "dimensionality of chunks doesn't match the dataspace");
840838

839+
/* Increment # of chunk dimensions, to account for datatype size as last element */
840+
dset->shared->layout.u.chunk.ndims++;
841+
841842
/* Set chunk sizes */
842843
if (H5D__chunk_set_sizes(dset) < 0)
843844
HGOTO_ERROR(H5E_DATASET, H5E_BADVALUE, FAIL, "unable to set chunk sizes");

‎src/H5Dint.c

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,8 @@ H5D__init_package(void)
201201
H5D_def_dset.type_id = H5I_INVALID_HID;
202202
H5D_def_dset.dapl_id = H5I_INVALID_HID;
203203
H5D_def_dset.dcpl_id = H5I_INVALID_HID;
204+
/* By default, do not copy layout immediately */
205+
H5D_def_dset.layout_copied_to_dcpl = false;
204206

205207
/* Get the default dataset creation property list values and initialize the
206208
* default dataset with them.
@@ -485,13 +487,19 @@ H5D__new(hid_t dcpl_id, hid_t dapl_id, bool creating, bool vl_type)
485487
if (H5I_inc_ref(dcpl_id, false) < 0)
486488
HGOTO_ERROR(H5E_DATASET, H5E_CANTINC, NULL, "can't increment default DCPL ID");
487489
new_dset->dcpl_id = dcpl_id;
490+
491+
new_dset->layout_copied_to_dcpl = true;
488492
} /* end if */
489493
else {
490494
/* Get the property list */
491495
if (NULL == (plist = (H5P_genplist_t *)H5I_object(dcpl_id)))
492496
HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, NULL, "not a property list");
493497

494498
new_dset->dcpl_id = H5P_copy_plist(plist, false);
499+
500+
/* If a specific DCPL was provided, then the dset's internal DCPL now has an accurate layout */
501+
if (creating)
502+
new_dset->layout_copied_to_dcpl = true;
495503
} /* end else */
496504

497505
if (!vl_type && creating && dapl_id == H5P_DATASET_ACCESS_DEFAULT) {
@@ -1261,6 +1269,9 @@ H5D__create(H5F_t *file, hid_t type_id, const H5S_t *space, hid_t dcpl_id, hid_t
12611269
HGOTO_ERROR(H5E_ARGS, H5E_CANTINIT, NULL, "H5Z_has_optional_filter() failed");
12621270

12631271
if (false == ignore_filters) {
1272+
/* Layout only exists on DCPL at this point in dset creation */
1273+
assert(new_dset->shared->layout_copied_to_dcpl);
1274+
12641275
/* Check if the filters in the DCPL can be applied to this dataset */
12651276
if (H5Z_can_apply(new_dset->shared->dcpl_id, new_dset->shared->type_id) < 0)
12661277
HGOTO_ERROR(H5E_ARGS, H5E_CANTINIT, NULL, "I/O filters can't operate on this dataset");
@@ -3020,6 +3031,10 @@ H5D__check_filters(H5D_t *dataset)
30203031
if (fill_status == H5D_FILL_VALUE_DEFAULT || fill_status == H5D_FILL_VALUE_USER_DEFINED) {
30213032
if (fill->fill_time == H5D_FILL_TIME_ALLOC ||
30223033
(fill->fill_time == H5D_FILL_TIME_IFSET && fill_status == H5D_FILL_VALUE_USER_DEFINED)) {
3034+
/* Flush layout to DCPL before reading */
3035+
if (H5D_flush_layout_to_dcpl(dataset) < 0)
3036+
HGOTO_ERROR(H5E_DATASET, H5E_CANTSET, FAIL, "unable to flush layout");
3037+
30233038
/* Filters must have encoding enabled. Ensure that all filters can be applied */
30243039
if (H5Z_can_apply(dataset->shared->dcpl_id, dataset->shared->type_id) < 0)
30253040
HGOTO_ERROR(H5E_PLINE, H5E_CANAPPLY, FAIL, "can't apply filters");
@@ -3631,6 +3646,11 @@ H5D_get_create_plist(const H5D_t *dset)
36313646
if (NULL == (dcpl_plist = (H5P_genplist_t *)H5I_object(dset->shared->dcpl_id)))
36323647
HGOTO_ERROR(H5E_DATASET, H5E_BADTYPE, FAIL, "can't get property list");
36333648

3649+
/* If necessary, flush virtual layout changes to the DCPL before copying */
3650+
if (H5D_flush_layout_to_dcpl(dset) < 0) {
3651+
HGOTO_ERROR(H5E_DATASET, H5E_CANTSET, FAIL, "can't flush layout to DCPL");
3652+
}
3653+
36343654
/* Copy the creation property list */
36353655
if ((new_dcpl_id = H5P_copy_plist(dcpl_plist, true)) < 0)
36363656
HGOTO_ERROR(H5E_DATASET, H5E_CANTGET, FAIL, "unable to copy the creation property list");
@@ -4057,3 +4077,55 @@ H5D_get_dcpl_id(const H5D_obj_create_t *d)
40574077

40584078
FUNC_LEAVE_NOAPI(d->dcpl_id);
40594079
} /* end H5D_get_dcpl_id() */
4080+
4081+
/*-------------------------------------------------------------------------
4082+
* Function: H5D_flush_layout_to_dcpl
4083+
*
4084+
* Purpose: Copy the dataset's creation-time layout to the internal DCPL,
4085+
* if this has not yet been done.
4086+
*
4087+
* Return: Success: non-negative
4088+
*
4089+
* Failure: negative
4090+
*-------------------------------------------------------------------------
4091+
*/
4092+
herr_t
4093+
H5D_flush_layout_to_dcpl(const H5D_t *dset)
4094+
{
4095+
herr_t ret_value = SUCCEED;
4096+
H5P_genplist_t *dcpl = NULL;
4097+
bool ndims_modified = false;
4098+
4099+
FUNC_ENTER_NOAPI(FAIL)
4100+
4101+
if ((dcpl = H5P_object_verify(dset->shared->dcpl_id, H5P_DATASET_CREATE, true)) == NULL) {
4102+
HGOTO_ERROR(H5E_DATASET, H5E_BADID, FAIL, "invalid DCPL ID");
4103+
}
4104+
4105+
if (!dset->shared->layout_copied_to_dcpl) {
4106+
/* Don't modify default DCPL; short-circuit success */
4107+
if (H5P_is_default_plist(dset->shared->dcpl_id)) {
4108+
HGOTO_DONE(ret_value);
4109+
}
4110+
4111+
/* Adjust chunk dimensions to omit datatype size (in last dimension) for creation property */
4112+
if (H5D_CHUNKED == dset->shared->layout.type) {
4113+
dset->shared->layout.u.chunk.ndims--;
4114+
ndims_modified = true;
4115+
}
4116+
4117+
/* Copy layout property to DCPL from dataset */
4118+
if (H5P_set(dcpl, H5D_CRT_LAYOUT_NAME, (void *)&dset->shared->layout) < 0) {
4119+
HGOTO_ERROR(H5E_DATASET, H5E_CANTSET, FAIL, "can't set layout property");
4120+
}
4121+
}
4122+
4123+
done:
4124+
if (ret_value == SUCCEED)
4125+
dset->shared->layout_copied_to_dcpl = true;
4126+
4127+
if (ndims_modified)
4128+
dset->shared->layout.u.chunk.ndims++;
4129+
4130+
FUNC_LEAVE_NOAPI(ret_value);
4131+
} /* end H5D_flush_layout_to_dcpl() */

‎src/H5Dio.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -568,6 +568,10 @@ H5D__write(size_t count, H5D_dset_io_info_t *dset_info)
568568

569569
/* All filters in the DCPL must have encoding enabled. */
570570
if (!dset_info[i].dset->shared->checked_filters) {
571+
/* Flush layout to DCPL before readaing */
572+
if (H5D_flush_layout_to_dcpl(dset_info[i].dset) < 0)
573+
HGOTO_ERROR(H5E_DATASET, H5E_CANTSET, FAIL, "unable to flush layout");
574+
571575
if (H5Z_can_apply(dset_info[i].dset->shared->dcpl_id, dset_info[i].dset->shared->type_id) < 0)
572576
HGOTO_ERROR(H5E_PLINE, H5E_CANAPPLY, FAIL, "can't apply filters");
573577

‎src/H5Dlayout.c

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -570,11 +570,11 @@ H5D__layout_oh_create(H5F_t *file, H5O_t *oh, H5D_t *dset, hid_t dapl_id)
570570
herr_t
571571
H5D__layout_oh_read(H5D_t *dataset, hid_t dapl_id, H5P_genplist_t *plist)
572572
{
573-
htri_t msg_exists; /* Whether a particular type of message exists */
574-
bool pline_copied = false; /* Flag to indicate that dcpl_cache.pline's message was copied */
575-
bool layout_copied = false; /* Flag to indicate that layout message was copied */
576-
bool efl_copied = false; /* Flag to indicate that the EFL message was copied */
577-
herr_t ret_value = SUCCEED; /* Return value */
573+
htri_t msg_exists; /* Whether a particular type of message exists */
574+
bool pline_copied = false; /* Flag to indicate that dcpl_cache.pline's message was copied */
575+
bool layout_copied_to_dset = false; /* Flag to indicate that layout message was copied */
576+
bool efl_copied = false; /* Flag to indicate that the EFL message was copied */
577+
herr_t ret_value = SUCCEED; /* Return value */
578578

579579
FUNC_ENTER_PACKAGE
580580

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

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

633-
/* Adjust chunk dimensions to omit datatype size (in last dimension) for creation property */
634-
if (H5D_CHUNKED == dataset->shared->layout.type)
635-
dataset->shared->layout.u.chunk.ndims--;
633+
#ifndef NDEBUG
634+
/* Set invalid layout to detect erroneous usage */
635+
H5O_layout_t error_layout;
636+
error_layout.type = H5D_LAYOUT_ERROR;
637+
error_layout.version = 0;
638+
error_layout.ops = NULL;
639+
error_layout.storage.type = H5D_LAYOUT_ERROR;
636640

637-
/* Copy layout to the DCPL */
638-
if (H5P_set(plist, H5D_CRT_LAYOUT_NAME, &dataset->shared->layout) < 0)
639-
HGOTO_ERROR(H5E_DATASET, H5E_CANTSET, FAIL, "can't set layout");
641+
if (H5P_poke(plist, H5D_CRT_LAYOUT_NAME, &error_layout) < 0)
642+
HGOTO_ERROR(H5E_DATASET, H5E_CANTSET, FAIL, "unable to setup placeholder layout");
643+
#endif
640644

641645
/* Set chunk sizes */
642646
if (H5D_CHUNKED == dataset->shared->layout.type)
@@ -648,7 +652,7 @@ H5D__layout_oh_read(H5D_t *dataset, hid_t dapl_id, H5P_genplist_t *plist)
648652
if (pline_copied)
649653
if (H5O_msg_reset(H5O_PLINE_ID, &dataset->shared->dcpl_cache.pline) < 0)
650654
HDONE_ERROR(H5E_DATASET, H5E_CANTRESET, FAIL, "unable to reset pipeline info");
651-
if (layout_copied)
655+
if (layout_copied_to_dset)
652656
if (H5O_msg_reset(H5O_LAYOUT_ID, &dataset->shared->layout) < 0)
653657
HDONE_ERROR(H5E_DATASET, H5E_CANTRESET, FAIL, "unable to reset layout info");
654658
if (efl_copied)

‎src/H5Dpkg.h

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -531,16 +531,17 @@ typedef struct H5D_rdcdc_t {
531531
* there will be two IDs and two H5D_t structs, both sharing one H5D_shared_t.
532532
*/
533533
struct H5D_shared_t {
534-
size_t fo_count; /* Reference count */
535-
bool closing; /* Flag to indicate dataset is closing */
536-
hid_t type_id; /* ID for dataset's datatype */
537-
H5T_t *type; /* Datatype for this dataset */
538-
H5S_t *space; /* Dataspace of this dataset */
539-
hid_t dcpl_id; /* Dataset creation property id */
540-
hid_t dapl_id; /* Dataset access property id */
541-
H5D_dcpl_cache_t dcpl_cache; /* Cached DCPL values */
542-
H5O_layout_t layout; /* Data layout */
543-
bool checked_filters; /* true if dataset passes can_apply check */
534+
size_t fo_count; /* Reference count */
535+
bool closing; /* Flag to indicate dataset is closing */
536+
hid_t type_id; /* ID for dataset's datatype */
537+
H5T_t *type; /* Datatype for this dataset */
538+
H5S_t *space; /* Dataspace of this dataset */
539+
hid_t dcpl_id; /* Dataset creation property id */
540+
hid_t dapl_id; /* Dataset access property id */
541+
H5D_dcpl_cache_t dcpl_cache; /* Cached DCPL values */
542+
H5O_layout_t layout; /* Data layout */
543+
bool layout_copied_to_dcpl; /* Whether the layout has change not present in the DCPL */
544+
bool checked_filters; /* true if dataset passes can_apply check */
544545

545546
/* Cached dataspace info */
546547
unsigned ndims; /* The dataset's dataspace rank */

‎src/H5Dprivate.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,7 @@ H5_DLL herr_t H5D_flush_all(H5F_t *f);
179179
H5_DLL hid_t H5D_get_create_plist(const H5D_t *dset);
180180
H5_DLL hid_t H5D_get_access_plist(const H5D_t *dset);
181181
H5_DLL hid_t H5D_get_dcpl_id(const H5D_obj_create_t *d);
182+
H5_DLL herr_t H5D_flush_layout_to_dcpl(const H5D_t *dset);
182183

183184
/* Functions that operate on chunked storage */
184185
H5_DLL herr_t H5D_chunk_idx_reset(H5O_storage_chunk_t *storage, bool reset_addr);

0 commit comments

Comments
 (0)