Skip to content

Commit

Permalink
Improve performance of multiple calls to H5Sget_select_elem_pointlist (
Browse files Browse the repository at this point in the history
…#270) (#277)

* Cache the pointer to the next point to process after the last call to
H5S__get_select_elem_pointlist.  This allows the normal process of
iterating over the points in batches to be much more efficient, as the
library does not need to traverse the entirety of the preceding points
every time the funciton is re-entered.

* Update RELEASE.txt for point selection iteration performance fix.
  • Loading branch information
fortnern authored Jan 22, 2021
1 parent bf6629f commit 6ac30b7
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 7 deletions.
9 changes: 9 additions & 0 deletions release_docs/RELEASE.txt
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,15 @@ New Features

Library
-------
- Improved performance of H5Sget_select_elem_pointlist

Modified library to cache the point after the last block of points
retrieved by H5Sget_select_elem_pointlist, so a subsequent call to the
same function to retrieve the next block of points from the list can
proceed immediately without needing to iterate over the point list.

(NAF - 2021/01/19)

- Add S3 and HDFS VFDs to HDF5 maintenance

Fix windows requirements and java tests. Windows requires CMake 3.13.
Expand Down
5 changes: 5 additions & 0 deletions src/H5Spkg.h
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,11 @@ struct H5S_pnt_node_t {
/* Information about point selection list */
typedef struct {
H5S_pnt_node_t *head; /* Pointer to head of point list */

hsize_t last_idx; /* Index of the point after the last returned from H5S__get_select_elem_pointlist() */
H5S_pnt_node_t *last_idx_pnt; /* Point after the last returned from H5S__get_select_elem_pointlist().
* If we ever add a way to remove points or add points in the middle of
* the pointlist we will need to invalidate these fields. */
} H5S_pnt_list_t;

/* Information about new-style hyperslab spans */
Expand Down
33 changes: 26 additions & 7 deletions src/H5Spoint.c
Original file line number Diff line number Diff line change
Expand Up @@ -618,6 +618,10 @@ H5S_point_copy(H5S_t *dst, const H5S_t *src, hbool_t H5_ATTR_UNUSED share_select
curr = curr->next;
} /* end while */

/* Clear cached iteration point */
dst->select.sel_info.pnt_lst->last_idx = 0;
dst->select.sel_info.pnt_lst->last_idx_pnt = NULL;

done:
if (ret_value < 0 && dst->select.sel_info.pnt_lst) {
/* Traverse the (incomplete?) dst list, freeing all memory */
Expand Down Expand Up @@ -956,6 +960,7 @@ H5S_point_deserialize(H5S_t *space, const uint8_t *buf)
static herr_t
H5S_get_select_elem_pointlist(H5S_t *space, hsize_t startpoint, hsize_t numpoints, hsize_t *buf)
{
const hsize_t endpoint = startpoint + numpoints; /* Index of last point in iteration */
H5S_pnt_node_t *node; /* Point node */
unsigned rank; /* Dataspace rank */

Expand All @@ -967,14 +972,20 @@ H5S_get_select_elem_pointlist(H5S_t *space, hsize_t startpoint, hsize_t numpoint
/* Get the dataspace extent rank */
rank = space->extent.rank;

/* Get the head of the point list */
node = space->select.sel_info.pnt_lst->head;
/* Check for cached point at the correct index */
if(space->select.sel_info.pnt_lst->last_idx_pnt
&& startpoint == space->select.sel_info.pnt_lst->last_idx)
node = space->select.sel_info.pnt_lst->last_idx_pnt;
else {
/* Get the head of the point list */
node = space->select.sel_info.pnt_lst->head;

/* Iterate to the first point to return */
while (node != NULL && startpoint > 0) {
startpoint--;
node = node->next;
} /* end while */
/* Iterate to the first point to return */
while (node != NULL && startpoint > 0) {
startpoint--;
node = node->next;
} /* end while */
} /* end else */

/* Iterate through the node, copying each point's information */
while (node != NULL && numpoints > 0) {
Expand All @@ -984,6 +995,10 @@ H5S_get_select_elem_pointlist(H5S_t *space, hsize_t startpoint, hsize_t numpoint
node = node->next;
} /* end while */

/* Cached next point in iteration */
space->select.sel_info.pnt_lst->last_idx = endpoint;
space->select.sel_info.pnt_lst->last_idx_pnt = node;

FUNC_LEAVE_NOAPI(SUCCEED)
} /* H5S_get_select_elem_pointlist() */

Expand Down Expand Up @@ -1494,6 +1509,10 @@ H5S_point_project_simple(const H5S_t *base_space, H5S_t *new_space, hsize_t *off
} /* end while */
} /* end else */

/* Clear cached iteration point */
new_space->select.sel_info.pnt_lst->last_idx = 0;
new_space->select.sel_info.pnt_lst->last_idx_pnt = NULL;

/* Number of elements selected will be the same */
new_space->select.num_elem = base_space->select.num_elem;

Expand Down

0 comments on commit 6ac30b7

Please sign in to comment.