Merge branch 'master' into cornu/publish_beta_3

.github/workflows/ci.yml

@@ -58,6 +58,9 @@ jobs:
           - config:
               os: ubuntu-22.04
               flags: '-DHIGHFIVE_TEST_BOOST=Off -DCMAKE_CXX_STANDARD=20 -DHIGHFIVE_HAS_CONCEPTS=On'
+          - config:
+              os: ubuntu-24.04
+              flags: '-DHIGHFIVE_TEST_BOOST=Off -DCMAKE_CXX_STANDARD=20 -DHIGHFIVE_HAS_CONCEPTS=On'
 
     steps:
     - uses: actions/checkout@v3
@@ -96,7 +99,7 @@ jobs:
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        hdf5_version : [ hdf5-1_8_23, hdf5-1_10_11, hdf5-1_12_3, hdf5_1.14.4.1 ]
+        hdf5_version : [ hdf5-1_8_23, hdf5-1_10_11, hdf5-1_12_3, hdf5_1.14.4.3 ]
 
     steps:
     - uses: actions/checkout@v3

include/highfive/H5DataSpace.hpp

@@ -19,6 +19,14 @@
 
 namespace HighFive {
 
+namespace detail {
+/// @brief Create a HighFive::DataSpace from an HID, without incrementing the id.
+///
+/// @note This is internal API and subject to change.
+/// @internal
+DataSpace make_data_space(hid_t hid);
+}  // namespace detail
+
 /// \brief Class representing the space (dimensions) of a DataSet
 ///
 /// \code{.cpp}
@@ -254,9 +262,18 @@ class DataSpace: public Object {
   protected:
     DataSpace() = default;
 
+    static DataSpace fromId(hid_t hid) {
+        DataSpace space;
+        space._hid = hid;
+
+        return space;
+    }
+
     friend class Attribute;
     friend class File;
     friend class DataSet;
+
+    friend DataSpace detail::make_data_space(hid_t hid);
 };
 
 }  // namespace HighFive

include/highfive/bits/H5Dataspace_misc.hpp

