Skip to content

Conversation

mattjala
Copy link
Contributor

@mattjala mattjala commented Sep 10, 2025

Implement an r-tree data structure in a new module. It has three exposed methods: creation, destruction, and search.

The STR algorithm used during creation is based on the one described here.

This will be used to optimize VDS operations in an upcoming PR.


Important

Introduces an R-tree data structure with creation, search, and destruction methods, integrated into the build system and validated with new tests.

  • R-tree Implementation:
    • Adds new R-tree data structure in H5RT.c with methods H5RT_create(), H5RT_search(), and H5RT_free().
    • Implements STR algorithm for efficient R-tree packing.
  • CMake Integration:
    • Updates CMakeLists.txt to include H5RT.c and related headers.
  • Testing:
    • Adds rtree.c test file to validate R-tree creation and search functionalities.
    • Updates test/CMakeLists.txt to include rtree in test suite.

This description was created by Ellipsis for 1b8ba58. You can customize this summary. It will automatically update as commits are pushed.

byrnHDF
byrnHDF previously approved these changes Sep 22, 2025
Copy link
Contributor

@byrnHDF byrnHDF left a comment

Choose a reason for hiding this comment

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

CMake changes looks good.

@mattjala mattjala requested a review from fortnern September 30, 2025 20:13
src/H5private.h Outdated
void *wrapper_arg,
#endif
const void *a, const void *b
#if !defined(H5_HAVE_WIN32_API) && !defined(H5_HAVE_DARWIN)
Copy link
Collaborator

Choose a reason for hiding this comment

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

A little messy, but seems to be about what we need to do this portably.

It seems you don't need this section of #ifdefs due to this wrapper only being needed for platforms where the ordering is the same (void *, const void *, const void *), so this can probably be simplified to just HDqsort_r_wrapper_func(void *, const void *, const void *).

This section of code looks good, though should probably follow our usual conventions for portability wrappers:

  • All the actual code here (struct definition, HDqsort_r_wrapper_func() and HDqsort_r()) should be moved to H5system.c, just to keep as much non-macro code out of H5private.h as possible. HDqsort_r_wrapper_func() can remain static there, but HDqsort_r() should be made available and probably named differently since we want a single macro called HDqsort_r that just wraps qsort_r() on GNU systems and points to the HDqsort_r() function (named differently) on other systems that need the swapping. Don't worry about adding inline to the functions; it probably won't make much of a difference.
  • There should be a section in H5win32defs.h which defines a HDqsort_r macro and maps it to the differently-named function in H5system.c
  • The macro in this file should be surrounded by an #ifndef HDqsort_r / #endif pair
  • The macro in this file can retain the logic for #ifdef H5_HAVE_DARWIN, where it will map to the function in H5system.c and otherwise just map to qsort_r on GNU systems as it currently does
  • Also note that qsort_r on some BSDs also appears to have the flipped arguments, which is something we'll probably want to deal with. For example, the argument order is flipped up to FreeBSD 13.5, but appears to have been changed to GNU ordering in FreeBSD 14.0 (see https://cgit.freebsd.org/src/commit/?id=af3c78886fd8). More research would be needed on OpenBSD and others.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Based on that commit, I wonder if we can get away with using generics here since we require C11 now.

jhendersonHDF
jhendersonHDF previously approved these changes Oct 1, 2025
Copy link
Collaborator

@jhendersonHDF jhendersonHDF left a comment

Choose a reason for hiding this comment

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

One last minor comment; thanks for the changes to give us a generic qsort_r-like macro! We can make improvements with C11 generics later.

@lrknox lrknox merged commit ac5d525 into HDFGroup:develop Oct 2, 2025
173 of 175 checks passed
@github-project-automation github-project-automation bot moved this from Scheduled/On-Deck to Done in HDF5 - TRIAGE & TRACK Oct 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component - C Library Core C library issues (usually in the src directory)
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants