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.

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

Remove H5B debug checks #4089

Merged
merged 5 commits into from
Mar 9, 2024
Merged

Remove H5B debug checks #4089

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
74c7417
Remove H5B debug checks
derobins Mar 8, 2024
fc5f3de
Merge branch 'develop' into H5B__assert_lives
derobins Mar 8, 2024
c035e3e
Switch assert for real error handling
derobins Mar 9, 2024
060cfc1
Merge branch 'H5B__assert_lives' of https://github.com/derobins/hdf5 …
derobins Mar 9, 2024
d35afe7
Improve error message
derobins Mar 9, 2024
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
10 changes: 0 additions & 10 deletions config/cmake/HDF5DeveloperBuild.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,16 +139,6 @@ if (HDF5_ENABLE_DEBUG_H5T_REF)
list (APPEND HDF5_DEBUG_APIS H5T_REF_DEBUG)
endif ()

# HDF5 module debug definitions for debug code which may add
# considerable amounts of overhead when enabled and is usually
# only useful for specific circumstances rather than general
# developer use.
option (HDF5_ENABLE_DEBUG_H5B "Enable debugging of H5B module" OFF)
mark_as_advanced (HDF5_ENABLE_DEBUG_H5B)
if (HDF5_ENABLE_DEBUG_H5B)
list (APPEND HDF5_DEBUG_APIS H5B_DEBUG)
endif ()

option (HDF5_ENABLE_DEBUG_H5B2 "Enable debugging of H5B2 module" OFF)
mark_as_advanced (HDF5_ENABLE_DEBUG_H5B2)
if (HDF5_ENABLE_DEBUG_H5B2)
Expand Down
2 changes: 1 addition & 1 deletion configure.ac
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2572,7 +2572,7 @@ AC_SUBST([INTERNAL_DEBUG_OUTPUT])
## too specialized or have huge performance hits. These
## are not listed in the "all" packages list.
##
## all_packages="AC,B,B2,D,F,FA,FL,FS,I,MM,O,S,T,Z"
## all_packages="AC,B2,D,F,FA,FL,FS,I,MM,O,S,T,Z"
all_packages="AC,B2,CX,D,F,I,MM,O,S,T,Z"

case "X-$INTERNAL_DEBUG_OUTPUT" in
Expand Down
7 changes: 7 additions & 0 deletions release_docs/RELEASE.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,13 @@ New Features

Configuration:
-------------
- The CMake HDF5_ENABLE_DEBUG_H5B option has been removed

This enabled some additional version-1 B-tree checks. These have been
removed so the option is no longer necessary.

This option was CMake-only and marked as advanced.

- New option for building with static CRT in Windows

The following option has been added:
Expand Down
56 changes: 5 additions & 51 deletions src/H5B.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@

/*-------------------------------------------------------------------------
*
* Created: H5B.c
* Created: H5B.c
*
* Purpose: Implements balanced, sibling-linked, N-ary trees
* Purpose: Implements balanced, sibling-linked, N-ary trees
* capable of storing any type of data with unique key
* values.
*
Expand Down Expand Up @@ -236,9 +236,6 @@ H5B_create(H5F_t *f, const H5B_class_t *type, void *udata, haddr_t *addr_p /*out
*/
if (H5AC_insert_entry(f, H5AC_BT, *addr_p, bt, H5AC__NO_FLAGS_SET) < 0)
HGOTO_ERROR(H5E_BTREE, H5E_CANTINIT, FAIL, "can't add B-tree root node to cache");
#ifdef H5B_DEBUG
H5B__assert(f, *addr_p, shared->type, udata);
#endif

