Skip to content

Commit

Permalink
Refactor allocation of API context (#4942)
Browse files Browse the repository at this point in the history
Since each API context is local to a thread, use the stack to
store the context instead of allocating & releasing it each time.
This improves performance (slightly), reduces alloc/free calls,
and eliminates the H5FL package from the push & pop operations,
which helps simplify threadsafe operation.

One effect of this change is that the H5VLstart_lib_state /
H5VLfinish_lib_state API routines for pass through connector
authors now require a parameter that can be used to store
the library's context. It was probably a mistake to assume
that these two routines would not do this previously, so this
is essentially a bug fix for them.

Some other minor things:

 * Added API context push+pop operations to cache tests
  (I'm not actually certain why this was working before) and
  a few other places
* Cleaned up a bunch of warnings in test code (calloc args, mainly)
* Made header file inclusions more standard in some source files
  • Loading branch information
qkoziol authored Oct 24, 2024
1 parent 2c58357 commit 97e1ed4
Show file tree
Hide file tree
Showing 58 changed files with 1,354 additions and 954 deletions.
4 changes: 4 additions & 0 deletions release_docs/RELEASE.txt
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,10 @@ New Features

Library:
--------
- The H5VLstart_lib_state / H5VLfinish_lib_state API routines for pass-
through connector authors now require a parameter that can be used to
store the library's context.

- Removed H5FDperform_init API routine. Virtual File Driver (VFD)
developers who wish to provide an ID for their driver should create
a routine specific to their individual implementation.
Expand Down
5 changes: 3 additions & 2 deletions src/H5.c
Original file line number Diff line number Diff line change
Expand Up @@ -304,9 +304,10 @@ H5_term_library(void)
size_t at = 0;
char loop[1024];
H5E_auto2_t func;
H5CX_node_t api_ctx = {{0}, NULL}; /* API context node to push */

/* Acquire the API lock */
H5CANCEL_DECL
FUNC_ENTER_API_VARS
H5_API_LOCK

/* Don't do anything if the library is already closed */
Expand All @@ -317,7 +318,7 @@ H5_term_library(void)
H5_TERM_GLOBAL = true;

/* Push the API context without checking for errors */
H5CX_push_special();
H5CX_push(&api_ctx);

/* Check if we should display error output */
(void)H5Eget_auto2(H5E_DEFAULT, &func, NULL);
Expand Down
9 changes: 5 additions & 4 deletions src/H5Atest.c
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,10 @@ H5A__is_shared_test(hid_t attr_id)
herr_t
H5A__get_shared_rc_test(hid_t attr_id, hsize_t *ref_count)
{
H5A_t *attr; /* Attribute object for ID */
bool api_ctx_pushed = false; /* Whether API context pushed */
herr_t ret_value = SUCCEED; /* Return value */
H5A_t *attr; /* Attribute object for ID */
H5CX_node_t api_ctx = {{0}, NULL}; /* API context node to push */
bool api_ctx_pushed = false; /* Whether API context pushed */
herr_t ret_value = SUCCEED; /* Return value */

FUNC_ENTER_PACKAGE

Expand All @@ -116,7 +117,7 @@ H5A__get_shared_rc_test(hid_t attr_id, hsize_t *ref_count)
HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "not an attribute");

/* Push API context */
if (H5CX_push() < 0)
if (H5CX_push(&api_ctx) < 0)
HGOTO_ERROR(H5E_ATTR, H5E_CANTSET, FAIL, "can't set API context");
api_ctx_pushed = true;

Expand Down
415 changes: 47 additions & 368 deletions src/H5CX.c

Large diffs are not rendered by default.

241 changes: 235 additions & 6 deletions src/H5CXprivate.h

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion src/H5FDprivate.h
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ H5_DLL htri_t H5FD_is_driver_registered_by_name(const char *driver_name,
H5_DLL htri_t H5FD_is_driver_registered_by_value(H5FD_class_value_t driver_value, hid_t *registered_id);
H5_DLL hid_t H5FD_get_driver_id_by_name(const char *name, bool is_api);
H5_DLL hid_t H5FD_get_driver_id_by_value(H5FD_class_value_t value, bool is_api);
H5_DLL herr_t H5FD_open(bool try, H5FD_t **file, const char *name, unsigned flags, hid_t fapl_id,
H5_DLL herr_t H5FD_open(bool attempt, H5FD_t **file, const char *name, unsigned flags, hid_t fapl_id,
haddr_t maxaddr);
H5_DLL herr_t H5FD_close(H5FD_t *file);
H5_DLL int H5FD_cmp(const H5FD_t *f1, const H5FD_t *f2);
Expand Down
8 changes: 4 additions & 4 deletions src/H5Fprivate.h
Original file line number Diff line number Diff line change
Expand Up @@ -493,7 +493,7 @@ typedef enum H5F_prefix_open_t {

/* Private functions */
H5_DLL herr_t H5F_init(void);
H5_DLL herr_t H5F_open(bool try, H5F_t **file, const char *name, unsigned flags, hid_t fcpl_id,
H5_DLL herr_t H5F_open(bool attempt, H5F_t **file, const char *name, unsigned flags, hid_t fcpl_id,
hid_t fapl_id);
H5_DLL herr_t H5F_try_close(H5F_t *f, bool *was_closed /*out*/);
H5_DLL hid_t H5F_get_file_id(H5VL_object_t *vol_obj, H5I_type_t obj_type, bool app_ref);
Expand Down Expand Up @@ -657,9 +657,9 @@ H5_DLL herr_t H5F_shared_get_mpi_file_sync_required(const H5F_shared_t *f_sh,
H5_DLL herr_t H5F_efc_close(H5F_t *parent, H5F_t *file);

/* File prefix routines */
H5_DLL herr_t H5F_prefix_open_file(bool try, H5F_t **file, H5F_t *primary_file, H5F_prefix_open_t prefix_type,
const char *prop_prefix, const char *file_name, unsigned file_intent,
hid_t fapl_id);
H5_DLL herr_t H5F_prefix_open_file(bool attempt, H5F_t **file, H5F_t *primary_file,
H5F_prefix_open_t prefix_type, const char *prop_prefix,
const char *file_name, unsigned file_intent, hid_t fapl_id);

/* Global heap CWFS routines */
H5_DLL herr_t H5F_cwfs_add(H5F_t *f, struct H5HG_heap_t *heap);
Expand Down
18 changes: 10 additions & 8 deletions src/H5Ftest.c
Original file line number Diff line number Diff line change
Expand Up @@ -82,9 +82,10 @@
herr_t
H5F__get_sohm_mesg_count_test(hid_t file_id, unsigned type_id, size_t *mesg_count)
{
H5F_t *file; /* File info */
bool api_ctx_pushed = false; /* Whether API context pushed */
herr_t ret_value = SUCCEED; /* Return value */
H5F_t *file; /* File info */
H5CX_node_t api_ctx = {{0}, NULL}; /* API context node to push */
bool api_ctx_pushed = false; /* Whether API context pushed */
herr_t ret_value = SUCCEED; /* Return value */

FUNC_ENTER_PACKAGE

Expand All @@ -93,7 +94,7 @@ H5F__get_sohm_mesg_count_test(hid_t file_id, unsigned type_id, size_t *mesg_coun
HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "not a file");

/* Push API context */
if (H5CX_push() < 0)
if (H5CX_push(&api_ctx) < 0)
HGOTO_ERROR(H5E_FILE, H5E_CANTSET, FAIL, "can't set API context");
api_ctx_pushed = true;

Expand Down Expand Up @@ -123,9 +124,10 @@ H5F__get_sohm_mesg_count_test(hid_t file_id, unsigned type_id, size_t *mesg_coun
herr_t
H5F__check_cached_stab_test(hid_t file_id)
{
H5F_t *file; /* File info */
bool api_ctx_pushed = false; /* Whether API context pushed */
herr_t ret_value = SUCCEED; /* Return value */
H5F_t *file; /* File info */
H5CX_node_t api_ctx = {{0}, NULL}; /* API context node to push */
bool api_ctx_pushed = false; /* Whether API context pushed */
herr_t ret_value = SUCCEED; /* Return value */

FUNC_ENTER_PACKAGE

Expand All @@ -134,7 +136,7 @@ H5F__check_cached_stab_test(hid_t file_id)
HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "not a file");

/* Push API context */
if (H5CX_push() < 0)
if (H5CX_push(&api_ctx) < 0)
HGOTO_ERROR(H5E_FILE, H5E_CANTSET, FAIL, "can't set API context");
api_ctx_pushed = true;

Expand Down
83 changes: 45 additions & 38 deletions src/H5Gtest.c
Original file line number Diff line number Diff line change
Expand Up @@ -83,11 +83,12 @@
htri_t
H5G__is_empty_test(hid_t gid)
{
H5G_t *grp = NULL; /* Pointer to group */
htri_t msg_exists = false; /* Indicate that a header message is present */
htri_t linfo_exists = false; /* Indicate that the 'link info' message is present */
bool api_ctx_pushed = false; /* Whether API context pushed */
htri_t ret_value = true; /* Return value */
H5G_t *grp = NULL; /* Pointer to group */
htri_t msg_exists = false; /* Indicate that a header message is present */
htri_t linfo_exists = false; /* Indicate that the 'link info' message is present */
H5CX_node_t api_ctx = {{0}, NULL}; /* API context node to push */
bool api_ctx_pushed = false; /* Whether API context pushed */
htri_t ret_value = true; /* Return value */

FUNC_ENTER_PACKAGE

Expand All @@ -96,7 +97,7 @@ H5G__is_empty_test(hid_t gid)
HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "not a group");

/* Set API context */
if (H5CX_push() < 0)
if (H5CX_push(&api_ctx) < 0)
HGOTO_ERROR(H5E_SYM, H5E_CANTSET, FAIL, "can't set API context");
api_ctx_pushed = true;

Expand Down Expand Up @@ -203,10 +204,11 @@ H5G__is_empty_test(hid_t gid)
htri_t
H5G__has_links_test(hid_t gid, unsigned *nmsgs)
{
H5G_t *grp = NULL; /* Pointer to group */
htri_t msg_exists = 0; /* Indicate that a header message is present */
bool api_ctx_pushed = false; /* Whether API context pushed */
htri_t ret_value = true; /* Return value */
H5G_t *grp = NULL; /* Pointer to group */
htri_t msg_exists = 0; /* Indicate that a header message is present */
H5CX_node_t api_ctx = {{0}, NULL}; /* API context node to push */
bool api_ctx_pushed = false; /* Whether API context pushed */
htri_t ret_value = true; /* Return value */

FUNC_ENTER_PACKAGE

Expand All @@ -215,7 +217,7 @@ H5G__has_links_test(hid_t gid, unsigned *nmsgs)
HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "not a group");

/* Set API context */
if (H5CX_push() < 0)
if (H5CX_push(&api_ctx) < 0)
HGOTO_ERROR(H5E_SYM, H5E_CANTSET, FAIL, "can't set API context");
api_ctx_pushed = true;

Expand Down Expand Up @@ -269,10 +271,11 @@ H5G__has_links_test(hid_t gid, unsigned *nmsgs)
htri_t
H5G__has_stab_test(hid_t gid)
{
H5G_t *grp = NULL; /* Pointer to group */
htri_t msg_exists = 0; /* Indicate that a header message is present */
bool api_ctx_pushed = false; /* Whether API context pushed */
htri_t ret_value = true; /* Return value */
H5G_t *grp = NULL; /* Pointer to group */
htri_t msg_exists = 0; /* Indicate that a header message is present */
H5CX_node_t api_ctx = {{0}, NULL}; /* API context node to push */
bool api_ctx_pushed = false; /* Whether API context pushed */
htri_t ret_value = true; /* Return value */

FUNC_ENTER_PACKAGE

Expand All @@ -281,7 +284,7 @@ H5G__has_stab_test(hid_t gid)
HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "not a group");

/* Set API context */
if (H5CX_push() < 0)
if (H5CX_push(&api_ctx) < 0)
HGOTO_ERROR(H5E_SYM, H5E_CANTSET, FAIL, "can't set API context");
api_ctx_pushed = true;

Expand Down Expand Up @@ -327,10 +330,11 @@ H5G__has_stab_test(hid_t gid)
htri_t
H5G__is_new_dense_test(hid_t gid)
{
H5G_t *grp = NULL; /* Pointer to group */
htri_t msg_exists = 0; /* Indicate that a header message is present */
bool api_ctx_pushed = false; /* Whether API context pushed */
htri_t ret_value = true; /* Return value */
H5G_t *grp = NULL; /* Pointer to group */
htri_t msg_exists = 0; /* Indicate that a header message is present */
H5CX_node_t api_ctx = {{0}, NULL}; /* API context node to push */
bool api_ctx_pushed = false; /* Whether API context pushed */
htri_t ret_value = true; /* Return value */

FUNC_ENTER_PACKAGE

Expand All @@ -339,7 +343,7 @@ H5G__is_new_dense_test(hid_t gid)
HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "not a group");

/* Set API context */
if (H5CX_push() < 0)
if (H5CX_push(&api_ctx) < 0)
HGOTO_ERROR(H5E_SYM, H5E_CANTSET, FAIL, "can't set API context");
api_ctx_pushed = true;

Expand Down Expand Up @@ -403,12 +407,13 @@ H5G__is_new_dense_test(hid_t gid)
herr_t
H5G__new_dense_info_test(hid_t gid, hsize_t *name_count, hsize_t *corder_count)
{
H5B2_t *bt2_name = NULL; /* v2 B-tree handle for name index */
H5B2_t *bt2_corder = NULL; /* v2 B-tree handle for creation order index */
H5O_linfo_t linfo; /* Link info message */
H5G_t *grp = NULL; /* Pointer to group */
bool api_ctx_pushed = false; /* Whether API context pushed */
herr_t ret_value = SUCCEED; /* Return value */
H5B2_t *bt2_name = NULL; /* v2 B-tree handle for name index */
H5B2_t *bt2_corder = NULL; /* v2 B-tree handle for creation order index */
H5O_linfo_t linfo; /* Link info message */
H5G_t *grp = NULL; /* Pointer to group */
H5CX_node_t api_ctx = {{0}, NULL}; /* API context node to push */
bool api_ctx_pushed = false; /* Whether API context pushed */
herr_t ret_value = SUCCEED; /* Return value */

FUNC_ENTER_PACKAGE

Expand All @@ -417,7 +422,7 @@ H5G__new_dense_info_test(hid_t gid, hsize_t *name_count, hsize_t *corder_count)
HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "not a group");

/* Set API context */
if (H5CX_push() < 0)
if (H5CX_push(&api_ctx) < 0)
HGOTO_ERROR(H5E_SYM, H5E_CANTSET, FAIL, "can't set API context");
api_ctx_pushed = true;

Expand Down Expand Up @@ -494,10 +499,11 @@ H5G__new_dense_info_test(hid_t gid, hsize_t *name_count, hsize_t *corder_count)
herr_t
H5G__lheap_size_test(hid_t gid, size_t *lheap_size)
{
H5G_t *grp = NULL; /* Pointer to group */
H5O_stab_t stab; /* Symbol table message */
bool api_ctx_pushed = false; /* Whether API context pushed */
herr_t ret_value = SUCCEED; /* Return value */
H5G_t *grp = NULL; /* Pointer to group */
H5O_stab_t stab; /* Symbol table message */
H5CX_node_t api_ctx = {{0}, NULL}; /* API context node to push */
bool api_ctx_pushed = false; /* Whether API context pushed */
herr_t ret_value = SUCCEED; /* Return value */

FUNC_ENTER_PACKAGE

Expand All @@ -506,7 +512,7 @@ H5G__lheap_size_test(hid_t gid, size_t *lheap_size)
HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "not a group");

/* Set API context */
if (H5CX_push() < 0)
if (H5CX_push(&api_ctx) < 0)
HGOTO_ERROR(H5E_SYM, H5E_CANTSET, FAIL, "can't set API context");
api_ctx_pushed = true;

Expand Down Expand Up @@ -550,10 +556,11 @@ H5G__lheap_size_test(hid_t gid, size_t *lheap_size)
herr_t
H5G__user_path_test(hid_t obj_id, char *user_path, size_t *user_path_len, unsigned *obj_hidden)
{
void *obj_ptr; /* Pointer to object for ID */
const H5G_name_t *obj_path; /* Pointer to group hier. path for obj */
bool api_ctx_pushed = false; /* Whether API context pushed */
herr_t ret_value = SUCCEED; /* Return value */
void *obj_ptr; /* Pointer to object for ID */
const H5G_name_t *obj_path; /* Pointer to group hier. path for obj */
H5CX_node_t api_ctx = {{0}, NULL}; /* API context node to push */
bool api_ctx_pushed = false; /* Whether API context pushed */
herr_t ret_value = SUCCEED; /* Return value */

FUNC_ENTER_PACKAGE

Expand All @@ -566,7 +573,7 @@ H5G__user_path_test(hid_t obj_id, char *user_path, size_t *user_path_len, unsign
HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "can't get object for ID");

/* Set API context */
if (H5CX_push() < 0)
if (H5CX_push(&api_ctx) < 0)
HGOTO_ERROR(H5E_SYM, H5E_CANTSET, FAIL, "can't set API context");
api_ctx_pushed = true;

Expand Down
15 changes: 8 additions & 7 deletions src/H5Itest.c
Original file line number Diff line number Diff line change
Expand Up @@ -63,17 +63,18 @@
ssize_t
H5I__get_name_test(hid_t id, char *name /*out*/, size_t size, bool *cached)
{
H5VL_object_t *vol_obj; /* Object of id */
H5G_loc_t loc; /* Object location */
bool api_ctx_pushed = false; /* Whether API context pushed */
bool vol_wrapper_set = false; /* Whether the VOL object wrapping context was set up */
size_t name_len = 0; /* Length of name */
ssize_t ret_value = -1; /* Return value */
H5VL_object_t *vol_obj; /* Object of id */
H5G_loc_t loc; /* Object location */
H5CX_node_t api_ctx = {{0}, NULL}; /* API context node to push */
bool api_ctx_pushed = false; /* Whether API context pushed */
bool vol_wrapper_set = false; /* Whether the VOL object wrapping context was set up */
size_t name_len = 0; /* Length of name */
ssize_t ret_value = -1; /* Return value */

FUNC_ENTER_PACKAGE

/* Set API context */
if (H5CX_push() < 0)
if (H5CX_push(&api_ctx) < 0)
HGOTO_ERROR(H5E_SYM, H5E_CANTSET, (-1), "can't set API context");
api_ctx_pushed = true;

Expand Down
Loading

0 comments on commit 97e1ed4

Please sign in to comment.