@@ -21,6 +21,12 @@
 
 namespace HighFive {
 
+namespace detail {
+inline DataSpace make_data_space(hid_t hid) {
+    return DataSpace::fromId(hid);
+}
+}  // namespace detail
+
 inline DataSpace::DataSpace(const std::vector<size_t>& dims)
     : DataSpace(dims.begin(), dims.end()) {}
 

include/highfive/bits/H5Slice_traits.hpp

@@ -162,20 +162,7 @@ class HyperSlab {
     }
 
     DataSpace apply(const DataSpace& space_) const {
-        auto space = space_.clone();
-        for (const auto& sel: selects) {
-            if (sel.op == Op::None) {
-                detail::h5s_select_none(space.getId());
-            } else {
-                detail::h5s_select_hyperslab(space.getId(),
-                                             convert(sel.op),
-                                             sel.offset.empty() ? nullptr : sel.offset.data(),
-                                             sel.stride.empty() ? nullptr : sel.stride.data(),
-                                             sel.count.empty() ? nullptr : sel.count.data(),
-                                             sel.block.empty() ? nullptr : sel.block.data());
-            }
-        }
-        return space;
+        return apply_impl(space_);
     }
 
   private:
@@ -229,6 +216,124 @@ class HyperSlab {
     };
 
     std::vector<Select_> selects;
+
+  protected:
+    DataSpace select_none(const DataSpace& outer_space) const {
+        auto space = outer_space.clone();
+        detail::h5s_select_none(space.getId());
+        return space;
+    }
+
+    void select_hyperslab(DataSpace& space, const Select_& sel) const {
+        detail::h5s_select_hyperslab(space.getId(),
+                                     convert(sel.op),
+                                     sel.offset.empty() ? nullptr : sel.offset.data(),
+                                     sel.stride.empty() ? nullptr : sel.stride.data(),
+                                     sel.count.empty() ? nullptr : sel.count.data(),
+                                     sel.block.empty() ? nullptr : sel.block.data());
+    }
+
+#if H5_VERSION_GE(1, 10, 6)
+    /// The length of a stream of `Op::Or` starting at `begin`.
+    size_t detect_streak(Select_ const* begin, Select_ const* end, Op op) const {
+        assert(op == Op::Or);
+        auto it = std::find_if(begin, end, [op](const Select_& sel) { return sel.op != op; });
+        return static_cast<size_t>(it - begin);
+    }
+
+    DataSpace combine_selections(const DataSpace& left_space,
+                                 Op op,
+                                 const DataSpace& right_space) const {
+        assert(op == Op::Or);
+
+        auto left_type = detail::h5s_get_select_type(left_space.getId());
+        auto right_type = detail::h5s_get_select_type(right_space.getId());
+
+        // Since HDF5 doesn't allow `combine_selections` with a None
+        // selection, we need to avoid the issue:
+        if (left_type == H5S_SEL_NONE) {
+            return right_space;
+        } else if (right_type == H5S_SEL_NONE) {
+            return left_space;
+        } else if (left_type == H5S_SEL_ALL) {
+            return left_space;
+        } else if (right_type == H5S_SEL_ALL) {
+            return right_space;
+        } else {
+            return detail::make_data_space(
+                detail::h5s_combine_select(left_space.getId(), convert(op), right_space.getId()));
+        }
+    }
+
+    /// Reduce a sequence of `Op::Or` efficiently.
+    ///
+    /// The issue is that `H5Sselect_hyperslab` runs in time that linear of the
+    /// number of block in the existing selection. Therefore, a loop that adds
+    /// slab-by-slab has quadratic runtime in the number of slabs.
+    ///
+    /// Fortunately, `H5Scombine_select` doesn't suffer from the same problem.
+    /// However, it's only available in 1.10.6 and newer.
+    ///
+    /// The solution is to use divide-and-conquer to reduce (long) streaks of
+    /// `Op::Or` in what seems to be log-linear time.
+    DataSpace reduce_streak(const DataSpace& outer_space,
+                            Select_ const* begin,
+                            Select_ const* end,
+                            Op op) const {
+        assert(op == Op::Or);
+
+        if (begin == end) {
+            throw std::runtime_error("Broken logic in 'DataSpace::reduce_streak'.");
+        }
+
+        std::ptrdiff_t distance = end - begin;
+        if (distance == 1) {
+            auto space = select_none(outer_space);
+            select_hyperslab(space, *begin);
+            return space;
+        }
+
+        Select_ const* mid = begin + distance / 2;
+        auto right_space = reduce_streak(outer_space, begin, mid, op);
+        auto left_space = reduce_streak(outer_space, mid, end, op);
+
+        return combine_selections(left_space, op, right_space);
+    }
+
+    DataSpace apply_impl(const DataSpace& space_) const {
+        auto space = space_.clone();
+        auto n_selects = selects.size();
+        for (size_t i = 0; i < n_selects; ++i) {
+            auto begin = selects.data() + i;
+            auto end = selects.data() + n_selects;
+
+            auto n_ors = detect_streak(begin, end, Op::Or);
+
+            if (n_ors > 1) {
+                auto right_space = reduce_streak(space_, begin, begin + n_ors, Op::Or);
+                space = combine_selections(space, Op::Or, right_space);
+                i += n_ors - 1;
+            } else if (selects[i].op == Op::None) {
+                detail::h5s_select_none(space.getId());
+            } else {
+                select_hyperslab(space, selects[i]);
+            }
+        }
+        return space;
+    }
+#else
+    DataSpace apply_impl(const DataSpace& space_) const {
+        auto space = space_.clone();
+        for (const auto& sel: selects) {
+            if (sel.op == Op::None) {
+                detail::h5s_select_none(space.getId());
+            } else {
+                select_hyperslab(space, sel);
+            }
+        }
+        return space;
+    }
+#endif
 };
 
 ///

include/highfive/bits/h5s_wrapper.hpp

@@ -1,5 +1,6 @@
 #pragma once
 
+#include <H5Ipublic.h>
 #include <H5Spublic.h>
 namespace HighFive {
 namespace detail {
@@ -110,6 +111,26 @@ inline H5S_class_t h5s_get_simple_extent_type(hid_t space_id) {
     return cls;
 }
 
+inline H5S_sel_type h5s_get_select_type(hid_t space_id) {
+    H5S_sel_type type = H5Sget_select_type(space_id);
+    if (type < 0) {
+        HDF5ErrMapper::ToException<DataSpaceException>("Unable to get type of selection.");
+    }
+
+    return type;
+}
+
+#if H5_VERSION_GE(1, 10, 6)
+inline hid_t h5s_combine_select(hid_t space1_id, H5S_seloper_t op, hid_t space2_id) {
+    auto space_id = H5Scombine_select(space1_id, op, space2_id);
+    if (space_id == H5I_INVALID_HID) {
+        HDF5ErrMapper::ToException<DataSpaceException>("Unable to combine two selections.");
+    }
+
+    return space_id;
+}
+#endif
+
 
 }  // namespace detail
 }  // namespace HighFive

tests/unit/data_generator.hpp

@@ -68,7 +68,7 @@ std::vector<size_t> unravel(size_t flat_index, const Dims& dims) {
 }
 
 template <class Dims>
-static size_t flat_size(const Dims& dims) {
+size_t flat_size(const Dims& dims) {
     size_t n = 1;
     for (auto d: dims) {
         n *= d;