done:
if (ret_value < 0) {
Expand Down Expand Up @@ -399,23 +396,6 @@ H5B__split(H5F_t *f, H5B_ins_ud_t *bt_ud, unsigned idx, void *udata, H5B_ins_ud_
if (H5CX_get_btree_split_ratios(split_ratios) < 0)
HGOTO_ERROR(H5E_BTREE, H5E_CANTGET, FAIL, "can't retrieve B-tree split ratios");

#ifdef H5B_DEBUG
if (H5DEBUG(B)) {
const char *side;

if (!H5_addr_defined(bt_ud->bt->left) && !H5_addr_defined(bt_ud->bt->right))
side = "ONLY";
else if (!H5_addr_defined(bt_ud->bt->right))
side = "RIGHT";
else if (!H5_addr_defined(bt_ud->bt->left))
side = "LEFT";
else
side = "MIDDLE";
fprintf(H5DEBUG(B), "H5B__split: %3u {%5.3f,%5.3f,%5.3f} %6s", shared->two_k, split_ratios[0],
split_ratios[1], split_ratios[2], side);
}
#endif

/*
* Decide how to split the children of the old node among the old node
* and the new node.
Expand All @@ -437,10 +417,6 @@ H5B__split(H5F_t *f, H5B_ins_ud_t *bt_ud, unsigned idx, void *udata, H5B_ins_ud_
else if (idx >= nleft && 0 == nleft)
nleft++;
nright = shared->two_k - nleft;
#ifdef H5B_DEBUG
if (H5DEBUG(B))
fprintf(H5DEBUG(B), " split %3d/%-3d\n", nleft, nright);
#endif

/*
* Create the new B-tree node.
Expand Down Expand Up @@ -649,11 +625,6 @@ H5B_insert(H5F_t *f, const H5B_class_t *type, haddr_t addr, void *udata)
if (H5AC_unprotect(f, H5AC_BT, split_bt_ud.addr, split_bt_ud.bt, split_bt_ud.cache_flags) < 0)
HDONE_ERROR(H5E_BTREE, H5E_CANTUNPROTECT, FAIL, "unable to unprotect new child");

#ifdef H5B_DEBUG
if (ret_value >= 0)
H5B__assert(f, addr, type, udata);
#endif

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

Expand Down Expand Up @@ -944,14 +915,8 @@ H5B__insert_helper(H5F_t *f, H5B_ins_ud_t *bt_ud, const H5B_class_t *type, uint8
#endif /* H5_STRICT_FORMAT_CHECKS */
}
else if (cmp) {
/*
* We couldn't figure out which branch to follow out of this node. THIS
* IS A MAJOR PROBLEM THAT NEEDS TO BE FIXED --rpm.
*/
assert("INTERNAL HDF5 ERROR (contact rpm)" && 0);
#ifdef NDEBUG
HDabort();
derobins marked this conversation as resolved.
Show resolved Hide resolved
#endif /* NDEBUG */
/* We couldn't figure out which branch to follow out of this node */
assert("INTERNAL HDF5 ERROR" && 0);
Copy link
Member Author

Choose a reason for hiding this comment

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

I'll turn this into a real error handler

}
else if (bt->level > 0) {
/*
Expand Down Expand Up @@ -1054,15 +1019,7 @@ H5B__insert_helper(H5F_t *f, H5B_ins_ud_t *bt_ud, const H5B_class_t *type, uint8
if (split_bt_ud->bt) {
H5MM_memcpy(md_key, H5B_NKEY(split_bt_ud->bt, shared, 0), type->sizeof_nkey);
ret_value = H5B_INS_RIGHT;
#ifdef H5B_DEBUG
/*
* The max key in the original left node must be equal to the min key
* in the new node.
*/
cmp = (type->cmp2)(H5B_NKEY(bt, shared, bt->nchildren), udata, H5B_NKEY(split_bt_ud->bt, shared, 0));
assert(0 == cmp);
#endif
} /* end if */
}
else
ret_value = H5B_INS_NOOP;

Expand Down Expand Up @@ -1544,9 +1501,6 @@ H5B_remove(H5F_t *f, const H5B_class_t *type, haddr_t addr, void *udata)
H5B__remove_helper(f, addr, type, 0, lt_key, &lt_key_changed, udata, rt_key, &rt_key_changed))
HGOTO_ERROR(H5E_BTREE, H5E_CANTINIT, FAIL, "unable to remove entry from B-tree");

