Skip to content
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.

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

Propagate group creation properties to intermediate groups #4139

Merged
merged 9 commits into from
Mar 22, 2024
21 changes: 21 additions & 0 deletions src/H5Dint.c
Original file line number Diff line number Diff line change
Expand Up @@ -3975,3 +3975,24 @@ H5D__refresh(H5D_t *dset, hid_t dset_id)

FUNC_LEAVE_NOAPI(ret_value)
} /* end H5D__refresh() */

/*-------------------------------------------------------------------------
* Function: H5D_get_dcpl_id
*
* Purpose: Quick and dirty routine to retrieve the
* dcpl_id (dataset creation property list) from the
* dataset creation operation struct
*
* Return: 'dcpl_id' on success/abort on failure (shouldn't fail)
*-------------------------------------------------------------------------
*/
hid_t
H5D_get_dcpl_id(const H5D_obj_create_t *d)
{
/* Use FUNC_ENTER_NOAPI_NOINIT_NOERR here to avoid performance issues */
FUNC_ENTER_NOAPI_NOINIT_NOERR

assert(d);

FUNC_LEAVE_NOAPI(d->dcpl_id);
} /* end H5D_get_dcpl_id() */
4 changes: 2 additions & 2 deletions src/H5Dpkg.h
Original file line number Diff line number Diff line change
Expand Up @@ -578,12 +578,12 @@ typedef enum {
} H5D_time_alloc_t;

/* Typedef for dataset creation operation */
typedef struct {
struct H5D_obj_create_t {
hid_t type_id; /* Datatype for dataset */
const H5S_t *space; /* Dataspace for dataset */
hid_t dcpl_id; /* Dataset creation property list */
hid_t dapl_id; /* Dataset access property list */
} H5D_obj_create_t;
};