#ifdef H5B_DEBUG
H5B__assert(f, addr, type, udata);
#endif
done:
FUNC_LEAVE_NOAPI(ret_value)
} /* end H5B_remove() */
Expand Down
23 changes: 8 additions & 15 deletions src/H5Bdbg.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
*
* Created: H5Bdbg.c
*
* Purpose: Debugging routines for B-link tree package.
* Purpose: Debugging routines for B-link tree package
*
*-------------------------------------------------------------------------
*/
Expand All @@ -36,10 +36,9 @@
/*-------------------------------------------------------------------------
* Function: H5B_debug
*
* Purpose: Prints debugging info about a B-tree.
* Purpose: Prints debugging info about a B-tree
*
* Return: Non-negative on success/Negative on failure
*
*-------------------------------------------------------------------------
*/
herr_t
Expand Down Expand Up @@ -132,15 +131,15 @@ H5B_debug(H5F_t *f, haddr_t addr, FILE *stream, int indent, int fwidth, const H5
/*-------------------------------------------------------------------------
* Function: H5B__assert
*
* Purpose: Verifies that the tree is structured correctly.
*
* Return: Success: SUCCEED
* Purpose: Verifies that the tree is structured correctly
*
* Failure: aborts if something is wrong.
* Relies on assert(), so only built when NDEBUG is not set
*
* Return: Success: SUCCEED
* Failure: assert()
*-------------------------------------------------------------------------
*/
#ifdef H5B_DEBUG
#ifndef NDEBUG
herr_t
H5B__assert(H5F_t *f, haddr_t addr, const H5B_class_t *type, void *udata)
{
Expand All @@ -149,7 +148,6 @@ H5B__assert(H5F_t *f, haddr_t addr, const H5B_class_t *type, void *udata)
H5B_shared_t *shared; /* Pointer to shared B-tree info */
H5B_cache_ud_t cache_udata; /* User-data for metadata cache callback */
int ncell, cmp;
static int ncalls = 0;
herr_t status;
herr_t ret_value = SUCCEED; /* Return value */

Expand All @@ -162,11 +160,6 @@ H5B__assert(H5F_t *f, haddr_t addr, const H5B_class_t *type, void *udata)

FUNC_ENTER_PACKAGE

if (0 == ncalls++) {
if (H5DEBUG(B))
fprintf(H5DEBUG(B), "H5B: debugging B-trees (expensive)\n");
} /* end if */

/* Get shared info for B-tree */
if (NULL == (rc_shared = (type->get_shared)(f, udata)))
HGOTO_ERROR(H5E_BTREE, H5E_CANTGET, FAIL, "can't retrieve B-tree's shared ref. count object");
Expand Down Expand Up @@ -257,4 +250,4 @@ H5B__assert(H5F_t *f, haddr_t addr, const H5B_class_t *type, void *udata)
done:
FUNC_LEAVE_NOAPI(ret_value)
} /* end H5B__assert() */
#endif /* H5B_DEBUG */
#endif /* !NDEBUG */
2 changes: 1 addition & 1 deletion src/H5Bpkg.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ H5FL_EXTERN(H5B_t);
/* Package Private Prototypes */
/******************************/
H5_DLL herr_t H5B__node_dest(H5B_t *bt);
#ifdef H5B_DEBUG
#ifndef NDEBUG
herr_t H5B__assert(H5F_t *f, haddr_t addr, const H5B_class_t *type, void *udata);
#endif

Expand Down
10 changes: 0 additions & 10 deletions src/H5Bprivate.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,16 +31,6 @@
/* Library Private Macros */
/**************************/

/*
* Feature: Define this constant if you want to check B-tree consistency
* after each B-tree operation. Note that this slows down the
* library considerably! Debugging the B-tree depends on assert()
* being enabled.
*/
#ifdef NDEBUG
#undef H5B_DEBUG
#endif

/****************************/
/* Library Private Typedefs */
/****************************/
Expand Down
Loading