/* Typedef for filling a buffer with a fill value */
typedef struct H5D_fill_buf_info_t {
Expand Down
10 changes: 9 additions & 1 deletion src/H5Dprivate.h
Original file line number Diff line number Diff line change
Expand Up @@ -124,12 +124,19 @@
/* Default virtual dataset list size */
#define H5D_VIRTUAL_DEF_LIST_SIZE 8

#ifdef H5D_MODULE
#define H5D_OBJ_ID(D) (((H5D_obj_create_t *)(D))->dcpl_id)
#else /* H5D_MODULE */
#define H5D_OBJ_ID(D) (H5D_get_dcpl_id(D))
#endif

/****************************/
/* Library Private Typedefs */
/****************************/

/* Typedef for dataset in memory (defined in H5Dpkg.h) */
typedef struct H5D_t H5D_t;
typedef struct H5D_t H5D_t;
typedef struct H5D_obj_create_t H5D_obj_create_t;

/* Typedef for cached dataset creation property list information */
typedef struct H5D_dcpl_cache_t {
Expand Down Expand Up @@ -171,6 +178,7 @@ H5_DLL H5G_name_t *H5D_nameof(H5D_t *dataset);
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);

/* Functions that operate on chunked storage */
H5_DLL herr_t H5D_chunk_idx_reset(H5O_storage_chunk_t *storage, bool reset_addr);
Expand Down
21 changes: 21 additions & 0 deletions src/H5Gint.c
Original file line number Diff line number Diff line change
Expand Up @@ -1376,3 +1376,24 @@ H5G__get_info_by_idx(const H5G_loc_t *loc, const char *group_name, H5_index_t id

FUNC_LEAVE_NOAPI(ret_value)
} /* end H5G__get_info_by_idx() */

/*-------------------------------------------------------------------------
* Function: H5G_get_gcpl_id
*
* Purpose: Quick and dirty routine to retrieve the
* gcpl_id (group creation property list) from the
* group creation operation struct
*
* Return: 'gcpl_id' on success/abort on failure (shouldn't fail)
*-------------------------------------------------------------------------
*/
hid_t
H5G_get_gcpl_id(const H5G_obj_create_t *g)
{
/* Use FUNC_ENTER_NOAPI_NOINIT_NOERR here to avoid performance issues */
FUNC_ENTER_NOAPI_NOINIT_NOERR

assert(g);

FUNC_LEAVE_NOAPI(g->gcpl_id);
} /* end H5G_get_gcpl_id() */
4 changes: 2 additions & 2 deletions src/H5Gpkg.h
Original file line number Diff line number Diff line change
Expand Up @@ -279,11 +279,11 @@ typedef struct H5G_bt2_ud_ins_t {
} H5G_bt2_ud_ins_t;

/* Typedef for group creation operation */
typedef struct H5G_obj_create_t {
struct H5G_obj_create_t {
hid_t gcpl_id; /* Group creation property list */
H5G_cache_type_t cache_type; /* Type of symbol table entry cache */
H5G_cache_t cache; /* Cached data for symbol table entry */
} H5G_obj_create_t;
};

/* Callback information for copying groups */
typedef struct H5G_copy_file_ud_t {
Expand Down
5 changes: 5 additions & 0 deletions src/H5Gprivate.h
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,10 @@
/* If the module using this macro is allowed access to the private variables, access them directly */
#ifdef H5G_MODULE
#define H5G_MOUNTED(G) ((G)->shared->mounted)
#define H5G_OBJ_ID(G) (((H5G_obj_create_t *)(G))->gcpl_id)
#else /* H5G_MODULE */
#define H5G_MOUNTED(G) (H5G_mounted(G))
#define H5G_OBJ_ID(G) (H5G_get_gcpl_id(G))
#endif /* H5G_MODULE */

/*
Expand All @@ -109,6 +111,7 @@
#define H5G_TARGET_UDLINK 0x0004
#define H5G_TARGET_EXISTS 0x0008
#define H5G_CRT_INTMD_GROUP 0x0010
#define H5G_CRT_OBJ 0x0020

/* Type of operation being performed for call to H5G_name_replace() */
typedef enum {
Expand Down Expand Up @@ -136,6 +139,7 @@ typedef struct H5G_name_t {
/* Forward declarations (for prototypes & struct definitions) */
struct H5O_loc_t;
struct H5O_link_t;
typedef struct H5G_obj_create_t H5G_obj_create_t;

/*
* The "location" of an object in a group hierarchy. This points to an object
Expand Down Expand Up @@ -235,6 +239,7 @@ H5_DLL herr_t H5G_obj_remove_by_idx(const struct H5O_loc_t *grp_oloc, H5RS_str_t
H5_DLL herr_t H5G_obj_lookup_by_idx(const struct H5O_loc_t *grp_oloc, H5_index_t idx_type,
H5_iter_order_t order, hsize_t n, struct H5O_link_t *lnk);
H5_DLL hid_t H5G_get_create_plist(const H5G_t *grp);
H5_DLL hid_t H5G_get_gcpl_id(const H5G_obj_create_t *g);

/*
* These functions operate on symbol table nodes.
Expand Down
12 changes: 10 additions & 2 deletions src/H5Gtraverse.c
Original file line number Diff line number Diff line change
Expand Up @@ -614,6 +614,7 @@ H5G__traverse_real(const H5G_loc_t *_loc, const char *name, unsigned target, H5G
const H5O_linfo_t *linfo; /* Link info settings for new group */
const H5O_pline_t *pline; /* Filter pipeline settings for new group */
H5G_obj_create_t gcrt_info; /* Group creation info */
H5O_obj_create_t *ocrt_info; /* Object creation info in op_data */

/* Check for the parent group having a group info message */
/* (OK if not found) */
Expand Down Expand Up @@ -666,8 +667,15 @@ H5G__traverse_real(const H5G_loc_t *_loc, const char *name, unsigned target, H5G
pline = &def_pline;

/* Create the intermediate group */
/* XXX: Should we allow user to control the group creation params here? -QAK */
gcrt_info.gcpl_id = H5P_GROUP_CREATE_DEFAULT;
gcrt_info.gcpl_id = H5P_GROUP_CREATE_DEFAULT;
/* Propagate the object creation properties when creating intermedidate groups */
if ((target & H5G_CRT_OBJ) && (ocrt_info = H5L_OCRT_INFO(op_data)) != NULL) {
if (ocrt_info->obj_type == H5O_TYPE_GROUP)
gcrt_info.gcpl_id = H5G_OBJ_ID(ocrt_info->crt_info);
else if (ocrt_info->obj_type == H5O_TYPE_DATASET)
gcrt_info.gcpl_id = H5D_OBJ_ID(ocrt_info->crt_info);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like this may be problematic to assign a DCPL ID to a GCPL ID. They're both subclasses of H5P_CLS_OBJECT_CREATE_g, but if the code ever needs to retrieve some group-specific properties (maybe in the future), it seems like that may fail. Should gcrt_info have an ocpl_id field rather than a gcpl_id field to try to make it clear that only generic OCPL properties should be retrieved from it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like this may be problematic to assign a DCPL ID to a GCPL ID. They're both subclasses of H5P_CLS_OBJECT_CREATE_g, but if the code ever needs to retrieve some group-specific properties (maybe in the future), it seems like that may fail. Should gcrt_info have an ocpl_id field rather than a gcpl_id field to try to make it clear that only generic OCPL properties should be retrieved from it?

Agree

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have couple questions about changing gcpl_id to ocpl_id:
--Changing gcpl_id field in gcrt_info to ocpl_id would lead to changes in quite a few files which uses gcrt_info.gpcl_id.
--If I change gcpl_id in gcrt_info which is H5G_obj_create_t struct, should I also change dcpl_id field in H5D_obj_create_t struct to ocpl_id?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--Changing gcpl_id field in gcrt_info to ocpl_id would lead to changes in quite a few files which uses gcrt_info.gpcl_id.

It's probably fine to put this off to a separate focused PR since the code appears to currently only retrieve generic properties.

--If I change gcpl_id in gcrt_info which is H5G_obj_create_t struct, should I also change dcpl_id field in H5D_obj_create_t struct to ocpl_id?

As far as I can tell, it looks like the only place that sets up a H5D_obj_create_t struct sets its dcpl_id field to a DCPL, so that one shouldn't need to be changed.

}

gcrt_info.cache_type = H5G_NOTHING_CACHED;
memset(&gcrt_info.cache, 0, sizeof(gcrt_info.cache));
if (H5G__obj_create_real(grp_oloc.file, ginfo, linfo, pline, &gcrt_info,
Expand Down
26 changes: 24 additions & 2 deletions src/H5Lint.c
Original file line number Diff line number Diff line change
Expand Up @@ -91,13 +91,13 @@ typedef struct {
} H5L_trav_gnbi_t;

/* User data for path traversal callback to creating a link */
typedef struct {
struct H5L_trav_cr_t {
H5F_t *file; /* Pointer to the file */
H5P_genplist_t *lc_plist; /* Link creation property list */
H5G_name_t *path; /* Path to object being linked */
H5O_obj_create_t *ocrt_info; /* Pointer to object creation info */
H5O_link_t *lnk; /* Pointer to link information to insert */
} H5L_trav_cr_t;
};

/* User data for path traversal routine for moving and renaming a link */
typedef struct {
Expand Down Expand Up @@ -706,6 +706,9 @@ H5L__create_real(const H5G_loc_t *link_loc, const char *link_name, H5G_name_t *o
target_flags |= H5G_CRT_INTMD_GROUP;
} /* end if */

if (ocrt_info != NULL)
target_flags |= H5G_CRT_OBJ;

/* Set up user data
* FILE is used to make sure that hard links don't cross files, and
* should be NULL for other link types.
Expand Down Expand Up @@ -2152,3 +2155,22 @@ H5L_iterate(H5G_loc_t *loc, const char *group_name, H5_index_t idx_type, H5_iter
done:
FUNC_LEAVE_NOAPI(ret_value)
} /* end H5L_iterate() */

/*-------------------------------------------------------------------------
* Function: H5L_get_ocrt_info
*
* Purpose: Quick and dirty routine to retrieve the link's object_creation info
*
* Return: 'ocrt_info' on success/abort on failure (shouldn't fail)
*-------------------------------------------------------------------------
*/
H5O_obj_create_t *
H5L_get_ocrt_info(const H5L_trav_cr_t *l)
{
/* Use FUNC_ENTER_NOAPI_NOINIT_NOERR here to avoid performance issues */
FUNC_ENTER_NOAPI_NOINIT_NOERR

assert(l);

FUNC_LEAVE_NOAPI(l->ocrt_info);
} /* end H5L_get_ocrt_info() */
9 changes: 9 additions & 0 deletions src/H5Lprivate.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,12 @@
/* callback function for external link traversal */
#define H5L_ACS_ELINK_CB_NAME "external link callback"

#ifdef H5L_MODULE
#define H5L_OCRT_INFO(L) (((H5L_trav_cr_t *)(L))->ocrt_info)
#else /* H5L_MODULE */
#define H5L_OCRT_INFO(L) (H5L_get_ocrt_info(L))
#endif

/****************************/
/* Library Private Typedefs */
/****************************/
Expand All @@ -57,6 +63,8 @@ typedef struct H5L_elink_cb_t {
void *user_data;
} H5L_elink_cb_t;

typedef struct H5L_trav_cr_t H5L_trav_cr_t;

/*****************************/
/* Library Private Variables */
/*****************************/
Expand All @@ -75,6 +83,7 @@ H5_DLL herr_t H5L_get_info(const H5G_loc_t *loc, const char *name, H5L_info2_t *
H5_DLL herr_t H5L_register_external(void);
H5_DLL herr_t H5L_iterate(H5G_loc_t *loc, const char *group_name, H5_index_t idx_type, H5_iter_order_t order,
hsize_t *idx_p, H5L_iterate2_t op, void *op_data);
H5_DLL H5O_obj_create_t *H5L_get_ocrt_info(const H5L_trav_cr_t *l);

/* User-defined link functions */
H5_DLL herr_t H5L_register(const H5L_class_t *cls);
Expand Down
Loading