From 26de1a44930caf0fba82e9fad209b89397f3b97f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Fri, 17 May 2024 09:47:59 +0200 Subject: [PATCH 01/39] Remove explicit T[] loadChunk and storeChunk overload --- include/openPMD/Datatype.hpp | 3 ++- include/openPMD/RecordComponent.hpp | 31 ----------------------------- include/openPMD/RecordComponent.tpp | 29 +++++---------------------- 3 files changed, 7 insertions(+), 56 deletions(-) diff --git a/include/openPMD/Datatype.hpp b/include/openPMD/Datatype.hpp index c18b7cd82b..ce870a80a0 100644 --- a/include/openPMD/Datatype.hpp +++ b/include/openPMD/Datatype.hpp @@ -321,7 +321,8 @@ template inline constexpr Datatype determineDatatype(T &&val) { (void)val; // don't need this, it only has a name for Doxygen - using T_stripped = std::remove_cv_t>; + using T_stripped = + std::remove_extent_t>>; if constexpr (auxiliary::IsPointer_v) { return determineDatatype>(); diff --git a/include/openPMD/RecordComponent.hpp b/include/openPMD/RecordComponent.hpp index ebb5a80ca8..b427dc7f7b 100644 --- a/include/openPMD/RecordComponent.hpp +++ b/include/openPMD/RecordComponent.hpp @@ -268,25 +268,6 @@ class RecordComponent : public BaseRecordComponent template void loadChunk(std::shared_ptr data, Offset offset, Extent extent); - /** Load a chunk of data into pre-allocated memory, array version. - * - * @param data Preallocated, contiguous buffer, large enough to load the - * the requested data into it. - * The shared pointer must own and manage the buffer. - * Optimizations might be implemented based on this - * assumption (e.g. skipping the operation if the backend - * is the unique owner). - * The array-based overload helps avoid having to manually - * specify the delete[] destructor (C++17 feature). - * @param offset Offset within the dataset. Set to {0u} for full selection. - * @param extent Extent within the dataset, counted from the offset. - * Set to {-1u} for full selection. - * If offset is non-zero and extent is {-1u} the leftover - * extent in the record component will be selected. - */ - template - void loadChunk(std::shared_ptr data, Offset offset, Extent extent); - /** Load a chunk of data into pre-allocated memory, raw pointer version. * * @param data Preallocated, contiguous buffer, large enough to load the @@ -326,18 +307,6 @@ class RecordComponent : public BaseRecordComponent template void storeChunk(std::shared_ptr data, Offset offset, Extent extent); - /** Store a chunk of data from a chunk of memory, array version. - * - * @param data Preallocated, contiguous buffer, large enough to read the - * the specified data from it. - * The array-based overload helps avoid having to manually - * specify the delete[] destructor (C++17 feature). - * @param offset Offset within the dataset. - * @param extent Extent within the dataset, counted from the offset. - */ - template - void storeChunk(std::shared_ptr data, Offset offset, Extent extent); - /** Store a chunk of data from a chunk of memory, unique pointer version. * * @param data Preallocated, contiguous buffer, large enough to read the diff --git a/include/openPMD/RecordComponent.tpp b/include/openPMD/RecordComponent.tpp index 0a4086e3d8..a3cdfccf97 100644 --- a/include/openPMD/RecordComponent.tpp +++ b/include/openPMD/RecordComponent.tpp @@ -93,10 +93,11 @@ inline std::shared_ptr RecordComponent::loadChunk(Offset o, Extent e) #endif } -template -inline void -RecordComponent::loadChunk(std::shared_ptr data, Offset o, Extent e) +template +inline void RecordComponent::loadChunk( + std::shared_ptr data, Offset o, Extent e) { + using T = std::remove_extent_t; Datatype dtype = determineDatatype(data); if (dtype != getDatatype()) if (!isSameInteger(getDatatype()) && @@ -162,7 +163,7 @@ RecordComponent::loadChunk(std::shared_ptr data, Offset o, Extent e) T value = rc.m_constantValue.get(); - T *raw_ptr = data.get(); + auto raw_ptr = static_cast(data.get()); std::fill(raw_ptr, raw_ptr + numPoints, value); } else @@ -176,16 +177,6 @@ RecordComponent::loadChunk(std::shared_ptr data, Offset o, Extent e) } } -template -inline void RecordComponent::loadChunk( - std::shared_ptr ptr, Offset offset, Extent extent) -{ - loadChunk( - std::static_pointer_cast(std::move(ptr)), - std::move(offset), - std::move(extent)); -} - template inline void RecordComponent::loadChunkRaw(T *ptr, Offset offset, Extent extent) { @@ -233,16 +224,6 @@ RecordComponent::storeChunk(std::unique_ptr data, Offset o, Extent e) UniquePtrWithLambda(std::move(data)), std::move(o), std::move(e)); } -template -inline void -RecordComponent::storeChunk(std::shared_ptr data, Offset o, Extent e) -{ - storeChunk( - std::static_pointer_cast(std::move(data)), - std::move(o), - std::move(e)); -} - template void RecordComponent::storeChunkRaw(T *ptr, Offset offset, Extent extent) { From 445bde77bc589ca15189bfe9aae0a3ba331a3a3e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Fri, 17 May 2024 12:03:31 +0200 Subject: [PATCH 02/39] Add class structure for LoadChunk --- CMakeLists.txt | 1 + include/openPMD/LoadStoreChunk.hpp | 80 ++++++++++++++++++++++++ include/openPMD/auxiliary/TypeTraits.hpp | 14 +++++ src/LoadStoreChunk.cpp | 79 +++++++++++++++++++++++ 4 files changed, 174 insertions(+) create mode 100644 include/openPMD/LoadStoreChunk.hpp create mode 100644 src/LoadStoreChunk.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index c17631c6cb..77643696c8 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -456,6 +456,7 @@ set(CORE_SOURCE src/Format.cpp src/Iteration.cpp src/IterationEncoding.cpp + src/LoadStoreChunk.cpp src/Mesh.cpp src/ParticlePatches.cpp src/ParticleSpecies.cpp diff --git a/include/openPMD/LoadStoreChunk.hpp b/include/openPMD/LoadStoreChunk.hpp new file mode 100644 index 0000000000..b74e282fd0 --- /dev/null +++ b/include/openPMD/LoadStoreChunk.hpp @@ -0,0 +1,80 @@ +#pragma once + +#include "openPMD/Dataset.hpp" +#include "openPMD/Span.hpp" +#include "openPMD/auxiliary/TypeTraits.hpp" +#include "openPMD/auxiliary/UniquePtr.hpp" +#include + +namespace openPMD +{ +class RecordComponent; +template +class TypedConfigureStoreChunk; + +struct MemorySelection +{ + Offset offset; + Extent extent; +}; + +template +class ConfigureStoreChunk +{ +private: + RecordComponent &m_rc; + Offset m_offset; + Extent m_extent; + std::optional m_mem_select; + + ConfigureStoreChunk(RecordComponent &rc); + +public: + using return_type = auxiliary::IfIsVoid_t< + ChildClass, + /*then*/ ConfigureStoreChunk, + /*else*/ ChildClass>; + + auto offset(Offset) && -> return_type &&; + auto extent(Extent) && -> return_type &&; + auto memorySelection(MemorySelection) && -> return_type &&; + + template + auto fromSharedPtr( + std::shared_ptr) && -> TypedConfigureStoreChunk>; + template + auto fromUniquePtr(UniquePtrWithLambda) + && -> TypedConfigureStoreChunk>; + template + auto fromUniquePtr(std::unique_ptr) + && -> TypedConfigureStoreChunk>; + template + auto fromRawPtr(T *data) && -> TypedConfigureStoreChunk>; + template + auto fromContiguousContainer(T_ContiguousContainer &data) && -> + typename std::enable_if_t< + auxiliary::IsContiguousContainer_v, + TypedConfigureStoreChunk< + std::shared_ptr>>; + + template + auto enqueue() && -> DynamicMemoryView; +}; + +template +class TypedConfigureStoreChunk + : public ConfigureStoreChunk> +{ +private: + Ptr_Type m_buffer; + +public: + using parent_t = ConfigureStoreChunk>; + + auto as_parent() && -> parent_t &&; + auto as_parent() & -> parent_t &; + auto as_parent() const & -> parent_t const &; + + auto enqueue() && -> void; +}; +} // namespace openPMD diff --git a/include/openPMD/auxiliary/TypeTraits.hpp b/include/openPMD/auxiliary/TypeTraits.hpp index 526746de89..e7984bdec3 100644 --- a/include/openPMD/auxiliary/TypeTraits.hpp +++ b/include/openPMD/auxiliary/TypeTraits.hpp @@ -197,4 +197,18 @@ namespace detail }; } // namespace detail +template +struct IfIsVoid +{ + using type = Else; +}; + +template +struct IfIsVoid +{ + using type = Then; +}; +template +using IfIsVoid_t = typename IfIsVoid::type; + } // namespace openPMD::auxiliary diff --git a/src/LoadStoreChunk.cpp b/src/LoadStoreChunk.cpp new file mode 100644 index 0000000000..54ab159ad0 --- /dev/null +++ b/src/LoadStoreChunk.cpp @@ -0,0 +1,79 @@ + + +#include "openPMD/LoadStoreChunk.hpp" +#include "openPMD/DatatypeMacros.hpp" +#include "openPMD/RecordComponent.hpp" +#include "openPMD/auxiliary/UniquePtr.hpp" +#include + +namespace openPMD +{ +template +ConfigureStoreChunk::ConfigureStoreChunk(RecordComponent &rc) + : m_rc(rc), m_offset(rc.getDimensionality(), 0), m_extent{rc.getExtent()} +{} + +template +template +auto ConfigureStoreChunk::enqueue() && -> DynamicMemoryView +{ + throw std::runtime_error("Unimplemented!"); +} + +template +template +auto ConfigureStoreChunk::fromSharedPtr( + std::shared_ptr) && -> TypedConfigureStoreChunk> +{ + throw std::runtime_error("Unimplemented!"); +} +template +template +auto ConfigureStoreChunk::fromUniquePtr(UniquePtrWithLambda) + && -> TypedConfigureStoreChunk> +{ + throw std::runtime_error("Unimplemented!"); +} +template +template +auto ConfigureStoreChunk::fromRawPtr( + T *) && -> TypedConfigureStoreChunk> +{ + throw std::runtime_error("Unimplemented!"); +} + +#define INSTANTIATE_METHOD_TEMPLATES(base_class, dtype) \ + template auto base_class::enqueue() && -> DynamicMemoryView; \ + template auto base_class::fromSharedPtr(std::shared_ptr) \ + && -> TypedConfigureStoreChunk>; \ + template auto base_class::fromUniquePtr(UniquePtrWithLambda) \ + && -> TypedConfigureStoreChunk>; \ + template auto base_class::fromRawPtr( \ + dtype *) && -> TypedConfigureStoreChunk>; + +template class ConfigureStoreChunk; + +#define INSTANTIATE_METHOD_TEMPLATES_FOR_BASE(dtype) \ + INSTANTIATE_METHOD_TEMPLATES(ConfigureStoreChunk, dtype) + +OPENPMD_FOREACH_DATASET_DATATYPE(INSTANTIATE_METHOD_TEMPLATES_FOR_BASE) + +#undef INSTANTIATE_METHOD_TEMPLATES_FOR_BASE + +#define INSTANTIATE_TYPED_STORE_CHUNK(dtype) \ + template class TypedConfigureStoreChunk>; \ + INSTANTIATE_METHOD_TEMPLATES( \ + ConfigureStoreChunk>>, \ + dtype) \ + template class TypedConfigureStoreChunk>; \ + INSTANTIATE_METHOD_TEMPLATES( \ + ConfigureStoreChunk< \ + TypedConfigureStoreChunk>>, \ + dtype) + +OPENPMD_FOREACH_DATASET_DATATYPE(INSTANTIATE_TYPED_STORE_CHUNK) + +#undef INSTANTIATE_TYPED_STORE_CHUNK +#undef INSTANTIATE_METHOD_TEMPLATES + +} // namespace openPMD From 79d60c34e85d0729af80f9a62b5bc760898210ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Fri, 17 May 2024 12:15:39 +0200 Subject: [PATCH 03/39] use std::conditional_t knew there was sth like this --- include/openPMD/LoadStoreChunk.hpp | 7 ++++--- include/openPMD/auxiliary/TypeTraits.hpp | 15 --------------- 2 files changed, 4 insertions(+), 18 deletions(-) diff --git a/include/openPMD/LoadStoreChunk.hpp b/include/openPMD/LoadStoreChunk.hpp index b74e282fd0..d9c7a5b77d 100644 --- a/include/openPMD/LoadStoreChunk.hpp +++ b/include/openPMD/LoadStoreChunk.hpp @@ -2,9 +2,10 @@ #include "openPMD/Dataset.hpp" #include "openPMD/Span.hpp" -#include "openPMD/auxiliary/TypeTraits.hpp" #include "openPMD/auxiliary/UniquePtr.hpp" + #include +#include namespace openPMD { @@ -30,8 +31,8 @@ class ConfigureStoreChunk ConfigureStoreChunk(RecordComponent &rc); public: - using return_type = auxiliary::IfIsVoid_t< - ChildClass, + using return_type = std::conditional_t< + std::is_void_v, /*then*/ ConfigureStoreChunk, /*else*/ ChildClass>; diff --git a/include/openPMD/auxiliary/TypeTraits.hpp b/include/openPMD/auxiliary/TypeTraits.hpp index e7984bdec3..5a6cbb13bb 100644 --- a/include/openPMD/auxiliary/TypeTraits.hpp +++ b/include/openPMD/auxiliary/TypeTraits.hpp @@ -196,19 +196,4 @@ namespace detail using type = std::variant<>; }; } // namespace detail - -template -struct IfIsVoid -{ - using type = Else; -}; - -template -struct IfIsVoid -{ - using type = Then; -}; -template -using IfIsVoid_t = typename IfIsVoid::type; - } // namespace openPMD::auxiliary From 1d29837c7795056252f5e894672de077ea9283cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Fri, 17 May 2024 15:03:28 +0200 Subject: [PATCH 04/39] Implement storeChunkSpan() --- include/openPMD/LoadStoreChunk.hpp | 81 ++++++++++++++++++++--- include/openPMD/RecordComponent.hpp | 13 ++++ include/openPMD/RecordComponent.tpp | 47 ++++++++++---- src/LoadStoreChunk.cpp | 99 +++++++++++++++++++++++++++-- src/RecordComponent.cpp | 40 ++++++++++++ 5 files changed, 249 insertions(+), 31 deletions(-) diff --git a/include/openPMD/LoadStoreChunk.hpp b/include/openPMD/LoadStoreChunk.hpp index d9c7a5b77d..f4f67c8fd5 100644 --- a/include/openPMD/LoadStoreChunk.hpp +++ b/include/openPMD/LoadStoreChunk.hpp @@ -1,10 +1,10 @@ #pragma once #include "openPMD/Dataset.hpp" -#include "openPMD/Span.hpp" #include "openPMD/auxiliary/UniquePtr.hpp" #include +#include #include namespace openPMD @@ -12,6 +12,8 @@ namespace openPMD class RecordComponent; template class TypedConfigureStoreChunk; +template +class DynamicMemoryView; struct MemorySelection { @@ -19,26 +21,46 @@ struct MemorySelection Extent extent; }; +namespace internal +{ + struct StoreChunkConfig + { + Offset offset; + Extent extent; + }; + struct StoreChunkConfigFromBuffer + { + Offset offset; + Extent extent; + std::optional memorySelection; + }; +} // namespace internal + template class ConfigureStoreChunk { + friend class RecordComponent; + private: RecordComponent &m_rc; - Offset m_offset; - Extent m_extent; - std::optional m_mem_select; + std::optional m_offset; + std::optional m_extent; ConfigureStoreChunk(RecordComponent &rc); + auto dim() const -> uint8_t; + auto getOffset() const -> Offset; + auto getExtent() const -> Extent; + auto storeChunkConfig() const -> internal::StoreChunkConfig; + public: using return_type = std::conditional_t< std::is_void_v, /*then*/ ConfigureStoreChunk, /*else*/ ChildClass>; - auto offset(Offset) && -> return_type &&; - auto extent(Extent) && -> return_type &&; - auto memorySelection(MemorySelection) && -> return_type &&; + auto offset(Offset) -> return_type &; + auto extent(Extent) -> return_type &; template auto fromSharedPtr( @@ -59,23 +81,62 @@ class ConfigureStoreChunk std::shared_ptr>>; template - auto enqueue() && -> DynamicMemoryView; + auto enqueue() -> DynamicMemoryView; + // definition for this one is in RecordComponent.tpp since it needs the + // definition of class RecordComponent. + template + auto enqueue(F &&createBuffer) -> DynamicMemoryView; }; +// @todo rename as ConfigureStoreChunkFromBuffer template class TypedConfigureStoreChunk : public ConfigureStoreChunk> { +public: + using parent_t = ConfigureStoreChunk>; + private: + friend class ConfigureStoreChunk>; + Ptr_Type m_buffer; + std::optional m_mem_select; + + TypedConfigureStoreChunk(Ptr_Type buffer, parent_t &&); + + auto storeChunkConfig() const -> internal::StoreChunkConfigFromBuffer; public: - using parent_t = ConfigureStoreChunk>; + auto memorySelection(MemorySelection) & -> TypedConfigureStoreChunk &; auto as_parent() && -> parent_t &&; auto as_parent() & -> parent_t &; auto as_parent() const & -> parent_t const &; - auto enqueue() && -> void; + auto enqueue() & -> void; }; + +template +template +auto ConfigureStoreChunk::fromUniquePtr( + std::unique_ptr data) + && -> TypedConfigureStoreChunk> +{ + return fromUniquePtr(UniquePtrWithLambda(std::move(data))); +} +template +template +auto ConfigureStoreChunk::fromContiguousContainer( + T_ContiguousContainer &data) && -> + typename std::enable_if_t< + auxiliary::IsContiguousContainer_v, + TypedConfigureStoreChunk< + std::shared_ptr>> +{ + if (!m_extent.has_value() && dim() == 1) + { + m_extent = Extent{data.size()}; + } + return fromRawPtr(data.data()); +} } // namespace openPMD diff --git a/include/openPMD/RecordComponent.hpp b/include/openPMD/RecordComponent.hpp index b427dc7f7b..a161f453d2 100644 --- a/include/openPMD/RecordComponent.hpp +++ b/include/openPMD/RecordComponent.hpp @@ -22,6 +22,7 @@ #include "openPMD/Dataset.hpp" #include "openPMD/Datatype.hpp" +#include "openPMD/LoadStoreChunk.hpp" #include "openPMD/auxiliary/ShareRaw.hpp" #include "openPMD/auxiliary/TypeTraits.hpp" #include "openPMD/auxiliary/UniquePtr.hpp" @@ -135,6 +136,10 @@ class RecordComponent : public BaseRecordComponent friend class MeshRecordComponent; template friend T &internal::makeOwning(T &self, Series); + template + friend class ConfigureStoreChunk; + template + friend class TypedConfigureStoreChunk; public: enum class Allocation @@ -286,6 +291,8 @@ class RecordComponent : public BaseRecordComponent template void loadChunkRaw(T *data, Offset offset, Extent extent); + ConfigureStoreChunk prepareStoreChunk(); + /** Store a chunk of data from a chunk of memory. * * @param data Preallocated, contiguous buffer, large enough to read the @@ -472,6 +479,12 @@ class RecordComponent : public BaseRecordComponent void storeChunk( auxiliary::WriteBuffer buffer, Datatype datatype, Offset o, Extent e); + template + DynamicMemoryView storeChunkSpan_impl(internal::StoreChunkConfig); + template + DynamicMemoryView storeChunkSpanCreateBuffer_impl( + internal::StoreChunkConfig, F &&createBuffer); + // clang-format off OPENPMD_protected // clang-format on diff --git a/include/openPMD/RecordComponent.tpp b/include/openPMD/RecordComponent.tpp index a3cdfccf97..e9071a250b 100644 --- a/include/openPMD/RecordComponent.tpp +++ b/include/openPMD/RecordComponent.tpp @@ -21,6 +21,8 @@ #pragma once +#include "openPMD/Error.hpp" +#include "openPMD/LoadStoreChunk.hpp" #include "openPMD/RecordComponent.hpp" #include "openPMD/Span.hpp" #include "openPMD/auxiliary/Memory.hpp" @@ -268,6 +270,27 @@ template inline DynamicMemoryView RecordComponent::storeChunk(Offset o, Extent e, F &&createBuffer) { + return prepareStoreChunk() + .offset(std::move(o)) + .extent(std::move(e)) + .enqueue(std::forward(createBuffer)); +} + +template +inline DynamicMemoryView +RecordComponent::storeChunk(Offset offset, Extent extent) +{ + return prepareStoreChunk() + .offset(std::move(offset)) + .extent(std::move(extent)) + .enqueue(); +} + +template +inline DynamicMemoryView RecordComponent::storeChunkSpanCreateBuffer_impl( + internal::StoreChunkConfig cfg, F &&createBuffer) +{ + auto [o, e] = std::move(cfg); verifyChunk(o, e); /* @@ -322,20 +345,6 @@ RecordComponent::storeChunk(Offset o, Extent e, F &&createBuffer) return DynamicMemoryView{std::move(getBufferView), size, *this}; } -template -inline DynamicMemoryView -RecordComponent::storeChunk(Offset offset, Extent extent) -{ - return storeChunk(std::move(offset), std::move(extent), [](size_t size) { -#if (defined(_LIBCPP_VERSION) && _LIBCPP_VERSION < 11000) || \ - (defined(__apple_build_version__) && __clang_major__ < 14) - return std::shared_ptr{new T[size], [](auto *ptr) { delete[] ptr; }}; -#else - return std::shared_ptr< T[] >{ new T[ size ] }; -#endif - }); -} - namespace detail { template @@ -373,4 +382,14 @@ void RecordComponent::verifyChunk(Offset const &o, Extent const &e) const { verifyChunk(determineDatatype(), o, e); } + +// definitions for LoadStoreChunk.hpp +template +template +auto ConfigureStoreChunk::enqueue(F &&createBuffer) + -> DynamicMemoryView +{ + return m_rc.storeChunkSpanCreateBuffer_impl( + storeChunkConfig(), std::forward(createBuffer)); +} } // namespace openPMD diff --git a/src/LoadStoreChunk.cpp b/src/LoadStoreChunk.cpp index 54ab159ad0..05553f0e6e 100644 --- a/src/LoadStoreChunk.cpp +++ b/src/LoadStoreChunk.cpp @@ -1,23 +1,87 @@ #include "openPMD/LoadStoreChunk.hpp" -#include "openPMD/DatatypeMacros.hpp" #include "openPMD/RecordComponent.hpp" +#include "openPMD/Span.hpp" #include "openPMD/auxiliary/UniquePtr.hpp" + +// comment to keep clang-format from reordering +#include "openPMD/DatatypeMacros.hpp" + #include namespace openPMD { template ConfigureStoreChunk::ConfigureStoreChunk(RecordComponent &rc) - : m_rc(rc), m_offset(rc.getDimensionality(), 0), m_extent{rc.getExtent()} + : m_rc(rc) {} +template +auto ConfigureStoreChunk::dim() const -> uint8_t +{ + return m_rc.getDimensionality(); +} + +template +auto ConfigureStoreChunk::getOffset() const -> Offset +{ + if (m_offset.has_value()) + { + return *m_offset; + } + else + { + if (m_rc.joinedDimension().has_value()) + { + return Offset{}; + } + else + { + return Offset(dim(), 0); + } + } +} + +template +auto ConfigureStoreChunk::getExtent() const -> Extent +{ + if (m_extent.has_value()) + { + return *m_extent; + } + else + { + return Extent(dim(), 0); + } +} + +template +auto ConfigureStoreChunk::storeChunkConfig() const + -> internal::StoreChunkConfig +{ + return internal::StoreChunkConfig{getOffset(), getExtent()}; +} + +template +auto ConfigureStoreChunk::extent(Extent extent) -> return_type & +{ + m_extent = std::move(extent); + return *this; +} + +template +auto ConfigureStoreChunk::offset(Offset offset) -> return_type & +{ + m_offset = std::move(offset); + return *this; +} + template template -auto ConfigureStoreChunk::enqueue() && -> DynamicMemoryView +auto ConfigureStoreChunk::enqueue() -> DynamicMemoryView { - throw std::runtime_error("Unimplemented!"); + return m_rc.storeChunkSpan_impl(storeChunkConfig()); } template @@ -42,8 +106,30 @@ auto ConfigureStoreChunk::fromRawPtr( throw std::runtime_error("Unimplemented!"); } +template +TypedConfigureStoreChunk::TypedConfigureStoreChunk( + Ptr_Type buffer, parent_t &&parent) + : parent_t(std::move(parent)), m_buffer(std::move(buffer)) +{} + +template +auto TypedConfigureStoreChunk::as_parent() && -> parent_t && +{ + return std::move(*this); +} +template +auto TypedConfigureStoreChunk::as_parent() & -> parent_t & +{ + return *this; +} +template +auto TypedConfigureStoreChunk::as_parent() const & -> parent_t const & +{ + return *this; +} + #define INSTANTIATE_METHOD_TEMPLATES(base_class, dtype) \ - template auto base_class::enqueue() && -> DynamicMemoryView; \ + template auto base_class::enqueue() -> DynamicMemoryView; \ template auto base_class::fromSharedPtr(std::shared_ptr) \ && -> TypedConfigureStoreChunk>; \ template auto base_class::fromUniquePtr(UniquePtrWithLambda) \ @@ -51,11 +137,10 @@ auto ConfigureStoreChunk::fromRawPtr( template auto base_class::fromRawPtr( \ dtype *) && -> TypedConfigureStoreChunk>; -template class ConfigureStoreChunk; - #define INSTANTIATE_METHOD_TEMPLATES_FOR_BASE(dtype) \ INSTANTIATE_METHOD_TEMPLATES(ConfigureStoreChunk, dtype) +template class ConfigureStoreChunk; OPENPMD_FOREACH_DATASET_DATATYPE(INSTANTIATE_METHOD_TEMPLATES_FOR_BASE) #undef INSTANTIATE_METHOD_TEMPLATES_FOR_BASE diff --git a/src/RecordComponent.cpp b/src/RecordComponent.cpp index 0387268514..b2afbb3bd0 100644 --- a/src/RecordComponent.cpp +++ b/src/RecordComponent.cpp @@ -23,11 +23,15 @@ #include "openPMD/DatatypeHelpers.hpp" #include "openPMD/Error.hpp" #include "openPMD/IO/Format.hpp" +#include "openPMD/LoadStoreChunk.hpp" #include "openPMD/Series.hpp" #include "openPMD/auxiliary/Memory.hpp" #include "openPMD/backend/Attributable.hpp" #include "openPMD/backend/BaseRecord.hpp" +// comment +#include "openPMD/DatatypeMacros.hpp" + #include #include #include @@ -62,6 +66,42 @@ RecordComponent::RecordComponent() : BaseRecordComponent(NoInit()) setData(std::make_shared()); } +ConfigureStoreChunk RecordComponent::prepareStoreChunk() +{ + return ConfigureStoreChunk{*this}; +} + +namespace +{ +#if (defined(_LIBCPP_VERSION) && _LIBCPP_VERSION < 11000) || \ + (defined(__apple_build_version__) && __clang_major__ < 14) + template + auto createSpanBufferFallback(size_t size) -> std::shared_ptr + { + return std::shared_ptr{new T[size], [](auto *ptr) { delete[] ptr; }}; + } +#else + template + auto createSpanBufferFallback(size_t size) -> std::shared_ptr + { + return std::shared_ptr{new T[size]}; + } +#endif +} // namespace + +template +DynamicMemoryView +RecordComponent::storeChunkSpan_impl(internal::StoreChunkConfig cfg) +{ + return storeChunkSpanCreateBuffer_impl( + std::move(cfg), &createSpanBufferFallback); +} +#define OPENPMD_INSTANTIATE(dtype) \ + template DynamicMemoryView RecordComponent::storeChunkSpan_impl( \ + internal::StoreChunkConfig cfg); +OPENPMD_FOREACH_DATASET_DATATYPE(OPENPMD_INSTANTIATE) +#undef OPENPMD_INSTANTIATE + RecordComponent::RecordComponent(NoInit) : BaseRecordComponent(NoInit()) {} From 9549078b592d6be8eebd252b88bbdf322325958b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Fri, 17 May 2024 15:09:58 +0200 Subject: [PATCH 05/39] Remove accidentally compiled bool dataset store from Python APIs --- src/binding/python/RecordComponent.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/binding/python/RecordComponent.cpp b/src/binding/python/RecordComponent.cpp index 5645053f0e..05515c5dc6 100644 --- a/src/binding/python/RecordComponent.cpp +++ b/src/binding/python/RecordComponent.cpp @@ -628,7 +628,7 @@ inline PythonDynamicMemoryView store_chunk_span( std::begin(shape), [&maskIt](std::uint64_t) { return !*(maskIt++); }); - return switchNonVectorType( + return switchDatasetType( r.getDatatype(), r, offset, extent); } From 433f7a93781cca06206a94629333d6316a6971ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Fri, 17 May 2024 16:28:17 +0200 Subject: [PATCH 06/39] Implement for normal storeChunk() --- include/openPMD/LoadStoreChunk.hpp | 91 +++++++++++++--- include/openPMD/RecordComponent.hpp | 6 +- include/openPMD/RecordComponent.tpp | 22 ++-- .../openPMD/benchmark/mpi/MPIBenchmark.hpp | 2 +- src/LoadStoreChunk.cpp | 102 +++++++++++------- src/RecordComponent.cpp | 7 +- 6 files changed, 159 insertions(+), 71 deletions(-) diff --git a/include/openPMD/LoadStoreChunk.hpp b/include/openPMD/LoadStoreChunk.hpp index f4f67c8fd5..65e5355ad0 100644 --- a/include/openPMD/LoadStoreChunk.hpp +++ b/include/openPMD/LoadStoreChunk.hpp @@ -1,6 +1,7 @@ #pragma once #include "openPMD/Dataset.hpp" +#include "openPMD/auxiliary/ShareRawInternal.hpp" #include "openPMD/auxiliary/UniquePtr.hpp" #include @@ -34,19 +35,27 @@ namespace internal Extent extent; std::optional memorySelection; }; + + struct ConfigureStoreChunkData + { + ConfigureStoreChunkData(RecordComponent &); + + RecordComponent &m_rc; + std::optional m_offset; + std::optional m_extent; + }; } // namespace internal template -class ConfigureStoreChunk +class ConfigureStoreChunk : protected internal::ConfigureStoreChunkData { friend class RecordComponent; + template + friend class ConfigureStoreChunk; -private: - RecordComponent &m_rc; - std::optional m_offset; - std::optional m_extent; - +protected: ConfigureStoreChunk(RecordComponent &rc); + ConfigureStoreChunk(ConfigureStoreChunkData &&); auto dim() const -> uint8_t; auto getOffset() const -> Offset; @@ -58,23 +67,29 @@ class ConfigureStoreChunk std::is_void_v, /*then*/ ConfigureStoreChunk, /*else*/ ChildClass>; + template + using normalize_dataset_type = + std::remove_cv_t> const; auto offset(Offset) -> return_type &; auto extent(Extent) -> return_type &; + // @todo rvalue references..? template - auto fromSharedPtr( - std::shared_ptr) && -> TypedConfigureStoreChunk>; + auto fromSharedPtr(std::shared_ptr) + -> TypedConfigureStoreChunk>>; template auto fromUniquePtr(UniquePtrWithLambda) - && -> TypedConfigureStoreChunk>; + -> TypedConfigureStoreChunk< + UniquePtrWithLambda>>; template auto fromUniquePtr(std::unique_ptr) - && -> TypedConfigureStoreChunk>; + -> TypedConfigureStoreChunk< + UniquePtrWithLambda>>; template - auto fromRawPtr(T *data) && -> TypedConfigureStoreChunk>; + auto fromRawPtr(T *data) -> TypedConfigureStoreChunk>; template - auto fromContiguousContainer(T_ContiguousContainer &data) && -> + auto fromContiguousContainer(T_ContiguousContainer &data) -> typename std::enable_if_t< auxiliary::IsContiguousContainer_v, TypedConfigureStoreChunk< @@ -97,7 +112,8 @@ class TypedConfigureStoreChunk using parent_t = ConfigureStoreChunk>; private: - friend class ConfigureStoreChunk>; + template + friend class ConfigureStoreChunk; Ptr_Type m_buffer; std::optional m_mem_select; @@ -113,21 +129,64 @@ class TypedConfigureStoreChunk auto as_parent() & -> parent_t &; auto as_parent() const & -> parent_t const &; - auto enqueue() & -> void; + auto enqueue() -> void; }; +template +template +auto ConfigureStoreChunk::fromSharedPtr(std::shared_ptr data) + -> TypedConfigureStoreChunk>> +{ + if (!data) + { + throw std::runtime_error( + "Unallocated pointer passed during chunk store."); + } + return TypedConfigureStoreChunk>>( + std::static_pointer_cast>(std::move(data)), + {std::move(*this)}); +} +template +template +auto ConfigureStoreChunk::fromUniquePtr(UniquePtrWithLambda data) + -> TypedConfigureStoreChunk>> +{ + if (!data) + { + throw std::runtime_error( + "Unallocated pointer passed during chunk store."); + } + return TypedConfigureStoreChunk< + UniquePtrWithLambda>>( + std::move(data).template static_cast_>(), + {std::move(*this)}); +} +template +template +auto ConfigureStoreChunk::fromRawPtr(T *data) + -> TypedConfigureStoreChunk> +{ + if (!data) + { + throw std::runtime_error( + "Unallocated pointer passed during chunk store."); + } + return TypedConfigureStoreChunk>( + auxiliary::shareRaw(data), {std::move(*this)}); +} + template template auto ConfigureStoreChunk::fromUniquePtr( std::unique_ptr data) - && -> TypedConfigureStoreChunk> + -> TypedConfigureStoreChunk>> { return fromUniquePtr(UniquePtrWithLambda(std::move(data))); } template template auto ConfigureStoreChunk::fromContiguousContainer( - T_ContiguousContainer &data) && -> + T_ContiguousContainer &data) -> typename std::enable_if_t< auxiliary::IsContiguousContainer_v, TypedConfigureStoreChunk< diff --git a/include/openPMD/RecordComponent.hpp b/include/openPMD/RecordComponent.hpp index a161f453d2..c93f0db145 100644 --- a/include/openPMD/RecordComponent.hpp +++ b/include/openPMD/RecordComponent.hpp @@ -476,8 +476,10 @@ class RecordComponent : public BaseRecordComponent */ RecordComponent &makeEmpty(Dataset d); - void storeChunk( - auxiliary::WriteBuffer buffer, Datatype datatype, Offset o, Extent e); + void storeChunk_impl( + auxiliary::WriteBuffer buffer, + Datatype datatype, + internal::StoreChunkConfigFromBuffer); template DynamicMemoryView storeChunkSpan_impl(internal::StoreChunkConfig); diff --git a/include/openPMD/RecordComponent.tpp b/include/openPMD/RecordComponent.tpp index e9071a250b..57b3c2c039 100644 --- a/include/openPMD/RecordComponent.tpp +++ b/include/openPMD/RecordComponent.tpp @@ -31,6 +31,7 @@ #include "openPMD/auxiliary/UniquePtr.hpp" #include +#include #include namespace openPMD @@ -189,17 +190,11 @@ template inline void RecordComponent::storeChunk(std::shared_ptr data, Offset o, Extent e) { - if (!data) - throw std::runtime_error( - "Unallocated pointer passed during chunk store."); - Datatype dtype = determineDatatype(data); - - /* std::static_pointer_cast correctly reference-counts the pointer */ - storeChunk( - auxiliary::WriteBuffer(std::static_pointer_cast(data)), - dtype, - std::move(o), - std::move(e)); + prepareStoreChunk() + .offset(std::move(o)) + .extent(std::move(e)) + .fromSharedPtr(std::move(data)) + .enqueue(); } template @@ -211,11 +206,10 @@ RecordComponent::storeChunk(UniquePtrWithLambda data, Offset o, Extent e) "Unallocated pointer passed during chunk store."); Datatype dtype = determineDatatype<>(data); - storeChunk( + storeChunk_impl( auxiliary::WriteBuffer{std::move(data).template static_cast_()}, dtype, - std::move(o), - std::move(e)); + {std::move(o), std::move(e), std::nullopt}); } template diff --git a/include/openPMD/benchmark/mpi/MPIBenchmark.hpp b/include/openPMD/benchmark/mpi/MPIBenchmark.hpp index 3d6d78c7e4..8eadec41db 100644 --- a/include/openPMD/benchmark/mpi/MPIBenchmark.hpp +++ b/include/openPMD/benchmark/mpi/MPIBenchmark.hpp @@ -239,7 +239,7 @@ MPIBenchmark::runBenchmark(int rootThread) } for (Datatype dt : datatypes) { - switchType>(dt, exec, res, rootThread); + switchDatasetType>(dt, exec, res, rootThread); } return res; diff --git a/src/LoadStoreChunk.cpp b/src/LoadStoreChunk.cpp index 05553f0e6e..c79c7651b2 100644 --- a/src/LoadStoreChunk.cpp +++ b/src/LoadStoreChunk.cpp @@ -1,20 +1,56 @@ #include "openPMD/LoadStoreChunk.hpp" +#include "openPMD/Datatype.hpp" #include "openPMD/RecordComponent.hpp" #include "openPMD/Span.hpp" +#include "openPMD/auxiliary/Memory.hpp" +#include "openPMD/auxiliary/ShareRawInternal.hpp" +#include "openPMD/auxiliary/TypeTraits.hpp" #include "openPMD/auxiliary/UniquePtr.hpp" // comment to keep clang-format from reordering #include "openPMD/DatatypeMacros.hpp" +#include #include namespace openPMD { + +namespace internal +{ + ConfigureStoreChunkData::ConfigureStoreChunkData(RecordComponent &rc) + : m_rc(rc) + {} +} // namespace internal + +namespace +{ + template + auto asWriteBuffer(std::shared_ptr &&ptr) -> auxiliary::WriteBuffer + { + /* std::static_pointer_cast correctly reference-counts the pointer */ + return auxiliary::WriteBuffer( + std::static_pointer_cast(std::move(ptr))); + } + template + auto asWriteBuffer(UniquePtrWithLambda &&ptr) -> auxiliary::WriteBuffer + { + return auxiliary::WriteBuffer{ + std::move(ptr).template static_cast_()}; + } +} // namespace + template ConfigureStoreChunk::ConfigureStoreChunk(RecordComponent &rc) - : m_rc(rc) + : ConfigureStoreChunkData(rc) +{} + +template +ConfigureStoreChunk::ConfigureStoreChunk( + internal::ConfigureStoreChunkData &&data) + : ConfigureStoreChunkData(std::move(data)) {} template @@ -67,14 +103,14 @@ template auto ConfigureStoreChunk::extent(Extent extent) -> return_type & { m_extent = std::move(extent); - return *this; + return *static_cast(this); } template auto ConfigureStoreChunk::offset(Offset offset) -> return_type & { m_offset = std::move(offset); - return *this; + return *static_cast(this); } template @@ -84,28 +120,6 @@ auto ConfigureStoreChunk::enqueue() -> DynamicMemoryView return m_rc.storeChunkSpan_impl(storeChunkConfig()); } -template -template -auto ConfigureStoreChunk::fromSharedPtr( - std::shared_ptr) && -> TypedConfigureStoreChunk> -{ - throw std::runtime_error("Unimplemented!"); -} -template -template -auto ConfigureStoreChunk::fromUniquePtr(UniquePtrWithLambda) - && -> TypedConfigureStoreChunk> -{ - throw std::runtime_error("Unimplemented!"); -} -template -template -auto ConfigureStoreChunk::fromRawPtr( - T *) && -> TypedConfigureStoreChunk> -{ - throw std::runtime_error("Unimplemented!"); -} - template TypedConfigureStoreChunk::TypedConfigureStoreChunk( Ptr_Type buffer, parent_t &&parent) @@ -128,14 +142,25 @@ auto TypedConfigureStoreChunk::as_parent() const & -> parent_t const & return *this; } +template +auto TypedConfigureStoreChunk::storeChunkConfig() const + -> internal::StoreChunkConfigFromBuffer +{ + return internal::StoreChunkConfigFromBuffer{ + this->getOffset(), this->getExtent(), m_mem_select}; +} + +template +auto TypedConfigureStoreChunk::enqueue() -> void +{ + this->m_rc.storeChunk_impl( + asWriteBuffer(std::move(m_buffer)), + determineDatatype>(), + storeChunkConfig()); +} + #define INSTANTIATE_METHOD_TEMPLATES(base_class, dtype) \ - template auto base_class::enqueue() -> DynamicMemoryView; \ - template auto base_class::fromSharedPtr(std::shared_ptr) \ - && -> TypedConfigureStoreChunk>; \ - template auto base_class::fromUniquePtr(UniquePtrWithLambda) \ - && -> TypedConfigureStoreChunk>; \ - template auto base_class::fromRawPtr( \ - dtype *) && -> TypedConfigureStoreChunk>; + template auto base_class::enqueue() -> DynamicMemoryView; #define INSTANTIATE_METHOD_TEMPLATES_FOR_BASE(dtype) \ INSTANTIATE_METHOD_TEMPLATES(ConfigureStoreChunk, dtype) @@ -146,14 +171,19 @@ OPENPMD_FOREACH_DATASET_DATATYPE(INSTANTIATE_METHOD_TEMPLATES_FOR_BASE) #undef INSTANTIATE_METHOD_TEMPLATES_FOR_BASE #define INSTANTIATE_TYPED_STORE_CHUNK(dtype) \ - template class TypedConfigureStoreChunk>; \ + template class TypedConfigureStoreChunk>; \ + template class ConfigureStoreChunk< \ + TypedConfigureStoreChunk>>; \ INSTANTIATE_METHOD_TEMPLATES( \ - ConfigureStoreChunk>>, \ + ConfigureStoreChunk< \ + TypedConfigureStoreChunk>>, \ dtype) \ - template class TypedConfigureStoreChunk>; \ + template class TypedConfigureStoreChunk>; \ + template class ConfigureStoreChunk< \ + TypedConfigureStoreChunk>>; \ INSTANTIATE_METHOD_TEMPLATES( \ ConfigureStoreChunk< \ - TypedConfigureStoreChunk>>, \ + TypedConfigureStoreChunk>>, \ dtype) OPENPMD_FOREACH_DATASET_DATATYPE(INSTANTIATE_TYPED_STORE_CHUNK) diff --git a/src/RecordComponent.cpp b/src/RecordComponent.cpp index b2afbb3bd0..55c97f6431 100644 --- a/src/RecordComponent.cpp +++ b/src/RecordComponent.cpp @@ -499,9 +499,12 @@ void RecordComponent::readBase(bool require_unit_si) } } -void RecordComponent::storeChunk( - auxiliary::WriteBuffer buffer, Datatype dtype, Offset o, Extent e) +void RecordComponent::storeChunk_impl( + auxiliary::WriteBuffer buffer, + Datatype dtype, + internal::StoreChunkConfigFromBuffer cfg) { + auto [o, e, memorySelection] = std::move(cfg); verifyChunk(dtype, o, e); Parameter dWrite; From 037eb70a4566b98c1be54e76c4830bbc43723a62 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Fri, 17 May 2024 18:11:01 +0200 Subject: [PATCH 07/39] StoreChunk completely moved over to new syntax --- include/openPMD/LoadStoreChunk.hpp | 33 ++++++++------ include/openPMD/RecordComponent.tpp | 59 ++++++++++--------------- include/openPMD/auxiliary/Memory.hpp | 9 ++-- include/openPMD/auxiliary/UniquePtr.hpp | 7 +-- src/LoadStoreChunk.cpp | 10 ++--- 5 files changed, 58 insertions(+), 60 deletions(-) diff --git a/include/openPMD/LoadStoreChunk.hpp b/include/openPMD/LoadStoreChunk.hpp index 65e5355ad0..2afbf5067e 100644 --- a/include/openPMD/LoadStoreChunk.hpp +++ b/include/openPMD/LoadStoreChunk.hpp @@ -68,8 +68,7 @@ class ConfigureStoreChunk : protected internal::ConfigureStoreChunkData /*then*/ ConfigureStoreChunk, /*else*/ ChildClass>; template - using normalize_dataset_type = - std::remove_cv_t> const; + using normalize_dataset_type = std::remove_cv_t>; auto offset(Offset) -> return_type &; auto extent(Extent) -> return_type &; @@ -77,7 +76,8 @@ class ConfigureStoreChunk : protected internal::ConfigureStoreChunkData // @todo rvalue references..? template auto fromSharedPtr(std::shared_ptr) - -> TypedConfigureStoreChunk>>; + -> TypedConfigureStoreChunk< + std::shared_ptr const>>; template auto fromUniquePtr(UniquePtrWithLambda) -> TypedConfigureStoreChunk< @@ -87,13 +87,15 @@ class ConfigureStoreChunk : protected internal::ConfigureStoreChunkData -> TypedConfigureStoreChunk< UniquePtrWithLambda>>; template - auto fromRawPtr(T *data) -> TypedConfigureStoreChunk>; + auto + fromRawPtr(T *data) -> TypedConfigureStoreChunk< + std::shared_ptr const>>; template auto fromContiguousContainer(T_ContiguousContainer &data) -> typename std::enable_if_t< auxiliary::IsContiguousContainer_v, - TypedConfigureStoreChunk< - std::shared_ptr>>; + TypedConfigureStoreChunk const>>>; template auto enqueue() -> DynamicMemoryView; @@ -135,15 +137,18 @@ class TypedConfigureStoreChunk template template auto ConfigureStoreChunk::fromSharedPtr(std::shared_ptr data) - -> TypedConfigureStoreChunk>> + -> TypedConfigureStoreChunk< + std::shared_ptr const>> { if (!data) { throw std::runtime_error( "Unallocated pointer passed during chunk store."); } - return TypedConfigureStoreChunk>>( - std::static_pointer_cast>(std::move(data)), + return TypedConfigureStoreChunk< + std::shared_ptr const>>( + std::static_pointer_cast const>( + std::move(data)), {std::move(*this)}); } template @@ -164,14 +169,16 @@ auto ConfigureStoreChunk::fromUniquePtr(UniquePtrWithLambda data) template template auto ConfigureStoreChunk::fromRawPtr(T *data) - -> TypedConfigureStoreChunk> + -> TypedConfigureStoreChunk< + std::shared_ptr const>> { if (!data) { throw std::runtime_error( "Unallocated pointer passed during chunk store."); } - return TypedConfigureStoreChunk>( + return TypedConfigureStoreChunk< + std::shared_ptr const>>( auxiliary::shareRaw(data), {std::move(*this)}); } @@ -189,8 +196,8 @@ auto ConfigureStoreChunk::fromContiguousContainer( T_ContiguousContainer &data) -> typename std::enable_if_t< auxiliary::IsContiguousContainer_v, - TypedConfigureStoreChunk< - std::shared_ptr>> + TypedConfigureStoreChunk const>>> { if (!m_extent.has_value() && dim() == 1) { diff --git a/include/openPMD/RecordComponent.tpp b/include/openPMD/RecordComponent.tpp index 57b3c2c039..0d76c8d5c3 100644 --- a/include/openPMD/RecordComponent.tpp +++ b/include/openPMD/RecordComponent.tpp @@ -201,29 +201,32 @@ template inline void RecordComponent::storeChunk(UniquePtrWithLambda data, Offset o, Extent e) { - if (!data) - throw std::runtime_error( - "Unallocated pointer passed during chunk store."); - Datatype dtype = determineDatatype<>(data); - - storeChunk_impl( - auxiliary::WriteBuffer{std::move(data).template static_cast_()}, - dtype, - {std::move(o), std::move(e), std::nullopt}); + prepareStoreChunk() + .offset(std::move(o)) + .extent(std::move(e)) + .fromUniquePtr(std::move(data)) + .enqueue(); } template inline void RecordComponent::storeChunk(std::unique_ptr data, Offset o, Extent e) { - storeChunk( - UniquePtrWithLambda(std::move(data)), std::move(o), std::move(e)); + prepareStoreChunk() + .offset(std::move(o)) + .extent(std::move(e)) + .fromUniquePtr(std::move(data)) + .enqueue(); } template void RecordComponent::storeChunkRaw(T *ptr, Offset offset, Extent extent) { - storeChunk(auxiliary::shareRaw(ptr), std::move(offset), std::move(extent)); + prepareStoreChunk() + .offset(std::move(offset)) + .extent(std::move(extent)) + .fromRawPtr(ptr) + .enqueue(); } template @@ -231,33 +234,19 @@ inline typename std::enable_if_t< auxiliary::IsContiguousContainer_v> RecordComponent::storeChunk(T_ContiguousContainer &data, Offset o, Extent e) { - uint8_t dim = getDimensionality(); + auto storeChunkConfig = prepareStoreChunk(); - // default arguments - // offset = {0u}: expand to right dim {0u, 0u, ...} - Offset offset = o; - if (o.size() == 1u && o.at(0) == 0u) + auto joined_dim = joinedDimension(); + if (!joined_dim.has_value() && (o.size() != 1 || o.at(0) != 0u)) { - if (joinedDimension().has_value()) - { - offset.clear(); - } - else if (dim > 1u) - { - offset = Offset(dim, 0u); - } + storeChunkConfig.offset(std::move(o)); + } + if (e.size() != 1 || e.at(0) != -1u) + { + storeChunkConfig.extent(std::move(e)); } - // extent = {-1u}: take full size - Extent extent(dim, 1u); - // avoid outsmarting the user: - // - stdlib data container implement 1D -> 1D chunk to write - if (e.size() == 1u && e.at(0) == -1u && dim == 1u) - extent.at(0) = data.size(); - else - extent = e; - - storeChunk(auxiliary::shareRaw(data.data()), offset, extent); + std::move(storeChunkConfig).fromContiguousContainer(data).enqueue(); } template diff --git a/include/openPMD/auxiliary/Memory.hpp b/include/openPMD/auxiliary/Memory.hpp index 66ebb5fd34..62750784bc 100644 --- a/include/openPMD/auxiliary/Memory.hpp +++ b/include/openPMD/auxiliary/Memory.hpp @@ -180,9 +180,10 @@ namespace auxiliary WriteBuffer() : m_buffer(UniquePtrWithLambda()) {} - template - explicit WriteBuffer(Args &&...args) - : m_buffer(std::forward(args)...) + WriteBuffer(std::shared_ptr ptr) : m_buffer(std::move(ptr)) + {} + + WriteBuffer(UniquePtrWithLambda ptr) : m_buffer(std::move(ptr)) {} WriteBuffer(WriteBuffer &&) = default; @@ -196,7 +197,7 @@ namespace auxiliary return *this; } - WriteBuffer const &operator=(UniquePtrWithLambda ptr) + WriteBuffer const &operator=(UniquePtrWithLambda ptr) { m_buffer = std::move(ptr); return *this; diff --git a/include/openPMD/auxiliary/UniquePtr.hpp b/include/openPMD/auxiliary/UniquePtr.hpp index 6a64763b13..622a2b4ac2 100644 --- a/include/openPMD/auxiliary/UniquePtr.hpp +++ b/include/openPMD/auxiliary/UniquePtr.hpp @@ -170,10 +170,11 @@ template UniquePtrWithLambda UniquePtrWithLambda::static_cast_() && { using other_type = std::remove_extent_t; + auto original_ptr = this->release(); return UniquePtrWithLambda{ - static_cast(this->release()), - [deleter = std::move(this->get_deleter())](other_type *ptr) { - deleter(static_cast(ptr)); + static_cast(original_ptr), + [deleter = std::move(this->get_deleter()), original_ptr](other_type *) { + deleter(original_ptr); }}; } } // namespace openPMD diff --git a/src/LoadStoreChunk.cpp b/src/LoadStoreChunk.cpp index c79c7651b2..59e80166f2 100644 --- a/src/LoadStoreChunk.cpp +++ b/src/LoadStoreChunk.cpp @@ -37,8 +37,8 @@ namespace template auto asWriteBuffer(UniquePtrWithLambda &&ptr) -> auxiliary::WriteBuffer { - return auxiliary::WriteBuffer{ - std::move(ptr).template static_cast_()}; + return auxiliary::WriteBuffer( + std::move(ptr).template static_cast_()); } } // namespace @@ -178,12 +178,12 @@ OPENPMD_FOREACH_DATASET_DATATYPE(INSTANTIATE_METHOD_TEMPLATES_FOR_BASE) ConfigureStoreChunk< \ TypedConfigureStoreChunk>>, \ dtype) \ - template class TypedConfigureStoreChunk>; \ + template class TypedConfigureStoreChunk>; \ template class ConfigureStoreChunk< \ - TypedConfigureStoreChunk>>; \ + TypedConfigureStoreChunk>>; \ INSTANTIATE_METHOD_TEMPLATES( \ ConfigureStoreChunk< \ - TypedConfigureStoreChunk>>, \ + TypedConfigureStoreChunk>>, \ dtype) OPENPMD_FOREACH_DATASET_DATATYPE(INSTANTIATE_TYPED_STORE_CHUNK) From 505344418f6a2737ee5dca72ad177ed5cd3e1dc7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Tue, 21 May 2024 10:40:15 +0200 Subject: [PATCH 08/39] Rename TypedConfigureStoreChunk -> ConfigureStoreChunkFromBuffer --- include/openPMD/LoadStoreChunk.hpp | 45 +++++++++++++++-------------- include/openPMD/RecordComponent.hpp | 2 +- src/LoadStoreChunk.cpp | 32 ++++++++++---------- 3 files changed, 42 insertions(+), 37 deletions(-) diff --git a/include/openPMD/LoadStoreChunk.hpp b/include/openPMD/LoadStoreChunk.hpp index 2afbf5067e..aa8d7fb6f1 100644 --- a/include/openPMD/LoadStoreChunk.hpp +++ b/include/openPMD/LoadStoreChunk.hpp @@ -12,7 +12,7 @@ namespace openPMD { class RecordComponent; template -class TypedConfigureStoreChunk; +class ConfigureStoreChunkFromBuffer; template class DynamicMemoryView; @@ -76,26 +76,27 @@ class ConfigureStoreChunk : protected internal::ConfigureStoreChunkData // @todo rvalue references..? template auto fromSharedPtr(std::shared_ptr) - -> TypedConfigureStoreChunk< + -> ConfigureStoreChunkFromBuffer< std::shared_ptr const>>; template auto fromUniquePtr(UniquePtrWithLambda) - -> TypedConfigureStoreChunk< + -> ConfigureStoreChunkFromBuffer< UniquePtrWithLambda>>; template auto fromUniquePtr(std::unique_ptr) - -> TypedConfigureStoreChunk< + -> ConfigureStoreChunkFromBuffer< UniquePtrWithLambda>>; template auto - fromRawPtr(T *data) -> TypedConfigureStoreChunk< + fromRawPtr(T *data) -> ConfigureStoreChunkFromBuffer< std::shared_ptr const>>; template auto fromContiguousContainer(T_ContiguousContainer &data) -> typename std::enable_if_t< auxiliary::IsContiguousContainer_v, - TypedConfigureStoreChunk const>>>; + ConfigureStoreChunkFromBuffer< + std::shared_ptr const>>>; template auto enqueue() -> DynamicMemoryView; @@ -105,13 +106,13 @@ class ConfigureStoreChunk : protected internal::ConfigureStoreChunkData auto enqueue(F &&createBuffer) -> DynamicMemoryView; }; -// @todo rename as ConfigureStoreChunkFromBuffer template -class TypedConfigureStoreChunk - : public ConfigureStoreChunk> +class ConfigureStoreChunkFromBuffer + : public ConfigureStoreChunk> { public: - using parent_t = ConfigureStoreChunk>; + using parent_t = + ConfigureStoreChunk>; private: template @@ -120,12 +121,12 @@ class TypedConfigureStoreChunk Ptr_Type m_buffer; std::optional m_mem_select; - TypedConfigureStoreChunk(Ptr_Type buffer, parent_t &&); + ConfigureStoreChunkFromBuffer(Ptr_Type buffer, parent_t &&); auto storeChunkConfig() const -> internal::StoreChunkConfigFromBuffer; public: - auto memorySelection(MemorySelection) & -> TypedConfigureStoreChunk &; + auto memorySelection(MemorySelection) & -> ConfigureStoreChunkFromBuffer &; auto as_parent() && -> parent_t &&; auto as_parent() & -> parent_t &; @@ -137,7 +138,7 @@ class TypedConfigureStoreChunk template template auto ConfigureStoreChunk::fromSharedPtr(std::shared_ptr data) - -> TypedConfigureStoreChunk< + -> ConfigureStoreChunkFromBuffer< std::shared_ptr const>> { if (!data) @@ -145,7 +146,7 @@ auto ConfigureStoreChunk::fromSharedPtr(std::shared_ptr data) throw std::runtime_error( "Unallocated pointer passed during chunk store."); } - return TypedConfigureStoreChunk< + return ConfigureStoreChunkFromBuffer< std::shared_ptr const>>( std::static_pointer_cast const>( std::move(data)), @@ -154,14 +155,15 @@ auto ConfigureStoreChunk::fromSharedPtr(std::shared_ptr data) template template auto ConfigureStoreChunk::fromUniquePtr(UniquePtrWithLambda data) - -> TypedConfigureStoreChunk>> + -> ConfigureStoreChunkFromBuffer< + UniquePtrWithLambda>> { if (!data) { throw std::runtime_error( "Unallocated pointer passed during chunk store."); } - return TypedConfigureStoreChunk< + return ConfigureStoreChunkFromBuffer< UniquePtrWithLambda>>( std::move(data).template static_cast_>(), {std::move(*this)}); @@ -169,7 +171,7 @@ auto ConfigureStoreChunk::fromUniquePtr(UniquePtrWithLambda data) template template auto ConfigureStoreChunk::fromRawPtr(T *data) - -> TypedConfigureStoreChunk< + -> ConfigureStoreChunkFromBuffer< std::shared_ptr const>> { if (!data) @@ -177,7 +179,7 @@ auto ConfigureStoreChunk::fromRawPtr(T *data) throw std::runtime_error( "Unallocated pointer passed during chunk store."); } - return TypedConfigureStoreChunk< + return ConfigureStoreChunkFromBuffer< std::shared_ptr const>>( auxiliary::shareRaw(data), {std::move(*this)}); } @@ -186,7 +188,8 @@ template template auto ConfigureStoreChunk::fromUniquePtr( std::unique_ptr data) - -> TypedConfigureStoreChunk>> + -> ConfigureStoreChunkFromBuffer< + UniquePtrWithLambda>> { return fromUniquePtr(UniquePtrWithLambda(std::move(data))); } @@ -196,7 +199,7 @@ auto ConfigureStoreChunk::fromContiguousContainer( T_ContiguousContainer &data) -> typename std::enable_if_t< auxiliary::IsContiguousContainer_v, - TypedConfigureStoreChunk const>>> { if (!m_extent.has_value() && dim() == 1) diff --git a/include/openPMD/RecordComponent.hpp b/include/openPMD/RecordComponent.hpp index c93f0db145..7eceb9c778 100644 --- a/include/openPMD/RecordComponent.hpp +++ b/include/openPMD/RecordComponent.hpp @@ -139,7 +139,7 @@ class RecordComponent : public BaseRecordComponent template friend class ConfigureStoreChunk; template - friend class TypedConfigureStoreChunk; + friend class ConfigureStoreChunkFromBuffer; public: enum class Allocation diff --git a/src/LoadStoreChunk.cpp b/src/LoadStoreChunk.cpp index 59e80166f2..ddd13a34e3 100644 --- a/src/LoadStoreChunk.cpp +++ b/src/LoadStoreChunk.cpp @@ -121,29 +121,30 @@ auto ConfigureStoreChunk::enqueue() -> DynamicMemoryView } template -TypedConfigureStoreChunk::TypedConfigureStoreChunk( +ConfigureStoreChunkFromBuffer::ConfigureStoreChunkFromBuffer( Ptr_Type buffer, parent_t &&parent) : parent_t(std::move(parent)), m_buffer(std::move(buffer)) {} template -auto TypedConfigureStoreChunk::as_parent() && -> parent_t && +auto ConfigureStoreChunkFromBuffer::as_parent() && -> parent_t && { return std::move(*this); } template -auto TypedConfigureStoreChunk::as_parent() & -> parent_t & +auto ConfigureStoreChunkFromBuffer::as_parent() & -> parent_t & { return *this; } template -auto TypedConfigureStoreChunk::as_parent() const & -> parent_t const & +auto ConfigureStoreChunkFromBuffer::as_parent() + const & -> parent_t const & { return *this; } template -auto TypedConfigureStoreChunk::storeChunkConfig() const +auto ConfigureStoreChunkFromBuffer::storeChunkConfig() const -> internal::StoreChunkConfigFromBuffer { return internal::StoreChunkConfigFromBuffer{ @@ -151,7 +152,7 @@ auto TypedConfigureStoreChunk::storeChunkConfig() const } template -auto TypedConfigureStoreChunk::enqueue() -> void +auto ConfigureStoreChunkFromBuffer::enqueue() -> void { this->m_rc.storeChunk_impl( asWriteBuffer(std::move(m_buffer)), @@ -170,25 +171,26 @@ OPENPMD_FOREACH_DATASET_DATATYPE(INSTANTIATE_METHOD_TEMPLATES_FOR_BASE) #undef INSTANTIATE_METHOD_TEMPLATES_FOR_BASE -#define INSTANTIATE_TYPED_STORE_CHUNK(dtype) \ - template class TypedConfigureStoreChunk>; \ +#define INSTANTIATE_STORE_CHUNK_FROM_BUFFER(dtype) \ + template class ConfigureStoreChunkFromBuffer< \ + std::shared_ptr>; \ template class ConfigureStoreChunk< \ - TypedConfigureStoreChunk>>; \ + ConfigureStoreChunkFromBuffer>>; \ INSTANTIATE_METHOD_TEMPLATES( \ ConfigureStoreChunk< \ - TypedConfigureStoreChunk>>, \ + ConfigureStoreChunkFromBuffer>>, \ dtype) \ - template class TypedConfigureStoreChunk>; \ + template class ConfigureStoreChunkFromBuffer>; \ template class ConfigureStoreChunk< \ - TypedConfigureStoreChunk>>; \ + ConfigureStoreChunkFromBuffer>>; \ INSTANTIATE_METHOD_TEMPLATES( \ ConfigureStoreChunk< \ - TypedConfigureStoreChunk>>, \ + ConfigureStoreChunkFromBuffer>>, \ dtype) -OPENPMD_FOREACH_DATASET_DATATYPE(INSTANTIATE_TYPED_STORE_CHUNK) +OPENPMD_FOREACH_DATASET_DATATYPE(INSTANTIATE_STORE_CHUNK_FROM_BUFFER) -#undef INSTANTIATE_TYPED_STORE_CHUNK +#undef INSTANTIATE_STORE_CHUNK_FROM_BUFFER #undef INSTANTIATE_METHOD_TEMPLATES } // namespace openPMD From de83ec5d1d357e9d5b04fc628a3362afda3ad48d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Tue, 21 May 2024 11:13:21 +0200 Subject: [PATCH 09/39] Implement memory selection in ADIOS2 --- include/openPMD/Dataset.hpp | 6 ++++++ include/openPMD/IO/ADIOS/ADIOS2File.hpp | 3 +++ include/openPMD/IO/ADIOS/ADIOS2IOHandler.hpp | 12 ++++++++++++ include/openPMD/IO/IOTask.hpp | 2 ++ include/openPMD/LoadStoreChunk.hpp | 6 ------ src/IO/ADIOS/ADIOS2File.cpp | 18 ++++++++++++++---- src/IO/ADIOS/ADIOS2IOHandler.cpp | 3 ++- src/RecordComponent.cpp | 1 + 8 files changed, 40 insertions(+), 11 deletions(-) diff --git a/include/openPMD/Dataset.hpp b/include/openPMD/Dataset.hpp index 0032888541..02feef1859 100644 --- a/include/openPMD/Dataset.hpp +++ b/include/openPMD/Dataset.hpp @@ -34,6 +34,12 @@ namespace openPMD using Extent = std::vector; using Offset = std::vector; +struct MemorySelection +{ + Offset offset; + Extent extent; +}; + class Dataset { friend class RecordComponent; diff --git a/include/openPMD/IO/ADIOS/ADIOS2File.hpp b/include/openPMD/IO/ADIOS/ADIOS2File.hpp index 0bcdaa6131..d9f7250c35 100644 --- a/include/openPMD/IO/ADIOS/ADIOS2File.hpp +++ b/include/openPMD/IO/ADIOS/ADIOS2File.hpp @@ -20,11 +20,13 @@ */ #pragma once +#include "openPMD/Dataset.hpp" #include "openPMD/IO/ADIOS/ADIOS2Auxiliary.hpp" #include "openPMD/IO/AbstractIOHandler.hpp" #include "openPMD/IO/IOTask.hpp" #include "openPMD/IO/InvalidatableFile.hpp" #include "openPMD/config.hpp" +#include #if openPMD_HAVE_ADIOS2 #include @@ -106,6 +108,7 @@ struct BufferedUniquePtrPut std::string name; Offset offset; Extent extent; + std::optional memorySelection; UniquePtrWithLambda data; Datatype dtype = Datatype::UNDEFINED; diff --git a/include/openPMD/IO/ADIOS/ADIOS2IOHandler.hpp b/include/openPMD/IO/ADIOS/ADIOS2IOHandler.hpp index db3162a2da..09cd56ec69 100644 --- a/include/openPMD/IO/ADIOS/ADIOS2IOHandler.hpp +++ b/include/openPMD/IO/ADIOS/ADIOS2IOHandler.hpp @@ -20,6 +20,7 @@ */ #pragma once +#include "openPMD/Dataset.hpp" #include "openPMD/Error.hpp" #include "openPMD/IO/ADIOS/ADIOS2Auxiliary.hpp" #include "openPMD/IO/ADIOS/ADIOS2FilePosition.hpp" @@ -426,6 +427,7 @@ class ADIOS2IOHandlerImpl adios2::Variable verifyDataset( Offset const &offset, Extent const &extent, + std::optional const &memorySelection, adios2::IO &IO, std::string const &varName) { @@ -507,6 +509,16 @@ class ADIOS2IOHandlerImpl var.SetSelection( {adios2::Dims(offset.begin(), offset.end()), adios2::Dims(extent.begin(), extent.end())}); + if (memorySelection.has_value()) + { + var.SetMemorySelection( + {adios2::Dims( + memorySelection->offset.begin(), + memorySelection->offset.end()), + adios2::Dims( + memorySelection->extent.begin(), + memorySelection->extent.end())}); + } return var; } diff --git a/include/openPMD/IO/IOTask.hpp b/include/openPMD/IO/IOTask.hpp index 731372f9e1..24ba4caa14 100644 --- a/include/openPMD/IO/IOTask.hpp +++ b/include/openPMD/IO/IOTask.hpp @@ -33,6 +33,7 @@ #include #include #include +#include #include #include #include @@ -436,6 +437,7 @@ struct OPENPMDAPI_EXPORT Parameter Extent extent = {}; Offset offset = {}; + std::optional memorySelection = std::nullopt; Datatype dtype = Datatype::UNDEFINED; auxiliary::WriteBuffer data; }; diff --git a/include/openPMD/LoadStoreChunk.hpp b/include/openPMD/LoadStoreChunk.hpp index aa8d7fb6f1..23207762e4 100644 --- a/include/openPMD/LoadStoreChunk.hpp +++ b/include/openPMD/LoadStoreChunk.hpp @@ -16,12 +16,6 @@ class ConfigureStoreChunkFromBuffer; template class DynamicMemoryView; -struct MemorySelection -{ - Offset offset; - Extent extent; -}; - namespace internal { struct StoreChunkConfig diff --git a/src/IO/ADIOS/ADIOS2File.cpp b/src/IO/ADIOS/ADIOS2File.cpp index ee0c1a9062..55b3d8c982 100644 --- a/src/IO/ADIOS/ADIOS2File.cpp +++ b/src/IO/ADIOS/ADIOS2File.cpp @@ -27,6 +27,7 @@ #include "openPMD/auxiliary/StringManip.hpp" #include +#include #if openPMD_USE_VERIFY #define VERIFY(CONDITION, TEXT) \ @@ -59,8 +60,8 @@ void DatasetReader::call( adios2::Engine &engine, std::string const &fileName) { - adios2::Variable var = - impl->verifyDataset(bp.param.offset, bp.param.extent, IO, bp.name); + adios2::Variable var = impl->verifyDataset( + bp.param.offset, bp.param.extent, std::nullopt, IO, bp.name); if (!var) { throw std::runtime_error( @@ -89,7 +90,11 @@ void WriteDataset::call(ADIOS2File &ba, detail::BufferedPut &bp) auto ptr = static_cast(arg.get()); adios2::Variable var = ba.m_impl->verifyDataset( - bp.param.offset, bp.param.extent, ba.m_IO, bp.name); + bp.param.offset, + bp.param.extent, + bp.param.memorySelection, + ba.m_IO, + bp.name); ba.getEngine().Put(var, ptr); } @@ -101,6 +106,7 @@ void WriteDataset::call(ADIOS2File &ba, detail::BufferedPut &bp) bput.name = std::move(bp.name); bput.offset = std::move(bp.param.offset); bput.extent = std::move(bp.param.extent); + bput.memorySelection = std::move(bp.param.memorySelection); /* * Note: Moving is required here since it's a unique_ptr. * std::forward<>() would theoretically work, but it @@ -147,7 +153,11 @@ struct RunUniquePtrPut { auto ptr = static_cast(bufferedPut.data.get()); adios2::Variable var = ba.m_impl->verifyDataset( - bufferedPut.offset, bufferedPut.extent, ba.m_IO, bufferedPut.name); + bufferedPut.offset, + bufferedPut.extent, + bufferedPut.memorySelection, + ba.m_IO, + bufferedPut.name); ba.getEngine().Put(var, ptr); } diff --git a/src/IO/ADIOS/ADIOS2IOHandler.cpp b/src/IO/ADIOS/ADIOS2IOHandler.cpp index bdbd43325a..0b4cc09107 100644 --- a/src/IO/ADIOS/ADIOS2IOHandler.cpp +++ b/src/IO/ADIOS/ADIOS2IOHandler.cpp @@ -40,6 +40,7 @@ #include #include #include +#include #include #include #include @@ -1110,7 +1111,7 @@ namespace detail auto &IO = ba.m_IO; auto &engine = ba.getEngine(); adios2::Variable variable = impl->verifyDataset( - params.offset, params.extent, IO, varName); + params.offset, params.extent, std::nullopt, IO, varName); adios2::Dims offset(params.offset.begin(), params.offset.end()); adios2::Dims extent(params.extent.begin(), params.extent.end()); variable.SetSelection({std::move(offset), std::move(extent)}); diff --git a/src/RecordComponent.cpp b/src/RecordComponent.cpp index 55c97f6431..d5fd894732 100644 --- a/src/RecordComponent.cpp +++ b/src/RecordComponent.cpp @@ -510,6 +510,7 @@ void RecordComponent::storeChunk_impl( Parameter dWrite; dWrite.offset = std::move(o); dWrite.extent = std::move(e); + dWrite.memorySelection = memorySelection; dWrite.dtype = dtype; /* std::static_pointer_cast correctly reference-counts the pointer */ dWrite.data = std::move(buffer); From ae0ec914879081580ca0fa02dcc5f3a8f12c3664 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Tue, 21 May 2024 11:14:55 +0200 Subject: [PATCH 10/39] Throw errors in JSON/HDF5 when supplying memory selection --- src/IO/HDF5/HDF5IOHandler.cpp | 7 +++++++ src/IO/JSON/JSONIOHandlerImpl.cpp | 7 +++++++ 2 files changed, 14 insertions(+) diff --git a/src/IO/HDF5/HDF5IOHandler.cpp b/src/IO/HDF5/HDF5IOHandler.cpp index 32a9e80d74..efe8bd441b 100644 --- a/src/IO/HDF5/HDF5IOHandler.cpp +++ b/src/IO/HDF5/HDF5IOHandler.cpp @@ -1533,6 +1533,13 @@ void HDF5IOHandlerImpl::writeDataset( "[HDF5] Writing into a dataset in a file opened as read only is " "not possible."); + if (parameters.memorySelection.has_value()) + { + throw error::OperationUnsupportedInBackend( + "HDF5", + "Non-contiguous memory selections not supported in HDF5 backend."); + } + auto res = getFile(writable); File file = res ? res.value() : getFile(writable->parent).value(); diff --git a/src/IO/JSON/JSONIOHandlerImpl.cpp b/src/IO/JSON/JSONIOHandlerImpl.cpp index e06aa36ed8..b21aba2d34 100644 --- a/src/IO/JSON/JSONIOHandlerImpl.cpp +++ b/src/IO/JSON/JSONIOHandlerImpl.cpp @@ -873,6 +873,13 @@ void JSONIOHandlerImpl::writeDataset( access::write(m_handler->m_backendAccess), "[JSON] Cannot write data in read-only mode."); + if (parameters.memorySelection.has_value()) + { + throw error::OperationUnsupportedInBackend( + "JSON", + "Non-contiguous memory selections not supported in JSON backend."); + } + auto pos = setAndGetFilePosition(writable); auto file = refreshFileFromParent(writable); auto &j = obtainJsonContents(writable); From 1d7cffd500d19ebbc70ee3aeeec5dd92418e3d8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Tue, 21 May 2024 16:33:32 +0200 Subject: [PATCH 11/39] Test memory selection --- include/openPMD/IO/ADIOS/ADIOS2IOHandler.hpp | 26 ++++++++ include/openPMD/IO/ADIOS/macros.hpp | 20 +++++++ include/openPMD/LoadStoreChunk.hpp | 2 +- src/LoadStoreChunk.cpp | 9 +++ test/ParallelIOTest.cpp | 63 +++++++++++++++++++- 5 files changed, 117 insertions(+), 3 deletions(-) diff --git a/include/openPMD/IO/ADIOS/ADIOS2IOHandler.hpp b/include/openPMD/IO/ADIOS/ADIOS2IOHandler.hpp index 09cd56ec69..23bed6a50c 100644 --- a/include/openPMD/IO/ADIOS/ADIOS2IOHandler.hpp +++ b/include/openPMD/IO/ADIOS/ADIOS2IOHandler.hpp @@ -24,6 +24,7 @@ #include "openPMD/Error.hpp" #include "openPMD/IO/ADIOS/ADIOS2Auxiliary.hpp" #include "openPMD/IO/ADIOS/ADIOS2FilePosition.hpp" +#include "openPMD/IO/ADIOS/macros.hpp" #include "openPMD/IO/AbstractIOHandler.hpp" #include "openPMD/IO/AbstractIOHandlerImpl.hpp" #include "openPMD/IO/AbstractIOHandlerImplCommon.hpp" @@ -509,6 +510,7 @@ class ADIOS2IOHandlerImpl var.SetSelection( {adios2::Dims(offset.begin(), offset.end()), adios2::Dims(extent.begin(), extent.end())}); + if (memorySelection.has_value()) { var.SetMemorySelection( @@ -518,7 +520,30 @@ class ADIOS2IOHandlerImpl adios2::Dims( memorySelection->extent.begin(), memorySelection->extent.end())}); + if constexpr (!CanTheMemorySelectionBeReset) + { + if (!printedWarningsAlready.memorySelection) + { + std::cerr + << "[Warning] Using a version of ADIOS2 that cannot " + "reset memory selections on a variable, once " + "specified. When using memory selections, then " + "please specify it explicitly on all storeChunk() " + "calls. Further info: " + "https://github.com/ornladios/ADIOS2/pull/4169." + << std::endl; + printedWarningsAlready.memorySelection = true; + } + } } + else + { + if constexpr (CanTheMemorySelectionBeReset) + { + var.SetMemorySelection(); + } + } + return var; } @@ -526,6 +551,7 @@ class ADIOS2IOHandlerImpl { bool noGroupBased = false; bool blosc2bp5 = false; + bool memorySelection = false; } printedWarningsAlready; }; // ADIOS2IOHandlerImpl diff --git a/include/openPMD/IO/ADIOS/macros.hpp b/include/openPMD/IO/ADIOS/macros.hpp index 8618573713..8bd7ad9f62 100644 --- a/include/openPMD/IO/ADIOS/macros.hpp +++ b/include/openPMD/IO/ADIOS/macros.hpp @@ -34,6 +34,26 @@ #define openPMD_HAVE_ADIOS2_BP5 0 #endif +namespace detail +{ +template +struct CanTheMemorySelectionBeReset +{ + static constexpr bool value = false; +}; + +template +struct CanTheMemorySelectionBeReset< + Variable, + decltype(std::declval().SetMemorySelection())> +{ + static constexpr bool value = true; +}; +} // namespace detail + +constexpr bool CanTheMemorySelectionBeReset = + detail::CanTheMemorySelectionBeReset>::value; + #else #define openPMD_HAS_ADIOS_2_8 0 diff --git a/include/openPMD/LoadStoreChunk.hpp b/include/openPMD/LoadStoreChunk.hpp index 23207762e4..dbc4a08f0c 100644 --- a/include/openPMD/LoadStoreChunk.hpp +++ b/include/openPMD/LoadStoreChunk.hpp @@ -120,7 +120,7 @@ class ConfigureStoreChunkFromBuffer auto storeChunkConfig() const -> internal::StoreChunkConfigFromBuffer; public: - auto memorySelection(MemorySelection) & -> ConfigureStoreChunkFromBuffer &; + auto memorySelection(MemorySelection) -> ConfigureStoreChunkFromBuffer &; auto as_parent() && -> parent_t &&; auto as_parent() & -> parent_t &; diff --git a/src/LoadStoreChunk.cpp b/src/LoadStoreChunk.cpp index ddd13a34e3..89712b728e 100644 --- a/src/LoadStoreChunk.cpp +++ b/src/LoadStoreChunk.cpp @@ -13,6 +13,7 @@ #include "openPMD/DatatypeMacros.hpp" #include +#include #include namespace openPMD @@ -151,6 +152,14 @@ auto ConfigureStoreChunkFromBuffer::storeChunkConfig() const this->getOffset(), this->getExtent(), m_mem_select}; } +template +auto ConfigureStoreChunkFromBuffer::memorySelection( + MemorySelection sel) -> ConfigureStoreChunkFromBuffer & +{ + this->m_mem_select = std::make_optional(std::move(sel)); + return *this; +} + template auto ConfigureStoreChunkFromBuffer::enqueue() -> void { diff --git a/test/ParallelIOTest.cpp b/test/ParallelIOTest.cpp index c6d90d773e..de415b450e 100644 --- a/test/ParallelIOTest.cpp +++ b/test/ParallelIOTest.cpp @@ -429,13 +429,42 @@ void available_chunks_test(std::string const &file_ending) } )END"; - std::vector data{2, 4, 6, 8}; + std::vector xdata{2, 4, 6, 8}; + std::vector ydata{0, 0, 0, 0, 0, // + 0, 1, 2, 3, 0, // + 0, 4, 5, 6, 0, // + 0, 7, 8, 9, 0, // + 0, 0, 0, 0, 0}; + std::vector ydata_firstandlastrow{-1, -1, -1}; { Series write(name, Access::CREATE, MPI_COMM_WORLD, parameters.str()); Iteration it0 = write.iterations[0]; auto E_x = it0.meshes["E"]["x"]; E_x.resetDataset({Datatype::INT, {mpi_size, 4}}); - E_x.storeChunk(data, {mpi_rank, 0}, {1, 4}); + E_x.storeChunk(xdata, {mpi_rank, 0}, {1, 4}); + auto E_y = it0.meshes["E"]["y"]; + E_y.resetDataset({Datatype::INT, {5, 3ul * mpi_size}}); + E_y.prepareStoreChunk() + .fromContiguousContainer(ydata_firstandlastrow) + .offset({0, 3ul * mpi_rank}) + .extent({1, 3}) + .enqueue(); + E_y.prepareStoreChunk() + .offset({1, 3ul * mpi_rank}) + .extent({3, 3}) + .fromContiguousContainer(ydata) + .memorySelection({{1, 1}, {5, 5}}) + .enqueue(); + // if condition checks if this PR is available in ADIOS2: + // https://github.com/ornladios/ADIOS2/pull/4169 + if constexpr (CanTheMemorySelectionBeReset) + { + E_y.prepareStoreChunk() + .fromContiguousContainer(ydata_firstandlastrow) + .offset({4, 3ul * mpi_rank}) + .extent({1, 3}) + .enqueue(); + } it0.close(); } @@ -467,6 +496,36 @@ void available_chunks_test(std::string const &file_ending) { REQUIRE(ranks[i] == i); } + + auto E_y = it0.meshes["E"]["y"]; + auto width = E_y.getExtent()[1]; + auto first_row = E_y.loadChunk({0, 0}, {1, width}); + auto middle_rows = E_y.loadChunk({1, 0}, {3, width}); + auto last_row = E_y.loadChunk({4, 0}, {1, width}); + read.flush(); + + for (auto row : [&]() -> std::vector *> { + if constexpr (CanTheMemorySelectionBeReset) + { + return {&first_row, &last_row}; + } + else + { + return {&first_row}; + } + }()) + { + for (size_t i = 0; i < width; ++i) + { + REQUIRE(row->get()[i] == -1); + } + } + for (size_t i = 0; i < width * 3; ++i) + { + size_t row = i / width; + int required_value = row * 3 + (i % 3) + 1; + REQUIRE(middle_rows.get()[i] == required_value); + } } } From 4d1091d93ab9a2c4cce3719ba7c4f94c8dd381a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Wed, 22 May 2024 11:16:17 +0200 Subject: [PATCH 12/39] Directly reset memory selection after applying --- include/openPMD/IO/ADIOS/ADIOS2IOHandler.hpp | 22 -------------- src/IO/ADIOS/ADIOS2File.cpp | 32 ++++++++++++++++++++ 2 files changed, 32 insertions(+), 22 deletions(-) diff --git a/include/openPMD/IO/ADIOS/ADIOS2IOHandler.hpp b/include/openPMD/IO/ADIOS/ADIOS2IOHandler.hpp index 23bed6a50c..30b703f917 100644 --- a/include/openPMD/IO/ADIOS/ADIOS2IOHandler.hpp +++ b/include/openPMD/IO/ADIOS/ADIOS2IOHandler.hpp @@ -520,28 +520,6 @@ class ADIOS2IOHandlerImpl adios2::Dims( memorySelection->extent.begin(), memorySelection->extent.end())}); - if constexpr (!CanTheMemorySelectionBeReset) - { - if (!printedWarningsAlready.memorySelection) - { - std::cerr - << "[Warning] Using a version of ADIOS2 that cannot " - "reset memory selections on a variable, once " - "specified. When using memory selections, then " - "please specify it explicitly on all storeChunk() " - "calls. Further info: " - "https://github.com/ornladios/ADIOS2/pull/4169." - << std::endl; - printedWarningsAlready.memorySelection = true; - } - } - } - else - { - if constexpr (CanTheMemorySelectionBeReset) - { - var.SetMemorySelection(); - } } return var; diff --git a/src/IO/ADIOS/ADIOS2File.cpp b/src/IO/ADIOS/ADIOS2File.cpp index 55b3d8c982..f3d9500f0d 100644 --- a/src/IO/ADIOS/ADIOS2File.cpp +++ b/src/IO/ADIOS/ADIOS2File.cpp @@ -23,6 +23,7 @@ #include "openPMD/Error.hpp" #include "openPMD/IO/ADIOS/ADIOS2IOHandler.hpp" #include "openPMD/IO/AbstractIOHandler.hpp" +#include "openPMD/IO/ADIOS/macros.hpp" #include "openPMD/auxiliary/Environment.hpp" #include "openPMD/auxiliary/StringManip.hpp" @@ -75,6 +76,12 @@ void DatasetReader::call( template inline constexpr bool always_false_v = false; +static constexpr char const *warningMemorySelection = + "[Warning] Using a version of ADIOS2 that cannot reset memory selections " + "on a variable, once specified. When using memory selections, then please " + "specify it explicitly on all storeChunk() calls. Further info: " + "https://github.com/ornladios/ADIOS2/pull/4169."; + template void WriteDataset::call(ADIOS2File &ba, detail::BufferedPut &bp) { @@ -97,6 +104,19 @@ void WriteDataset::call(ADIOS2File &ba, detail::BufferedPut &bp) bp.name); ba.getEngine().Put(var, ptr); + if (bp.param.memorySelection.has_value()) + { + if constexpr (CanTheMemorySelectionBeReset) + { + var.SetMemorySelection(); + } + else if (!ba.m_impl->printedWarningsAlready.memorySelection) + { + std::cerr << warningMemorySelection << std::endl; + ba.m_impl->printedWarningsAlready.memorySelection = + true; + } + } } else if constexpr (std::is_same_v< ptr_type, @@ -159,6 +179,18 @@ struct RunUniquePtrPut ba.m_IO, bufferedPut.name); ba.getEngine().Put(var, ptr); + if (bufferedPut.memorySelection.has_value()) + { + if constexpr (CanTheMemorySelectionBeReset) + { + var.SetMemorySelection(); + } + else if (!ba.m_impl->printedWarningsAlready.memorySelection) + { + std::cerr << warningMemorySelection << std::endl; + ba.m_impl->printedWarningsAlready.memorySelection = true; + } + } } static constexpr char const *errorMsg = "RunUniquePtrPut"; From 2055bce18b2b7c4138265d38914fcd8a26cf1715 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Wed, 22 May 2024 11:41:00 +0200 Subject: [PATCH 13/39] Rename in preparation for storeChunk adaptations --- include/openPMD/LoadStoreChunk.hpp | 98 ++++------------------------- include/openPMD/LoadStoreChunk.tpp | 80 +++++++++++++++++++++++ include/openPMD/RecordComponent.hpp | 4 +- include/openPMD/RecordComponent.tpp | 16 ++--- src/LoadStoreChunk.cpp | 34 +++++----- src/RecordComponent.cpp | 4 +- test/ParallelIOTest.cpp | 6 +- 7 files changed, 125 insertions(+), 117 deletions(-) create mode 100644 include/openPMD/LoadStoreChunk.tpp diff --git a/include/openPMD/LoadStoreChunk.hpp b/include/openPMD/LoadStoreChunk.hpp index dbc4a08f0c..882b127d05 100644 --- a/include/openPMD/LoadStoreChunk.hpp +++ b/include/openPMD/LoadStoreChunk.hpp @@ -41,15 +41,15 @@ namespace internal } // namespace internal template -class ConfigureStoreChunk : protected internal::ConfigureStoreChunkData +class ConfigureLoadStore : protected internal::ConfigureStoreChunkData { friend class RecordComponent; template - friend class ConfigureStoreChunk; + friend class ConfigureLoadStore; protected: - ConfigureStoreChunk(RecordComponent &rc); - ConfigureStoreChunk(ConfigureStoreChunkData &&); + ConfigureLoadStore(RecordComponent &rc); + ConfigureLoadStore(ConfigureStoreChunkData &&); auto dim() const -> uint8_t; auto getOffset() const -> Offset; @@ -59,7 +59,7 @@ class ConfigureStoreChunk : protected internal::ConfigureStoreChunkData public: using return_type = std::conditional_t< std::is_void_v, - /*then*/ ConfigureStoreChunk, + /*then*/ ConfigureLoadStore, /*else*/ ChildClass>; template using normalize_dataset_type = std::remove_cv_t>; @@ -93,24 +93,24 @@ class ConfigureStoreChunk : protected internal::ConfigureStoreChunkData typename T_ContiguousContainer::value_type> const>>>; template - auto enqueue() -> DynamicMemoryView; + [[nodiscard]] auto enqueueStore() -> DynamicMemoryView; // definition for this one is in RecordComponent.tpp since it needs the // definition of class RecordComponent. template - auto enqueue(F &&createBuffer) -> DynamicMemoryView; + [[nodiscard]] auto enqueueStore(F &&createBuffer) -> DynamicMemoryView; }; template class ConfigureStoreChunkFromBuffer - : public ConfigureStoreChunk> + : public ConfigureLoadStore> { public: using parent_t = - ConfigureStoreChunk>; + ConfigureLoadStore>; private: template - friend class ConfigureStoreChunk; + friend class ConfigureLoadStore; Ptr_Type m_buffer; std::optional m_mem_select; @@ -126,80 +126,8 @@ class ConfigureStoreChunkFromBuffer auto as_parent() & -> parent_t &; auto as_parent() const & -> parent_t const &; - auto enqueue() -> void; + auto enqueueStore() -> void; }; - -template -template -auto ConfigureStoreChunk::fromSharedPtr(std::shared_ptr data) - -> ConfigureStoreChunkFromBuffer< - std::shared_ptr const>> -{ - if (!data) - { - throw std::runtime_error( - "Unallocated pointer passed during chunk store."); - } - return ConfigureStoreChunkFromBuffer< - std::shared_ptr const>>( - std::static_pointer_cast const>( - std::move(data)), - {std::move(*this)}); -} -template -template -auto ConfigureStoreChunk::fromUniquePtr(UniquePtrWithLambda data) - -> ConfigureStoreChunkFromBuffer< - UniquePtrWithLambda>> -{ - if (!data) - { - throw std::runtime_error( - "Unallocated pointer passed during chunk store."); - } - return ConfigureStoreChunkFromBuffer< - UniquePtrWithLambda>>( - std::move(data).template static_cast_>(), - {std::move(*this)}); -} -template -template -auto ConfigureStoreChunk::fromRawPtr(T *data) - -> ConfigureStoreChunkFromBuffer< - std::shared_ptr const>> -{ - if (!data) - { - throw std::runtime_error( - "Unallocated pointer passed during chunk store."); - } - return ConfigureStoreChunkFromBuffer< - std::shared_ptr const>>( - auxiliary::shareRaw(data), {std::move(*this)}); -} - -template -template -auto ConfigureStoreChunk::fromUniquePtr( - std::unique_ptr data) - -> ConfigureStoreChunkFromBuffer< - UniquePtrWithLambda>> -{ - return fromUniquePtr(UniquePtrWithLambda(std::move(data))); -} -template -template -auto ConfigureStoreChunk::fromContiguousContainer( - T_ContiguousContainer &data) -> - typename std::enable_if_t< - auxiliary::IsContiguousContainer_v, - ConfigureStoreChunkFromBuffer const>>> -{ - if (!m_extent.has_value() && dim() == 1) - { - m_extent = Extent{data.size()}; - } - return fromRawPtr(data.data()); -} } // namespace openPMD + +#include "openPMD/LoadStoreChunk.tpp" diff --git a/include/openPMD/LoadStoreChunk.tpp b/include/openPMD/LoadStoreChunk.tpp new file mode 100644 index 0000000000..215df35302 --- /dev/null +++ b/include/openPMD/LoadStoreChunk.tpp @@ -0,0 +1,80 @@ +#pragma once + +#include "openPMD/LoadStoreChunk.hpp" + +namespace openPMD +{ + +template +template +auto ConfigureLoadStore::fromSharedPtr(std::shared_ptr data) + -> ConfigureStoreChunkFromBuffer< + std::shared_ptr const>> +{ + if (!data) + { + throw std::runtime_error( + "Unallocated pointer passed during chunk store."); + } + return ConfigureStoreChunkFromBuffer< + std::shared_ptr const>>( + std::static_pointer_cast const>( + std::move(data)), + {std::move(*this)}); +} +template +template +auto ConfigureLoadStore::fromUniquePtr(UniquePtrWithLambda data) + -> ConfigureStoreChunkFromBuffer< + UniquePtrWithLambda>> +{ + if (!data) + { + throw std::runtime_error( + "Unallocated pointer passed during chunk store."); + } + return ConfigureStoreChunkFromBuffer< + UniquePtrWithLambda>>( + std::move(data).template static_cast_>(), + {std::move(*this)}); +} +template +template +auto ConfigureLoadStore::fromRawPtr(T *data) + -> ConfigureStoreChunkFromBuffer< + std::shared_ptr const>> +{ + if (!data) + { + throw std::runtime_error( + "Unallocated pointer passed during chunk store."); + } + return ConfigureStoreChunkFromBuffer< + std::shared_ptr const>>( + auxiliary::shareRaw(data), {std::move(*this)}); +} + +template +template +auto ConfigureLoadStore::fromUniquePtr(std::unique_ptr data) + -> ConfigureStoreChunkFromBuffer< + UniquePtrWithLambda>> +{ + return fromUniquePtr(UniquePtrWithLambda(std::move(data))); +} +template +template +auto ConfigureLoadStore::fromContiguousContainer( + T_ContiguousContainer &data) -> + typename std::enable_if_t< + auxiliary::IsContiguousContainer_v, + ConfigureStoreChunkFromBuffer const>>> +{ + if (!m_extent.has_value() && dim() == 1) + { + m_extent = Extent{data.size()}; + } + return fromRawPtr(data.data()); +} +} // namespace openPMD diff --git a/include/openPMD/RecordComponent.hpp b/include/openPMD/RecordComponent.hpp index 7eceb9c778..2f8bff416a 100644 --- a/include/openPMD/RecordComponent.hpp +++ b/include/openPMD/RecordComponent.hpp @@ -137,7 +137,7 @@ class RecordComponent : public BaseRecordComponent template friend T &internal::makeOwning(T &self, Series); template - friend class ConfigureStoreChunk; + friend class ConfigureLoadStore; template friend class ConfigureStoreChunkFromBuffer; @@ -291,7 +291,7 @@ class RecordComponent : public BaseRecordComponent template void loadChunkRaw(T *data, Offset offset, Extent extent); - ConfigureStoreChunk prepareStoreChunk(); + ConfigureLoadStore prepareStoreChunk(); /** Store a chunk of data from a chunk of memory. * diff --git a/include/openPMD/RecordComponent.tpp b/include/openPMD/RecordComponent.tpp index 0d76c8d5c3..0d35a3653a 100644 --- a/include/openPMD/RecordComponent.tpp +++ b/include/openPMD/RecordComponent.tpp @@ -194,7 +194,7 @@ RecordComponent::storeChunk(std::shared_ptr data, Offset o, Extent e) .offset(std::move(o)) .extent(std::move(e)) .fromSharedPtr(std::move(data)) - .enqueue(); + .enqueueStore(); } template @@ -205,7 +205,7 @@ RecordComponent::storeChunk(UniquePtrWithLambda data, Offset o, Extent e) .offset(std::move(o)) .extent(std::move(e)) .fromUniquePtr(std::move(data)) - .enqueue(); + .enqueueStore(); } template @@ -216,7 +216,7 @@ RecordComponent::storeChunk(std::unique_ptr data, Offset o, Extent e) .offset(std::move(o)) .extent(std::move(e)) .fromUniquePtr(std::move(data)) - .enqueue(); + .enqueueStore(); } template @@ -226,7 +226,7 @@ void RecordComponent::storeChunkRaw(T *ptr, Offset offset, Extent extent) .offset(std::move(offset)) .extent(std::move(extent)) .fromRawPtr(ptr) - .enqueue(); + .enqueueStore(); } template @@ -246,7 +246,7 @@ RecordComponent::storeChunk(T_ContiguousContainer &data, Offset o, Extent e) storeChunkConfig.extent(std::move(e)); } - std::move(storeChunkConfig).fromContiguousContainer(data).enqueue(); + std::move(storeChunkConfig).fromContiguousContainer(data).enqueueStore(); } template @@ -256,7 +256,7 @@ RecordComponent::storeChunk(Offset o, Extent e, F &&createBuffer) return prepareStoreChunk() .offset(std::move(o)) .extent(std::move(e)) - .enqueue(std::forward(createBuffer)); + .enqueueStore(std::forward(createBuffer)); } template @@ -266,7 +266,7 @@ RecordComponent::storeChunk(Offset offset, Extent extent) return prepareStoreChunk() .offset(std::move(offset)) .extent(std::move(extent)) - .enqueue(); + .enqueueStore(); } template @@ -369,7 +369,7 @@ void RecordComponent::verifyChunk(Offset const &o, Extent const &e) const // definitions for LoadStoreChunk.hpp template template -auto ConfigureStoreChunk::enqueue(F &&createBuffer) +auto ConfigureLoadStore::enqueueStore(F &&createBuffer) -> DynamicMemoryView { return m_rc.storeChunkSpanCreateBuffer_impl( diff --git a/src/LoadStoreChunk.cpp b/src/LoadStoreChunk.cpp index 89712b728e..8b27ae7e41 100644 --- a/src/LoadStoreChunk.cpp +++ b/src/LoadStoreChunk.cpp @@ -44,24 +44,24 @@ namespace } // namespace template -ConfigureStoreChunk::ConfigureStoreChunk(RecordComponent &rc) +ConfigureLoadStore::ConfigureLoadStore(RecordComponent &rc) : ConfigureStoreChunkData(rc) {} template -ConfigureStoreChunk::ConfigureStoreChunk( +ConfigureLoadStore::ConfigureLoadStore( internal::ConfigureStoreChunkData &&data) : ConfigureStoreChunkData(std::move(data)) {} template -auto ConfigureStoreChunk::dim() const -> uint8_t +auto ConfigureLoadStore::dim() const -> uint8_t { return m_rc.getDimensionality(); } template -auto ConfigureStoreChunk::getOffset() const -> Offset +auto ConfigureLoadStore::getOffset() const -> Offset { if (m_offset.has_value()) { @@ -81,7 +81,7 @@ auto ConfigureStoreChunk::getOffset() const -> Offset } template -auto ConfigureStoreChunk::getExtent() const -> Extent +auto ConfigureLoadStore::getExtent() const -> Extent { if (m_extent.has_value()) { @@ -94,21 +94,21 @@ auto ConfigureStoreChunk::getExtent() const -> Extent } template -auto ConfigureStoreChunk::storeChunkConfig() const +auto ConfigureLoadStore::storeChunkConfig() const -> internal::StoreChunkConfig { return internal::StoreChunkConfig{getOffset(), getExtent()}; } template -auto ConfigureStoreChunk::extent(Extent extent) -> return_type & +auto ConfigureLoadStore::extent(Extent extent) -> return_type & { m_extent = std::move(extent); return *static_cast(this); } template -auto ConfigureStoreChunk::offset(Offset offset) -> return_type & +auto ConfigureLoadStore::offset(Offset offset) -> return_type & { m_offset = std::move(offset); return *static_cast(this); @@ -116,7 +116,7 @@ auto ConfigureStoreChunk::offset(Offset offset) -> return_type & template template -auto ConfigureStoreChunk::enqueue() -> DynamicMemoryView +auto ConfigureLoadStore::enqueueStore() -> DynamicMemoryView { return m_rc.storeChunkSpan_impl(storeChunkConfig()); } @@ -161,7 +161,7 @@ auto ConfigureStoreChunkFromBuffer::memorySelection( } template -auto ConfigureStoreChunkFromBuffer::enqueue() -> void +auto ConfigureStoreChunkFromBuffer::enqueueStore() -> void { this->m_rc.storeChunk_impl( asWriteBuffer(std::move(m_buffer)), @@ -170,12 +170,12 @@ auto ConfigureStoreChunkFromBuffer::enqueue() -> void } #define INSTANTIATE_METHOD_TEMPLATES(base_class, dtype) \ - template auto base_class::enqueue() -> DynamicMemoryView; + template auto base_class::enqueueStore() -> DynamicMemoryView; #define INSTANTIATE_METHOD_TEMPLATES_FOR_BASE(dtype) \ - INSTANTIATE_METHOD_TEMPLATES(ConfigureStoreChunk, dtype) + INSTANTIATE_METHOD_TEMPLATES(ConfigureLoadStore, dtype) -template class ConfigureStoreChunk; +template class ConfigureLoadStore; OPENPMD_FOREACH_DATASET_DATATYPE(INSTANTIATE_METHOD_TEMPLATES_FOR_BASE) #undef INSTANTIATE_METHOD_TEMPLATES_FOR_BASE @@ -183,17 +183,17 @@ OPENPMD_FOREACH_DATASET_DATATYPE(INSTANTIATE_METHOD_TEMPLATES_FOR_BASE) #define INSTANTIATE_STORE_CHUNK_FROM_BUFFER(dtype) \ template class ConfigureStoreChunkFromBuffer< \ std::shared_ptr>; \ - template class ConfigureStoreChunk< \ + template class ConfigureLoadStore< \ ConfigureStoreChunkFromBuffer>>; \ INSTANTIATE_METHOD_TEMPLATES( \ - ConfigureStoreChunk< \ + ConfigureLoadStore< \ ConfigureStoreChunkFromBuffer>>, \ dtype) \ template class ConfigureStoreChunkFromBuffer>; \ - template class ConfigureStoreChunk< \ + template class ConfigureLoadStore< \ ConfigureStoreChunkFromBuffer>>; \ INSTANTIATE_METHOD_TEMPLATES( \ - ConfigureStoreChunk< \ + ConfigureLoadStore< \ ConfigureStoreChunkFromBuffer>>, \ dtype) diff --git a/src/RecordComponent.cpp b/src/RecordComponent.cpp index d5fd894732..718c020f6e 100644 --- a/src/RecordComponent.cpp +++ b/src/RecordComponent.cpp @@ -66,9 +66,9 @@ RecordComponent::RecordComponent() : BaseRecordComponent(NoInit()) setData(std::make_shared()); } -ConfigureStoreChunk RecordComponent::prepareStoreChunk() +ConfigureLoadStore RecordComponent::prepareStoreChunk() { - return ConfigureStoreChunk{*this}; + return ConfigureLoadStore{*this}; } namespace diff --git a/test/ParallelIOTest.cpp b/test/ParallelIOTest.cpp index de415b450e..84d3c1f585 100644 --- a/test/ParallelIOTest.cpp +++ b/test/ParallelIOTest.cpp @@ -448,13 +448,13 @@ void available_chunks_test(std::string const &file_ending) .fromContiguousContainer(ydata_firstandlastrow) .offset({0, 3ul * mpi_rank}) .extent({1, 3}) - .enqueue(); + .enqueueStore(); E_y.prepareStoreChunk() .offset({1, 3ul * mpi_rank}) .extent({3, 3}) .fromContiguousContainer(ydata) .memorySelection({{1, 1}, {5, 5}}) - .enqueue(); + .enqueueStore(); // if condition checks if this PR is available in ADIOS2: // https://github.com/ornladios/ADIOS2/pull/4169 if constexpr (CanTheMemorySelectionBeReset) @@ -463,7 +463,7 @@ void available_chunks_test(std::string const &file_ending) .fromContiguousContainer(ydata_firstandlastrow) .offset({4, 3ul * mpi_rank}) .extent({1, 3}) - .enqueue(); + .enqueueStore(); } it0.close(); } From a5531af78b41b226fc088c418826fa2b8e532aaf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Wed, 22 May 2024 13:35:20 +0200 Subject: [PATCH 14/39] Prepare class structure for load_chunk --- include/openPMD/LoadStoreChunk.hpp | 78 ++++++++++++++++++++--------- include/openPMD/LoadStoreChunk.tpp | 30 ++++------- include/openPMD/RecordComponent.hpp | 2 +- src/LoadStoreChunk.cpp | 59 ++++++++++++++-------- 4 files changed, 103 insertions(+), 66 deletions(-) diff --git a/include/openPMD/LoadStoreChunk.hpp b/include/openPMD/LoadStoreChunk.hpp index 882b127d05..317211e9bb 100644 --- a/include/openPMD/LoadStoreChunk.hpp +++ b/include/openPMD/LoadStoreChunk.hpp @@ -11,8 +11,10 @@ namespace openPMD { class RecordComponent; -template +template class ConfigureStoreChunkFromBuffer; +template +class ConfigureLoadStoreFromBuffer; template class DynamicMemoryView; @@ -67,30 +69,28 @@ class ConfigureLoadStore : protected internal::ConfigureStoreChunkData auto offset(Offset) -> return_type &; auto extent(Extent) -> return_type &; + template + using shared_ptr_return_type = ConfigureLoadStoreFromBuffer< + std::shared_ptr const>>; + template + using unique_ptr_return_type = ConfigureStoreChunkFromBuffer< + UniquePtrWithLambda>, + void>; + // @todo rvalue references..? template - auto fromSharedPtr(std::shared_ptr) - -> ConfigureStoreChunkFromBuffer< - std::shared_ptr const>>; + auto fromSharedPtr(std::shared_ptr) -> shared_ptr_return_type; template - auto fromUniquePtr(UniquePtrWithLambda) - -> ConfigureStoreChunkFromBuffer< - UniquePtrWithLambda>>; + auto fromUniquePtr(UniquePtrWithLambda) -> unique_ptr_return_type; template - auto fromUniquePtr(std::unique_ptr) - -> ConfigureStoreChunkFromBuffer< - UniquePtrWithLambda>>; + auto fromUniquePtr(std::unique_ptr) -> unique_ptr_return_type; template - auto - fromRawPtr(T *data) -> ConfigureStoreChunkFromBuffer< - std::shared_ptr const>>; + auto fromRawPtr(T *data) -> shared_ptr_return_type; template - auto fromContiguousContainer(T_ContiguousContainer &data) -> - typename std::enable_if_t< + auto fromContiguousContainer(T_ContiguousContainer &data) + -> std::enable_if_t< auxiliary::IsContiguousContainer_v, - ConfigureStoreChunkFromBuffer< - std::shared_ptr const>>>; + shared_ptr_return_type>; template [[nodiscard]] auto enqueueStore() -> DynamicMemoryView; @@ -98,15 +98,24 @@ class ConfigureLoadStore : protected internal::ConfigureStoreChunkData // definition of class RecordComponent. template [[nodiscard]] auto enqueueStore(F &&createBuffer) -> DynamicMemoryView; + + template + [[nodiscard]] auto enqueueLoad() -> std::shared_ptr; }; -template +template class ConfigureStoreChunkFromBuffer - : public ConfigureLoadStore> + : public ConfigureLoadStore, + /*then*/ ConfigureStoreChunkFromBuffer, + /*else*/ ChildClass>> { public: - using parent_t = - ConfigureLoadStore>; + using return_type = std::conditional_t< + std::is_void_v, + /*then*/ ConfigureStoreChunkFromBuffer, + /*else*/ ChildClass>; + using parent_t = ConfigureLoadStore; private: template @@ -115,12 +124,13 @@ class ConfigureStoreChunkFromBuffer Ptr_Type m_buffer; std::optional m_mem_select; - ConfigureStoreChunkFromBuffer(Ptr_Type buffer, parent_t &&); - auto storeChunkConfig() const -> internal::StoreChunkConfigFromBuffer; +protected: + ConfigureStoreChunkFromBuffer(Ptr_Type buffer, parent_t &&); + public: - auto memorySelection(MemorySelection) -> ConfigureStoreChunkFromBuffer &; + auto memorySelection(MemorySelection) -> return_type &; auto as_parent() && -> parent_t &&; auto as_parent() & -> parent_t &; @@ -128,6 +138,24 @@ class ConfigureStoreChunkFromBuffer auto enqueueStore() -> void; }; + +template +class ConfigureLoadStoreFromBuffer + : public ConfigureStoreChunkFromBuffer< + Ptr_Type, + ConfigureLoadStoreFromBuffer> +{ + using parent_t = ConfigureStoreChunkFromBuffer< + Ptr_Type, + ConfigureLoadStoreFromBuffer>; + template + friend class ConfigureLoadStore; + ConfigureLoadStoreFromBuffer( + Ptr_Type buffer, typename parent_t::parent_t &&); + +public: + auto enqueueLoad() -> void; +}; } // namespace openPMD #include "openPMD/LoadStoreChunk.tpp" diff --git a/include/openPMD/LoadStoreChunk.tpp b/include/openPMD/LoadStoreChunk.tpp index 215df35302..8f227aea03 100644 --- a/include/openPMD/LoadStoreChunk.tpp +++ b/include/openPMD/LoadStoreChunk.tpp @@ -4,20 +4,17 @@ namespace openPMD { - template template auto ConfigureLoadStore::fromSharedPtr(std::shared_ptr data) - -> ConfigureStoreChunkFromBuffer< - std::shared_ptr const>> + -> shared_ptr_return_type { if (!data) { throw std::runtime_error( "Unallocated pointer passed during chunk store."); } - return ConfigureStoreChunkFromBuffer< - std::shared_ptr const>>( + return shared_ptr_return_type( std::static_pointer_cast const>( std::move(data)), {std::move(*this)}); @@ -25,51 +22,46 @@ auto ConfigureLoadStore::fromSharedPtr(std::shared_ptr data) template template auto ConfigureLoadStore::fromUniquePtr(UniquePtrWithLambda data) - -> ConfigureStoreChunkFromBuffer< - UniquePtrWithLambda>> + -> unique_ptr_return_type + { if (!data) { throw std::runtime_error( "Unallocated pointer passed during chunk store."); } - return ConfigureStoreChunkFromBuffer< - UniquePtrWithLambda>>( + return unique_ptr_return_type( std::move(data).template static_cast_>(), {std::move(*this)}); } template template auto ConfigureLoadStore::fromRawPtr(T *data) - -> ConfigureStoreChunkFromBuffer< - std::shared_ptr const>> + -> shared_ptr_return_type { if (!data) { throw std::runtime_error( "Unallocated pointer passed during chunk store."); } - return ConfigureStoreChunkFromBuffer< - std::shared_ptr const>>( + return shared_ptr_return_type( auxiliary::shareRaw(data), {std::move(*this)}); } template template auto ConfigureLoadStore::fromUniquePtr(std::unique_ptr data) - -> ConfigureStoreChunkFromBuffer< - UniquePtrWithLambda>> + -> unique_ptr_return_type { return fromUniquePtr(UniquePtrWithLambda(std::move(data))); } template template auto ConfigureLoadStore::fromContiguousContainer( - T_ContiguousContainer &data) -> - typename std::enable_if_t< + T_ContiguousContainer &data) + -> std::enable_if_t< auxiliary::IsContiguousContainer_v, - ConfigureStoreChunkFromBuffer const>>> + shared_ptr_return_type> { if (!m_extent.has_value() && dim() == 1) { diff --git a/include/openPMD/RecordComponent.hpp b/include/openPMD/RecordComponent.hpp index 2f8bff416a..32199d8db2 100644 --- a/include/openPMD/RecordComponent.hpp +++ b/include/openPMD/RecordComponent.hpp @@ -138,7 +138,7 @@ class RecordComponent : public BaseRecordComponent friend T &internal::makeOwning(T &self, Series); template friend class ConfigureLoadStore; - template + template friend class ConfigureStoreChunkFromBuffer; public: diff --git a/src/LoadStoreChunk.cpp b/src/LoadStoreChunk.cpp index 8b27ae7e41..69202eb91b 100644 --- a/src/LoadStoreChunk.cpp +++ b/src/LoadStoreChunk.cpp @@ -121,47 +121,49 @@ auto ConfigureLoadStore::enqueueStore() -> DynamicMemoryView return m_rc.storeChunkSpan_impl(storeChunkConfig()); } -template -ConfigureStoreChunkFromBuffer::ConfigureStoreChunkFromBuffer( - Ptr_Type buffer, parent_t &&parent) +template +ConfigureStoreChunkFromBuffer:: + ConfigureStoreChunkFromBuffer(Ptr_Type buffer, parent_t &&parent) : parent_t(std::move(parent)), m_buffer(std::move(buffer)) {} -template -auto ConfigureStoreChunkFromBuffer::as_parent() && -> parent_t && +template +auto ConfigureStoreChunkFromBuffer::as_parent() + && -> parent_t && { return std::move(*this); } -template -auto ConfigureStoreChunkFromBuffer::as_parent() & -> parent_t & +template +auto ConfigureStoreChunkFromBuffer::as_parent() + & -> parent_t & { return *this; } -template -auto ConfigureStoreChunkFromBuffer::as_parent() +template +auto ConfigureStoreChunkFromBuffer::as_parent() const & -> parent_t const & { return *this; } -template -auto ConfigureStoreChunkFromBuffer::storeChunkConfig() const - -> internal::StoreChunkConfigFromBuffer +template +auto ConfigureStoreChunkFromBuffer::storeChunkConfig() + const -> internal::StoreChunkConfigFromBuffer { return internal::StoreChunkConfigFromBuffer{ this->getOffset(), this->getExtent(), m_mem_select}; } -template -auto ConfigureStoreChunkFromBuffer::memorySelection( - MemorySelection sel) -> ConfigureStoreChunkFromBuffer & +template +auto ConfigureStoreChunkFromBuffer::memorySelection( + MemorySelection sel) -> return_type & { this->m_mem_select = std::make_optional(std::move(sel)); - return *this; + return *static_cast(this); } -template -auto ConfigureStoreChunkFromBuffer::enqueueStore() -> void +template +auto ConfigureStoreChunkFromBuffer::enqueueStore() -> void { this->m_rc.storeChunk_impl( asWriteBuffer(std::move(m_buffer)), @@ -169,6 +171,19 @@ auto ConfigureStoreChunkFromBuffer::enqueueStore() -> void storeChunkConfig()); } +template +ConfigureLoadStoreFromBuffer::ConfigureLoadStoreFromBuffer( + Ptr_Type buffer, typename parent_t::parent_t &&parent) + : parent_t(std::move(buffer), std::move(parent)) +{ + static_assert( + std::is_same_v< + Ptr_Type, + std::shared_ptr>, + "ConfigureLoadStoreFromBuffer must be instantiated with a shared_ptr " + "type."); +} + #define INSTANTIATE_METHOD_TEMPLATES(base_class, dtype) \ template auto base_class::enqueueStore() -> DynamicMemoryView; @@ -181,13 +196,15 @@ OPENPMD_FOREACH_DATASET_DATATYPE(INSTANTIATE_METHOD_TEMPLATES_FOR_BASE) #undef INSTANTIATE_METHOD_TEMPLATES_FOR_BASE #define INSTANTIATE_STORE_CHUNK_FROM_BUFFER(dtype) \ + template class ConfigureLoadStoreFromBuffer>; \ template class ConfigureStoreChunkFromBuffer< \ - std::shared_ptr>; \ + std::shared_ptr, \ + ConfigureLoadStoreFromBuffer>>; \ template class ConfigureLoadStore< \ - ConfigureStoreChunkFromBuffer>>; \ + ConfigureLoadStoreFromBuffer>>; \ INSTANTIATE_METHOD_TEMPLATES( \ ConfigureLoadStore< \ - ConfigureStoreChunkFromBuffer>>, \ + ConfigureLoadStoreFromBuffer>>, \ dtype) \ template class ConfigureStoreChunkFromBuffer>; \ template class ConfigureLoadStore< \ From 96da02cee16ddafe86767063477e27062438701c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Wed, 22 May 2024 14:57:37 +0200 Subject: [PATCH 15/39] WIP loadChunk --- include/openPMD/LoadStoreChunk.hpp | 22 +++--- include/openPMD/RecordComponent.hpp | 16 +++- include/openPMD/RecordComponent.tpp | 113 ++++++++++++++++------------ src/LoadStoreChunk.cpp | 70 +++++++++++------ src/RecordComponent.cpp | 8 +- test/ParallelIOTest.cpp | 6 +- test/SerialIOTest.cpp | 2 +- 7 files changed, 142 insertions(+), 95 deletions(-) diff --git a/include/openPMD/LoadStoreChunk.hpp b/include/openPMD/LoadStoreChunk.hpp index 317211e9bb..597943b516 100644 --- a/include/openPMD/LoadStoreChunk.hpp +++ b/include/openPMD/LoadStoreChunk.hpp @@ -20,21 +20,21 @@ class DynamicMemoryView; namespace internal { - struct StoreChunkConfig + struct LoadStoreConfig { Offset offset; Extent extent; }; - struct StoreChunkConfigFromBuffer + struct LoadStoreConfigWithBuffer { Offset offset; Extent extent; std::optional memorySelection; }; - struct ConfigureStoreChunkData + struct ConfigureLoadStoreData { - ConfigureStoreChunkData(RecordComponent &); + ConfigureLoadStoreData(RecordComponent &); RecordComponent &m_rc; std::optional m_offset; @@ -43,7 +43,7 @@ namespace internal } // namespace internal template -class ConfigureLoadStore : protected internal::ConfigureStoreChunkData +class ConfigureLoadStore : protected internal::ConfigureLoadStoreData { friend class RecordComponent; template @@ -51,12 +51,12 @@ class ConfigureLoadStore : protected internal::ConfigureStoreChunkData protected: ConfigureLoadStore(RecordComponent &rc); - ConfigureLoadStore(ConfigureStoreChunkData &&); + ConfigureLoadStore(ConfigureLoadStoreData &&); auto dim() const -> uint8_t; - auto getOffset() const -> Offset; - auto getExtent() const -> Extent; - auto storeChunkConfig() const -> internal::StoreChunkConfig; + auto getOffset() -> Offset const &; + auto getExtent() -> Extent const &; + auto storeChunkConfig() -> internal::LoadStoreConfig; public: using return_type = std::conditional_t< @@ -117,14 +117,14 @@ class ConfigureStoreChunkFromBuffer /*else*/ ChildClass>; using parent_t = ConfigureLoadStore; -private: +protected: template friend class ConfigureLoadStore; Ptr_Type m_buffer; std::optional m_mem_select; - auto storeChunkConfig() const -> internal::StoreChunkConfigFromBuffer; + auto storeChunkConfig() -> internal::LoadStoreConfigWithBuffer; protected: ConfigureStoreChunkFromBuffer(Ptr_Type buffer, parent_t &&); diff --git a/include/openPMD/RecordComponent.hpp b/include/openPMD/RecordComponent.hpp index 32199d8db2..7ad892b51c 100644 --- a/include/openPMD/RecordComponent.hpp +++ b/include/openPMD/RecordComponent.hpp @@ -140,6 +140,8 @@ class RecordComponent : public BaseRecordComponent friend class ConfigureLoadStore; template friend class ConfigureStoreChunkFromBuffer; + template + friend class ConfigureLoadStoreFromBuffer; public: enum class Allocation @@ -291,7 +293,7 @@ class RecordComponent : public BaseRecordComponent template void loadChunkRaw(T *data, Offset offset, Extent extent); - ConfigureLoadStore prepareStoreChunk(); + ConfigureLoadStore prepareLoadStore(); /** Store a chunk of data from a chunk of memory. * @@ -479,13 +481,19 @@ class RecordComponent : public BaseRecordComponent void storeChunk_impl( auxiliary::WriteBuffer buffer, Datatype datatype, - internal::StoreChunkConfigFromBuffer); + internal::LoadStoreConfigWithBuffer); template - DynamicMemoryView storeChunkSpan_impl(internal::StoreChunkConfig); + DynamicMemoryView storeChunkSpan_impl(internal::LoadStoreConfig); template DynamicMemoryView storeChunkSpanCreateBuffer_impl( - internal::StoreChunkConfig, F &&createBuffer); + internal::LoadStoreConfig, F &&createBuffer); + + template + void + loadChunk_impl(std::shared_ptr, internal::LoadStoreConfigWithBuffer); + template + std::shared_ptr loadChunkAllocate_impl(internal::LoadStoreConfig); // clang-format off OPENPMD_protected diff --git a/include/openPMD/RecordComponent.tpp b/include/openPMD/RecordComponent.tpp index 0d35a3653a..fb4a5b738e 100644 --- a/include/openPMD/RecordComponent.tpp +++ b/include/openPMD/RecordComponent.tpp @@ -61,27 +61,35 @@ template inline std::shared_ptr RecordComponent::loadChunk(Offset o, Extent e) { uint8_t dim = getDimensionality(); + auto operation = prepareLoadStore(); // default arguments // offset = {0u}: expand to right dim {0u, 0u, ...} - Offset offset = o; - if (o.size() == 1u && o.at(0) == 0u && dim > 1u) - offset = Offset(dim, 0u); + if (o.size() != 1u || o.at(0) != 0u || dim <= 1u) + { + operation.offset(std::move(o)); + } // extent = {-1u}: take full size - Extent extent(dim, 1u); - if (e.size() == 1u && e.at(0) == -1u) + if (e.size() != 1u || e.at(0) != -1u) { - extent = getExtent(); - for (uint8_t i = 0u; i < dim; ++i) - extent[i] -= offset[i]; + operation.extent(std::move(e)); } - else - extent = e; - uint64_t numPoints = 1u; - for (auto const &dimensionSize : extent) - numPoints *= dimensionSize; + return operation.enqueueLoad(); +} + +template +inline std::shared_ptr +RecordComponent::loadChunkAllocate_impl(internal::LoadStoreConfig cfg) +{ + auto [o, e] = std::move(cfg); + + size_t numPoints = 1; + for (auto val : e) + { + numPoints *= val; + } #if (defined(_LIBCPP_VERSION) && _LIBCPP_VERSION < 11000) || \ (defined(__apple_build_version__) && __clang_major__ < 14) @@ -91,7 +99,11 @@ inline std::shared_ptr RecordComponent::loadChunk(Offset o, Extent e) return newData; #else auto newData = std::shared_ptr(new T[numPoints]); - loadChunk(newData, offset, extent); + prepareLoadStore() + .offset(std::move(o)) + .extent(std::move(e)) + .fromSharedPtr(newData) + .enqueueLoad(); return std::static_pointer_cast(std::move(newData)); #endif } @@ -100,7 +112,31 @@ template inline void RecordComponent::loadChunk( std::shared_ptr data, Offset o, Extent e) { - using T = std::remove_extent_t; + uint8_t dim = getDimensionality(); + auto operation = prepareLoadStore(); + + // default arguments + // offset = {0u}: expand to right dim {0u, 0u, ...} + if (o.size() != 1u || o.at(0) != 0u || dim <= 1u) + { + operation.offset(std::move(o)); + } + + // extent = {-1u}: take full size + if (e.size() != 1u || e.at(0) != -1u) + { + operation.extent(std::move(e)); + } + + operation.fromSharedPtr(std::move(data)).enqueueLoad(); +} + +template +inline void RecordComponent::loadChunk_impl( + std::shared_ptr data, + internal::LoadStoreConfigWithBuffer cfg) +{ + using T = std::remove_cv_t>; Datatype dtype = determineDatatype(data); if (dtype != getDatatype()) if (!isSameInteger(getDatatype()) && @@ -117,24 +153,8 @@ inline void RecordComponent::loadChunk( throw std::runtime_error(err_msg); } - uint8_t dim = getDimensionality(); - - // default arguments - // offset = {0u}: expand to right dim {0u, 0u, ...} - Offset offset = o; - if (o.size() == 1u && o.at(0) == 0u && dim > 1u) - offset = Offset(dim, 0u); - - // extent = {-1u}: take full size - Extent extent(dim, 1u); - if (e.size() == 1u && e.at(0) == -1u) - { - extent = getExtent(); - for (uint8_t i = 0u; i < dim; ++i) - extent[i] -= offset[i]; - } - else - extent = e; + auto dim = getDimensionality(); + auto [offset, extent, memorySelection] = std::move(cfg); if (extent.size() != dim || offset.size() != dim) { @@ -153,9 +173,6 @@ inline void RecordComponent::loadChunk( "Chunk does not reside inside dataset (Dimension on index " + std::to_string(i) + ". DS: " + std::to_string(dse[i]) + " - Chunk: " + std::to_string(offset[i] + extent[i]) + ")"); - if (!data) - throw std::runtime_error( - "Unallocated pointer passed during chunk loading."); auto &rc = get(); if (constant()) @@ -166,8 +183,9 @@ inline void RecordComponent::loadChunk( T value = rc.m_constantValue.get(); - auto raw_ptr = static_cast(data.get()); - std::fill(raw_ptr, raw_ptr + numPoints, value); + // @todo + // auto raw_ptr = static_cast(data.get()); + // std::fill(raw_ptr, raw_ptr + numPoints, value); } else { @@ -175,7 +193,8 @@ inline void RecordComponent::loadChunk( dRead.offset = offset; dRead.extent = extent; dRead.dtype = getDatatype(); - dRead.data = std::static_pointer_cast(data); + // @todo + // dRead.data = std::static_pointer_cast(data); rc.push_chunk(IOTask(this, dRead)); } } @@ -190,7 +209,7 @@ template inline void RecordComponent::storeChunk(std::shared_ptr data, Offset o, Extent e) { - prepareStoreChunk() + prepareLoadStore() .offset(std::move(o)) .extent(std::move(e)) .fromSharedPtr(std::move(data)) @@ -201,7 +220,7 @@ template inline void RecordComponent::storeChunk(UniquePtrWithLambda data, Offset o, Extent e) { - prepareStoreChunk() + prepareLoadStore() .offset(std::move(o)) .extent(std::move(e)) .fromUniquePtr(std::move(data)) @@ -212,7 +231,7 @@ template inline void RecordComponent::storeChunk(std::unique_ptr data, Offset o, Extent e) { - prepareStoreChunk() + prepareLoadStore() .offset(std::move(o)) .extent(std::move(e)) .fromUniquePtr(std::move(data)) @@ -222,7 +241,7 @@ RecordComponent::storeChunk(std::unique_ptr data, Offset o, Extent e) template void RecordComponent::storeChunkRaw(T *ptr, Offset offset, Extent extent) { - prepareStoreChunk() + prepareLoadStore() .offset(std::move(offset)) .extent(std::move(extent)) .fromRawPtr(ptr) @@ -234,7 +253,7 @@ inline typename std::enable_if_t< auxiliary::IsContiguousContainer_v> RecordComponent::storeChunk(T_ContiguousContainer &data, Offset o, Extent e) { - auto storeChunkConfig = prepareStoreChunk(); + auto storeChunkConfig = prepareLoadStore(); auto joined_dim = joinedDimension(); if (!joined_dim.has_value() && (o.size() != 1 || o.at(0) != 0u)) @@ -253,7 +272,7 @@ template inline DynamicMemoryView RecordComponent::storeChunk(Offset o, Extent e, F &&createBuffer) { - return prepareStoreChunk() + return prepareLoadStore() .offset(std::move(o)) .extent(std::move(e)) .enqueueStore(std::forward(createBuffer)); @@ -263,7 +282,7 @@ template inline DynamicMemoryView RecordComponent::storeChunk(Offset offset, Extent extent) { - return prepareStoreChunk() + return prepareLoadStore() .offset(std::move(offset)) .extent(std::move(extent)) .enqueueStore(); @@ -271,7 +290,7 @@ RecordComponent::storeChunk(Offset offset, Extent extent) template inline DynamicMemoryView RecordComponent::storeChunkSpanCreateBuffer_impl( - internal::StoreChunkConfig cfg, F &&createBuffer) + internal::LoadStoreConfig cfg, F &&createBuffer) { auto [o, e] = std::move(cfg); verifyChunk(o, e); diff --git a/src/LoadStoreChunk.cpp b/src/LoadStoreChunk.cpp index 69202eb91b..6d9ab0e253 100644 --- a/src/LoadStoreChunk.cpp +++ b/src/LoadStoreChunk.cpp @@ -21,7 +21,7 @@ namespace openPMD namespace internal { - ConfigureStoreChunkData::ConfigureStoreChunkData(RecordComponent &rc) + ConfigureLoadStoreData::ConfigureLoadStoreData(RecordComponent &rc) : m_rc(rc) {} } // namespace internal @@ -45,13 +45,13 @@ namespace template ConfigureLoadStore::ConfigureLoadStore(RecordComponent &rc) - : ConfigureStoreChunkData(rc) + : ConfigureLoadStoreData(rc) {} template ConfigureLoadStore::ConfigureLoadStore( - internal::ConfigureStoreChunkData &&data) - : ConfigureStoreChunkData(std::move(data)) + internal::ConfigureLoadStoreData &&data) + : ConfigureLoadStoreData(std::move(data)) {} template @@ -61,43 +61,48 @@ auto ConfigureLoadStore::dim() const -> uint8_t } template -auto ConfigureLoadStore::getOffset() const -> Offset +auto ConfigureLoadStore::getOffset() -> Offset const & { - if (m_offset.has_value()) - { - return *m_offset; - } - else + if (!m_offset.has_value()) { if (m_rc.joinedDimension().has_value()) { - return Offset{}; + m_offset = std::make_optional(); } else { - return Offset(dim(), 0); + m_offset = std::make_optional(dim(), 0); } } + return *m_offset; } template -auto ConfigureLoadStore::getExtent() const -> Extent +auto ConfigureLoadStore::getExtent() -> Extent const & { - if (m_extent.has_value()) - { - return *m_extent; - } - else + if (!m_extent.has_value()) { - return Extent(dim(), 0); + m_extent = std::make_optional(m_rc.getExtent()); + if (m_offset.has_value()) + { + auto it_o = m_offset->begin(); + auto end_o = m_offset->end(); + auto it_e = m_extent->begin(); + auto end_e = m_extent->end(); + for (; it_o != end_o && it_e != end_e; ++it_e, ++it_o) + { + *it_e -= *it_o; + } + } } + return *m_extent; } template -auto ConfigureLoadStore::storeChunkConfig() const - -> internal::StoreChunkConfig +auto ConfigureLoadStore::storeChunkConfig() + -> internal::LoadStoreConfig { - return internal::StoreChunkConfig{getOffset(), getExtent()}; + return internal::LoadStoreConfig{getOffset(), getExtent()}; } template @@ -121,6 +126,13 @@ auto ConfigureLoadStore::enqueueStore() -> DynamicMemoryView return m_rc.storeChunkSpan_impl(storeChunkConfig()); } +template +template +auto ConfigureLoadStore::enqueueLoad() -> std::shared_ptr +{ + return m_rc.loadChunkAllocate_impl(storeChunkConfig()); +} + template ConfigureStoreChunkFromBuffer:: ConfigureStoreChunkFromBuffer(Ptr_Type buffer, parent_t &&parent) @@ -148,9 +160,9 @@ auto ConfigureStoreChunkFromBuffer::as_parent() template auto ConfigureStoreChunkFromBuffer::storeChunkConfig() - const -> internal::StoreChunkConfigFromBuffer + -> internal::LoadStoreConfigWithBuffer { - return internal::StoreChunkConfigFromBuffer{ + return internal::LoadStoreConfigWithBuffer{ this->getOffset(), this->getExtent(), m_mem_select}; } @@ -184,8 +196,16 @@ ConfigureLoadStoreFromBuffer::ConfigureLoadStoreFromBuffer( "type."); } +template +auto ConfigureLoadStoreFromBuffer::enqueueLoad() -> void +{ + this->m_rc.loadChunk_impl( + std::move(this->m_buffer), this->storeChunkConfig()); +} + #define INSTANTIATE_METHOD_TEMPLATES(base_class, dtype) \ - template auto base_class::enqueueStore() -> DynamicMemoryView; + template auto base_class::enqueueStore() -> DynamicMemoryView; \ + template auto base_class::enqueueLoad() -> std::shared_ptr; #define INSTANTIATE_METHOD_TEMPLATES_FOR_BASE(dtype) \ INSTANTIATE_METHOD_TEMPLATES(ConfigureLoadStore, dtype) diff --git a/src/RecordComponent.cpp b/src/RecordComponent.cpp index 718c020f6e..2bbc901041 100644 --- a/src/RecordComponent.cpp +++ b/src/RecordComponent.cpp @@ -66,7 +66,7 @@ RecordComponent::RecordComponent() : BaseRecordComponent(NoInit()) setData(std::make_shared()); } -ConfigureLoadStore RecordComponent::prepareStoreChunk() +ConfigureLoadStore RecordComponent::prepareLoadStore() { return ConfigureLoadStore{*this}; } @@ -91,14 +91,14 @@ namespace template DynamicMemoryView -RecordComponent::storeChunkSpan_impl(internal::StoreChunkConfig cfg) +RecordComponent::storeChunkSpan_impl(internal::LoadStoreConfig cfg) { return storeChunkSpanCreateBuffer_impl( std::move(cfg), &createSpanBufferFallback); } #define OPENPMD_INSTANTIATE(dtype) \ template DynamicMemoryView RecordComponent::storeChunkSpan_impl( \ - internal::StoreChunkConfig cfg); + internal::LoadStoreConfig cfg); OPENPMD_FOREACH_DATASET_DATATYPE(OPENPMD_INSTANTIATE) #undef OPENPMD_INSTANTIATE @@ -502,7 +502,7 @@ void RecordComponent::readBase(bool require_unit_si) void RecordComponent::storeChunk_impl( auxiliary::WriteBuffer buffer, Datatype dtype, - internal::StoreChunkConfigFromBuffer cfg) + internal::LoadStoreConfigWithBuffer cfg) { auto [o, e, memorySelection] = std::move(cfg); verifyChunk(dtype, o, e); diff --git a/test/ParallelIOTest.cpp b/test/ParallelIOTest.cpp index 84d3c1f585..3e462c978c 100644 --- a/test/ParallelIOTest.cpp +++ b/test/ParallelIOTest.cpp @@ -444,12 +444,12 @@ void available_chunks_test(std::string const &file_ending) E_x.storeChunk(xdata, {mpi_rank, 0}, {1, 4}); auto E_y = it0.meshes["E"]["y"]; E_y.resetDataset({Datatype::INT, {5, 3ul * mpi_size}}); - E_y.prepareStoreChunk() + E_y.prepareLoadStore() .fromContiguousContainer(ydata_firstandlastrow) .offset({0, 3ul * mpi_rank}) .extent({1, 3}) .enqueueStore(); - E_y.prepareStoreChunk() + E_y.prepareLoadStore() .offset({1, 3ul * mpi_rank}) .extent({3, 3}) .fromContiguousContainer(ydata) @@ -459,7 +459,7 @@ void available_chunks_test(std::string const &file_ending) // https://github.com/ornladios/ADIOS2/pull/4169 if constexpr (CanTheMemorySelectionBeReset) { - E_y.prepareStoreChunk() + E_y.prepareLoadStore() .fromContiguousContainer(ydata_firstandlastrow) .offset({4, 3ul * mpi_rank}) .extent({1, 3}) diff --git a/test/SerialIOTest.cpp b/test/SerialIOTest.cpp index 7323a32582..aaa9988472 100644 --- a/test/SerialIOTest.cpp +++ b/test/SerialIOTest.cpp @@ -2892,7 +2892,7 @@ TEST_CASE("git_hdf5_legacy_picongpu", "[serial][hdf5]") auto radiationMask = o.iterations[200] .particles["e"]["radiationMask"][RecordComponent::SCALAR]; - switchNonVectorType( + switchDatasetType( radiationMask.getDatatype(), radiationMask); auto particlePatches = o.iterations[200].particles["e"].particlePatches; From fa1ddba8bca6dc59d1f61eed8051a331cfc9849a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Wed, 22 May 2024 15:22:41 +0200 Subject: [PATCH 16/39] Implement this for loadChunk --- include/openPMD/LoadStoreChunk.hpp | 36 +++++++++++++++++++++++--- include/openPMD/LoadStoreChunk.tpp | 3 +-- include/openPMD/RecordComponent.tpp | 16 +++++++----- src/LoadStoreChunk.cpp | 18 +++++++++---- src/binding/python/RecordComponent.cpp | 10 +------ 5 files changed, 57 insertions(+), 26 deletions(-) diff --git a/include/openPMD/LoadStoreChunk.hpp b/include/openPMD/LoadStoreChunk.hpp index 597943b516..7818ce0137 100644 --- a/include/openPMD/LoadStoreChunk.hpp +++ b/include/openPMD/LoadStoreChunk.hpp @@ -63,15 +63,35 @@ class ConfigureLoadStore : protected internal::ConfigureLoadStoreData std::is_void_v, /*then*/ ConfigureLoadStore, /*else*/ ChildClass>; - template - using normalize_dataset_type = std::remove_cv_t>; auto offset(Offset) -> return_type &; auto extent(Extent) -> return_type &; template - using shared_ptr_return_type = ConfigureLoadStoreFromBuffer< - std::shared_ptr const>>; + struct shared_ptr_return_type_impl + { + using type = ConfigureLoadStoreFromBuffer< + std::shared_ptr>>; + }; + template + struct shared_ptr_return_type_impl + { + using type = + ConfigureStoreChunkFromBuffer, void>; + }; + template + struct shared_ptr_return_type_impl + { + using type = + ConfigureStoreChunkFromBuffer, void>; + }; + + template + using shared_ptr_return_type = + typename shared_ptr_return_type_impl::type; + + template + using normalize_dataset_type = std::remove_cv_t>; template using unique_ptr_return_type = ConfigureStoreChunkFromBuffer< UniquePtrWithLambda>, @@ -137,6 +157,14 @@ class ConfigureStoreChunkFromBuffer auto as_parent() const & -> parent_t const &; auto enqueueStore() -> void; + template + auto enqueueLoad() + { + static_assert( + auxiliary::dependent_false_v, + "Cannot load chunk data into a buffer that is const or a " + "unique_ptr."); + } }; template diff --git a/include/openPMD/LoadStoreChunk.tpp b/include/openPMD/LoadStoreChunk.tpp index 8f227aea03..63b4b14263 100644 --- a/include/openPMD/LoadStoreChunk.tpp +++ b/include/openPMD/LoadStoreChunk.tpp @@ -15,8 +15,7 @@ auto ConfigureLoadStore::fromSharedPtr(std::shared_ptr data) "Unallocated pointer passed during chunk store."); } return shared_ptr_return_type( - std::static_pointer_cast const>( - std::move(data)), + std::static_pointer_cast>(std::move(data)), {std::move(*this)}); } template diff --git a/include/openPMD/RecordComponent.tpp b/include/openPMD/RecordComponent.tpp index fb4a5b738e..5a37e49ad7 100644 --- a/include/openPMD/RecordComponent.tpp +++ b/include/openPMD/RecordComponent.tpp @@ -83,6 +83,7 @@ template inline std::shared_ptr RecordComponent::loadChunkAllocate_impl(internal::LoadStoreConfig cfg) { + static_assert(!std::is_same_v, "EVIL"); auto [o, e] = std::move(cfg); size_t numPoints = 1; @@ -112,6 +113,7 @@ template inline void RecordComponent::loadChunk( std::shared_ptr data, Offset o, Extent e) { + static_assert(!std::is_same_v, "EVIL"); uint8_t dim = getDimensionality(); auto operation = prepareLoadStore(); @@ -183,9 +185,8 @@ inline void RecordComponent::loadChunk_impl( T value = rc.m_constantValue.get(); - // @todo - // auto raw_ptr = static_cast(data.get()); - // std::fill(raw_ptr, raw_ptr + numPoints, value); + auto raw_ptr = static_cast(data.get()); + std::fill(raw_ptr, raw_ptr + numPoints, value); } else { @@ -193,8 +194,7 @@ inline void RecordComponent::loadChunk_impl( dRead.offset = offset; dRead.extent = extent; dRead.dtype = getDatatype(); - // @todo - // dRead.data = std::static_pointer_cast(data); + dRead.data = std::static_pointer_cast(data); rc.push_chunk(IOTask(this, dRead)); } } @@ -202,7 +202,11 @@ inline void RecordComponent::loadChunk_impl( template inline void RecordComponent::loadChunkRaw(T *ptr, Offset offset, Extent extent) { - loadChunk(auxiliary::shareRaw(ptr), std::move(offset), std::move(extent)); + prepareLoadStore() + .offset(std::move(offset)) + .extent(std::move(extent)) + .fromRawPtr(ptr) + .enqueueLoad(); } template diff --git a/src/LoadStoreChunk.cpp b/src/LoadStoreChunk.cpp index 6d9ab0e253..7917090597 100644 --- a/src/LoadStoreChunk.cpp +++ b/src/LoadStoreChunk.cpp @@ -216,15 +216,15 @@ OPENPMD_FOREACH_DATASET_DATATYPE(INSTANTIATE_METHOD_TEMPLATES_FOR_BASE) #undef INSTANTIATE_METHOD_TEMPLATES_FOR_BASE #define INSTANTIATE_STORE_CHUNK_FROM_BUFFER(dtype) \ - template class ConfigureLoadStoreFromBuffer>; \ + template class ConfigureLoadStoreFromBuffer>; \ template class ConfigureStoreChunkFromBuffer< \ - std::shared_ptr, \ - ConfigureLoadStoreFromBuffer>>; \ + std::shared_ptr, \ + ConfigureLoadStoreFromBuffer>>; \ template class ConfigureLoadStore< \ - ConfigureLoadStoreFromBuffer>>; \ + ConfigureLoadStoreFromBuffer>>; \ INSTANTIATE_METHOD_TEMPLATES( \ ConfigureLoadStore< \ - ConfigureLoadStoreFromBuffer>>, \ + ConfigureLoadStoreFromBuffer>>, \ dtype) \ template class ConfigureStoreChunkFromBuffer>; \ template class ConfigureLoadStore< \ @@ -232,6 +232,14 @@ OPENPMD_FOREACH_DATASET_DATATYPE(INSTANTIATE_METHOD_TEMPLATES_FOR_BASE) INSTANTIATE_METHOD_TEMPLATES( \ ConfigureLoadStore< \ ConfigureStoreChunkFromBuffer>>, \ + dtype) \ + template class ConfigureStoreChunkFromBuffer< \ + std::shared_ptr>; \ + template class ConfigureLoadStore< \ + ConfigureStoreChunkFromBuffer>>; \ + INSTANTIATE_METHOD_TEMPLATES( \ + ConfigureLoadStore< \ + ConfigureStoreChunkFromBuffer>>, \ dtype) OPENPMD_FOREACH_DATASET_DATATYPE(INSTANTIATE_STORE_CHUNK_FROM_BUFFER) diff --git a/src/binding/python/RecordComponent.cpp b/src/binding/python/RecordComponent.cpp index 05515c5dc6..44eaae3111 100644 --- a/src/binding/python/RecordComponent.cpp +++ b/src/binding/python/RecordComponent.cpp @@ -600,14 +600,6 @@ struct StoreChunkSpan static constexpr char const *errorMsg = "RecordComponent.store_chunk()"; }; - -template <> -PythonDynamicMemoryView StoreChunkSpan::call( - RecordComponent &, Offset const &, Extent const &) -{ - throw std::runtime_error( - "[RecordComponent.store_chunk()] Only PODs allowed."); -} } // namespace inline PythonDynamicMemoryView store_chunk_span( @@ -698,7 +690,7 @@ void load_chunk( } } - switchNonVectorType( + switchDatasetType( r.getDatatype(), r, buffer, buffer_info, offset, extent); } From 0a21ab04653a62835b890bab46c404951280bad3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Wed, 22 May 2024 15:44:47 +0200 Subject: [PATCH 17/39] Test new loadChunk syntax --- test/ParallelIOTest.cpp | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/test/ParallelIOTest.cpp b/test/ParallelIOTest.cpp index 3e462c978c..a7a0c5a390 100644 --- a/test/ParallelIOTest.cpp +++ b/test/ParallelIOTest.cpp @@ -499,9 +499,14 @@ void available_chunks_test(std::string const &file_ending) auto E_y = it0.meshes["E"]["y"]; auto width = E_y.getExtent()[1]; - auto first_row = E_y.loadChunk({0, 0}, {1, width}); - auto middle_rows = E_y.loadChunk({1, 0}, {3, width}); - auto last_row = E_y.loadChunk({4, 0}, {1, width}); + auto first_row = + E_y.prepareLoadStore().extent({1, width}).enqueueLoad(); + auto middle_rows = E_y.prepareLoadStore() + .offset({1, 0}) + .extent({3, width}) + .enqueueLoad(); + auto last_row = + E_y.prepareLoadStore().offset({4, 0}).enqueueLoad(); read.flush(); for (auto row : [&]() -> std::vector *> { From 181a25364b9468071d709b8a6b4f10fea797050c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Wed, 22 May 2024 15:56:10 +0200 Subject: [PATCH 18/39] Fix CI issues --- include/openPMD/RecordComponent.tpp | 6 ++++- src/LoadStoreChunk.cpp | 36 ++++++++++++++++++----------- 2 files changed, 28 insertions(+), 14 deletions(-) diff --git a/include/openPMD/RecordComponent.tpp b/include/openPMD/RecordComponent.tpp index 5a37e49ad7..c5e3a994c8 100644 --- a/include/openPMD/RecordComponent.tpp +++ b/include/openPMD/RecordComponent.tpp @@ -96,7 +96,11 @@ RecordComponent::loadChunkAllocate_impl(internal::LoadStoreConfig cfg) (defined(__apple_build_version__) && __clang_major__ < 14) auto newData = std::shared_ptr(new T[numPoints], [](T *p) { delete[] p; }); - loadChunk(newData, offset, extent); + prepareLoadStore() + .offset(std::move(o)) + .extent(std::move(e)) + .fromSharedPtr(newData) + .enqueueLoad(); return newData; #else auto newData = std::shared_ptr(new T[numPoints]); diff --git a/src/LoadStoreChunk.cpp b/src/LoadStoreChunk.cpp index 7917090597..c229844dab 100644 --- a/src/LoadStoreChunk.cpp +++ b/src/LoadStoreChunk.cpp @@ -108,14 +108,14 @@ auto ConfigureLoadStore::storeChunkConfig() template auto ConfigureLoadStore::extent(Extent extent) -> return_type & { - m_extent = std::move(extent); + m_extent = std::make_optional(std::move(extent)); return *static_cast(this); } template auto ConfigureLoadStore::offset(Offset offset) -> return_type & { - m_offset = std::move(offset); + m_offset = std::make_optional(std::move(offset)); return *static_cast(this); } @@ -215,23 +215,33 @@ OPENPMD_FOREACH_DATASET_DATATYPE(INSTANTIATE_METHOD_TEMPLATES_FOR_BASE) #undef INSTANTIATE_METHOD_TEMPLATES_FOR_BASE +/* + * In the following macro, we replace `dtype` with `std::remove_cv_t` since otherwise clang-tidy won't understand it's a type and we cannot + * surround it with parentheses. The type names are surrounded with angle + * brackets, so the warning is useless. + */ + #define INSTANTIATE_STORE_CHUNK_FROM_BUFFER(dtype) \ - template class ConfigureLoadStoreFromBuffer>; \ + template class ConfigureLoadStoreFromBuffer< \ + std::shared_ptr>>; \ template class ConfigureStoreChunkFromBuffer< \ std::shared_ptr, \ - ConfigureLoadStoreFromBuffer>>; \ - template class ConfigureLoadStore< \ - ConfigureLoadStoreFromBuffer>>; \ + ConfigureLoadStoreFromBuffer< \ + std::shared_ptr>>>; \ + template class ConfigureLoadStore>>>; \ INSTANTIATE_METHOD_TEMPLATES( \ - ConfigureLoadStore< \ - ConfigureLoadStoreFromBuffer>>, \ + ConfigureLoadStore>>>, \ dtype) \ - template class ConfigureStoreChunkFromBuffer>; \ - template class ConfigureLoadStore< \ - ConfigureStoreChunkFromBuffer>>; \ + template class ConfigureStoreChunkFromBuffer< \ + UniquePtrWithLambda>>; \ + template class ConfigureLoadStore>>>; \ INSTANTIATE_METHOD_TEMPLATES( \ - ConfigureLoadStore< \ - ConfigureStoreChunkFromBuffer>>, \ + ConfigureLoadStore>>>, \ dtype) \ template class ConfigureStoreChunkFromBuffer< \ std::shared_ptr>; \ From 095ecdd2cc4edb48589e09f0351a07bc010b9277 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Fri, 24 May 2024 13:34:28 +0200 Subject: [PATCH 19/39] Add variant-based loadChunk --- include/openPMD/LoadStoreChunk.hpp | 4 ++++ src/LoadStoreChunk.cpp | 23 +++++++++++++++++++++++ test/SerialIOTest.cpp | 16 ++++++++++------ 3 files changed, 37 insertions(+), 6 deletions(-) diff --git a/include/openPMD/LoadStoreChunk.hpp b/include/openPMD/LoadStoreChunk.hpp index 7818ce0137..197e3459fc 100644 --- a/include/openPMD/LoadStoreChunk.hpp +++ b/include/openPMD/LoadStoreChunk.hpp @@ -121,6 +121,10 @@ class ConfigureLoadStore : protected internal::ConfigureLoadStoreData template [[nodiscard]] auto enqueueLoad() -> std::shared_ptr; + + using shared_ptr_dataset_types = auxiliary::detail:: + map_variant::type; + [[nodiscard]] auto enqueueLoadVariant() -> shared_ptr_dataset_types; }; template diff --git a/src/LoadStoreChunk.cpp b/src/LoadStoreChunk.cpp index c229844dab..4d81371604 100644 --- a/src/LoadStoreChunk.cpp +++ b/src/LoadStoreChunk.cpp @@ -133,6 +133,29 @@ auto ConfigureLoadStore::enqueueLoad() -> std::shared_ptr return m_rc.loadChunkAllocate_impl(storeChunkConfig()); } +namespace +{ + template + struct VisitorEnqueueLoadVariant + { + template + static auto call(RecordComponent const &, ConfigureLoadStore_t &cfg) -> + typename ConfigureLoadStore_t::shared_ptr_dataset_types + { + return cfg.template enqueueLoad(); + } + }; +} // namespace + +template +auto ConfigureLoadStore::enqueueLoadVariant() + -> shared_ptr_dataset_types +{ + return m_rc + .visit>>( + *this); +} + template ConfigureStoreChunkFromBuffer:: ConfigureStoreChunkFromBuffer(Ptr_Type buffer, parent_t &&parent) diff --git a/test/SerialIOTest.cpp b/test/SerialIOTest.cpp index aaa9988472..b88383e709 100644 --- a/test/SerialIOTest.cpp +++ b/test/SerialIOTest.cpp @@ -1676,13 +1676,17 @@ inline void write_test(const std::string &backend) auto opaqueTypeDataset = rc.visit(); auto variantTypeDataset = rc.loadChunkVariant(); + auto variantTypeDataset2 = rc.prepareLoadStore().enqueueLoadVariant(); rc.seriesFlush(); - std::visit( - [](auto &&shared_ptr) { - std::cout << "First value in loaded chunk: '" << shared_ptr.get()[0] - << '\'' << std::endl; - }, - variantTypeDataset); + for (auto ptr : {&variantTypeDataset, &variantTypeDataset2}) + { + std::visit( + [](auto &&shared_ptr) { + std::cout << "First value in loaded chunk: '" + << shared_ptr.get()[0] << '\'' << std::endl; + }, + *ptr); + } #ifndef _WIN32 REQUIRE(read.rankTable(/* collective = */ false) == compare); From e9319edd14d76a76ba008ec8395ce3f831808c86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Fri, 24 May 2024 17:58:52 +0200 Subject: [PATCH 20/39] Renaming --- include/openPMD/LoadStoreChunk.hpp | 10 +++++----- include/openPMD/LoadStoreChunk.tpp | 14 +++++++------- include/openPMD/RecordComponent.tpp | 18 +++++++++--------- test/ParallelIOTest.cpp | 6 +++--- 4 files changed, 24 insertions(+), 24 deletions(-) diff --git a/include/openPMD/LoadStoreChunk.hpp b/include/openPMD/LoadStoreChunk.hpp index 197e3459fc..864b7f9472 100644 --- a/include/openPMD/LoadStoreChunk.hpp +++ b/include/openPMD/LoadStoreChunk.hpp @@ -99,15 +99,15 @@ class ConfigureLoadStore : protected internal::ConfigureLoadStoreData // @todo rvalue references..? template - auto fromSharedPtr(std::shared_ptr) -> shared_ptr_return_type; + auto withSharedPtr(std::shared_ptr) -> shared_ptr_return_type; template - auto fromUniquePtr(UniquePtrWithLambda) -> unique_ptr_return_type; + auto withUniquePtr(UniquePtrWithLambda) -> unique_ptr_return_type; template - auto fromUniquePtr(std::unique_ptr) -> unique_ptr_return_type; + auto withUniquePtr(std::unique_ptr) -> unique_ptr_return_type; template - auto fromRawPtr(T *data) -> shared_ptr_return_type; + auto withRawPtr(T *data) -> shared_ptr_return_type; template - auto fromContiguousContainer(T_ContiguousContainer &data) + auto withContiguousContainer(T_ContiguousContainer &data) -> std::enable_if_t< auxiliary::IsContiguousContainer_v, shared_ptr_return_type>; diff --git a/include/openPMD/LoadStoreChunk.tpp b/include/openPMD/LoadStoreChunk.tpp index 63b4b14263..f91f61b5e0 100644 --- a/include/openPMD/LoadStoreChunk.tpp +++ b/include/openPMD/LoadStoreChunk.tpp @@ -6,7 +6,7 @@ namespace openPMD { template template -auto ConfigureLoadStore::fromSharedPtr(std::shared_ptr data) +auto ConfigureLoadStore::withSharedPtr(std::shared_ptr data) -> shared_ptr_return_type { if (!data) @@ -20,7 +20,7 @@ auto ConfigureLoadStore::fromSharedPtr(std::shared_ptr data) } template template -auto ConfigureLoadStore::fromUniquePtr(UniquePtrWithLambda data) +auto ConfigureLoadStore::withUniquePtr(UniquePtrWithLambda data) -> unique_ptr_return_type { @@ -35,7 +35,7 @@ auto ConfigureLoadStore::fromUniquePtr(UniquePtrWithLambda data) } template template -auto ConfigureLoadStore::fromRawPtr(T *data) +auto ConfigureLoadStore::withRawPtr(T *data) -> shared_ptr_return_type { if (!data) @@ -49,14 +49,14 @@ auto ConfigureLoadStore::fromRawPtr(T *data) template template -auto ConfigureLoadStore::fromUniquePtr(std::unique_ptr data) +auto ConfigureLoadStore::withUniquePtr(std::unique_ptr data) -> unique_ptr_return_type { - return fromUniquePtr(UniquePtrWithLambda(std::move(data))); + return withUniquePtr(UniquePtrWithLambda(std::move(data))); } template template -auto ConfigureLoadStore::fromContiguousContainer( +auto ConfigureLoadStore::withContiguousContainer( T_ContiguousContainer &data) -> std::enable_if_t< auxiliary::IsContiguousContainer_v, @@ -66,6 +66,6 @@ auto ConfigureLoadStore::fromContiguousContainer( { m_extent = Extent{data.size()}; } - return fromRawPtr(data.data()); + return withRawPtr(data.data()); } } // namespace openPMD diff --git a/include/openPMD/RecordComponent.tpp b/include/openPMD/RecordComponent.tpp index c5e3a994c8..c02910d9ac 100644 --- a/include/openPMD/RecordComponent.tpp +++ b/include/openPMD/RecordComponent.tpp @@ -99,7 +99,7 @@ RecordComponent::loadChunkAllocate_impl(internal::LoadStoreConfig cfg) prepareLoadStore() .offset(std::move(o)) .extent(std::move(e)) - .fromSharedPtr(newData) + .withSharedPtr(newData) .enqueueLoad(); return newData; #else @@ -107,7 +107,7 @@ RecordComponent::loadChunkAllocate_impl(internal::LoadStoreConfig cfg) prepareLoadStore() .offset(std::move(o)) .extent(std::move(e)) - .fromSharedPtr(newData) + .withSharedPtr(newData) .enqueueLoad(); return std::static_pointer_cast(std::move(newData)); #endif @@ -134,7 +134,7 @@ inline void RecordComponent::loadChunk( operation.extent(std::move(e)); } - operation.fromSharedPtr(std::move(data)).enqueueLoad(); + operation.withSharedPtr(std::move(data)).enqueueLoad(); } template @@ -209,7 +209,7 @@ inline void RecordComponent::loadChunkRaw(T *ptr, Offset offset, Extent extent) prepareLoadStore() .offset(std::move(offset)) .extent(std::move(extent)) - .fromRawPtr(ptr) + .withRawPtr(ptr) .enqueueLoad(); } @@ -220,7 +220,7 @@ RecordComponent::storeChunk(std::shared_ptr data, Offset o, Extent e) prepareLoadStore() .offset(std::move(o)) .extent(std::move(e)) - .fromSharedPtr(std::move(data)) + .withSharedPtr(std::move(data)) .enqueueStore(); } @@ -231,7 +231,7 @@ RecordComponent::storeChunk(UniquePtrWithLambda data, Offset o, Extent e) prepareLoadStore() .offset(std::move(o)) .extent(std::move(e)) - .fromUniquePtr(std::move(data)) + .withUniquePtr(std::move(data)) .enqueueStore(); } @@ -242,7 +242,7 @@ RecordComponent::storeChunk(std::unique_ptr data, Offset o, Extent e) prepareLoadStore() .offset(std::move(o)) .extent(std::move(e)) - .fromUniquePtr(std::move(data)) + .withUniquePtr(std::move(data)) .enqueueStore(); } @@ -252,7 +252,7 @@ void RecordComponent::storeChunkRaw(T *ptr, Offset offset, Extent extent) prepareLoadStore() .offset(std::move(offset)) .extent(std::move(extent)) - .fromRawPtr(ptr) + .withRawPtr(ptr) .enqueueStore(); } @@ -273,7 +273,7 @@ RecordComponent::storeChunk(T_ContiguousContainer &data, Offset o, Extent e) storeChunkConfig.extent(std::move(e)); } - std::move(storeChunkConfig).fromContiguousContainer(data).enqueueStore(); + std::move(storeChunkConfig).withContiguousContainer(data).enqueueStore(); } template diff --git a/test/ParallelIOTest.cpp b/test/ParallelIOTest.cpp index a7a0c5a390..8aeeb23dda 100644 --- a/test/ParallelIOTest.cpp +++ b/test/ParallelIOTest.cpp @@ -445,14 +445,14 @@ void available_chunks_test(std::string const &file_ending) auto E_y = it0.meshes["E"]["y"]; E_y.resetDataset({Datatype::INT, {5, 3ul * mpi_size}}); E_y.prepareLoadStore() - .fromContiguousContainer(ydata_firstandlastrow) + .withContiguousContainer(ydata_firstandlastrow) .offset({0, 3ul * mpi_rank}) .extent({1, 3}) .enqueueStore(); E_y.prepareLoadStore() .offset({1, 3ul * mpi_rank}) .extent({3, 3}) - .fromContiguousContainer(ydata) + .withContiguousContainer(ydata) .memorySelection({{1, 1}, {5, 5}}) .enqueueStore(); // if condition checks if this PR is available in ADIOS2: @@ -460,7 +460,7 @@ void available_chunks_test(std::string const &file_ending) if constexpr (CanTheMemorySelectionBeReset) { E_y.prepareLoadStore() - .fromContiguousContainer(ydata_firstandlastrow) + .withContiguousContainer(ydata_firstandlastrow) .offset({4, 3ul * mpi_rank}) .extent({1, 3}) .enqueueStore(); From 0cdae967d6df13761e6c101195eca64fdc7df1c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Mon, 24 Jun 2024 14:04:19 +0200 Subject: [PATCH 21/39] Use another workaround for fixing the wrong lint --- src/LoadStoreChunk.cpp | 42 +++++++++++++++++++++--------------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/src/LoadStoreChunk.cpp b/src/LoadStoreChunk.cpp index 4d81371604..f1b6e12b5d 100644 --- a/src/LoadStoreChunk.cpp +++ b/src/LoadStoreChunk.cpp @@ -238,33 +238,32 @@ OPENPMD_FOREACH_DATASET_DATATYPE(INSTANTIATE_METHOD_TEMPLATES_FOR_BASE) #undef INSTANTIATE_METHOD_TEMPLATES_FOR_BASE -/* - * In the following macro, we replace `dtype` with `std::remove_cv_t` since otherwise clang-tidy won't understand it's a type and we cannot - * surround it with parentheses. The type names are surrounded with angle - * brackets, so the warning is useless. - */ - +/* clang-format would destroy the NOLINT comments */ +// clang-format off #define INSTANTIATE_STORE_CHUNK_FROM_BUFFER(dtype) \ - template class ConfigureLoadStoreFromBuffer< \ - std::shared_ptr>>; \ + /* NOLINTNEXTLINE(bugprone-macro-parentheses) */ \ + template class ConfigureLoadStoreFromBuffer>; \ template class ConfigureStoreChunkFromBuffer< \ std::shared_ptr, \ - ConfigureLoadStoreFromBuffer< \ - std::shared_ptr>>>; \ - template class ConfigureLoadStore>>>; \ + /* NOLINTNEXTLINE(bugprone-macro-parentheses) */ \ + ConfigureLoadStoreFromBuffer>>; \ + template class ConfigureLoadStore< \ + /* NOLINTNEXTLINE(bugprone-macro-parentheses) */ \ + ConfigureLoadStoreFromBuffer>>; \ INSTANTIATE_METHOD_TEMPLATES( \ - ConfigureLoadStore>>>, \ + ConfigureLoadStore< \ + /* NOLINTNEXTLINE(bugprone-macro-parentheses) */ \ + ConfigureLoadStoreFromBuffer>>, \ dtype) \ - template class ConfigureStoreChunkFromBuffer< \ - UniquePtrWithLambda>>; \ - template class ConfigureLoadStore>>>; \ + /* NOLINTNEXTLINE(bugprone-macro-parentheses) */ \ + template class ConfigureStoreChunkFromBuffer>; \ + template class ConfigureLoadStore< \ + /* NOLINTNEXTLINE(bugprone-macro-parentheses) */ \ + ConfigureStoreChunkFromBuffer>>; \ INSTANTIATE_METHOD_TEMPLATES( \ - ConfigureLoadStore>>>, \ + ConfigureLoadStore< \ + /* NOLINTNEXTLINE(bugprone-macro-parentheses) */ \ + ConfigureStoreChunkFromBuffer>>, \ dtype) \ template class ConfigureStoreChunkFromBuffer< \ std::shared_ptr>; \ @@ -274,6 +273,7 @@ OPENPMD_FOREACH_DATASET_DATATYPE(INSTANTIATE_METHOD_TEMPLATES_FOR_BASE) ConfigureLoadStore< \ ConfigureStoreChunkFromBuffer>>, \ dtype) +// clang-format on OPENPMD_FOREACH_DATASET_DATATYPE(INSTANTIATE_STORE_CHUNK_FROM_BUFFER) From 7a6196cfe5a48044ce47cd46f8c81fe6090935e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Mon, 24 Jun 2024 15:02:30 +0200 Subject: [PATCH 22/39] Guard against memory selections in reading --- include/openPMD/RecordComponent.tpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/include/openPMD/RecordComponent.tpp b/include/openPMD/RecordComponent.tpp index c02910d9ac..4786fd47cf 100644 --- a/include/openPMD/RecordComponent.tpp +++ b/include/openPMD/RecordComponent.tpp @@ -142,6 +142,11 @@ inline void RecordComponent::loadChunk_impl( std::shared_ptr data, internal::LoadStoreConfigWithBuffer cfg) { + if (cfg.memorySelection.has_value()) + { + throw error::WrongAPIUsage( + "Unsupported: Memory selections in chunk loading."); + } using T = std::remove_cv_t>; Datatype dtype = determineDatatype(data); if (dtype != getDatatype()) From 2e1a6008f19e4db6e711c5d20bfced0f46dadd28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Mon, 24 Jun 2024 16:33:48 +0200 Subject: [PATCH 23/39] Some documentation and cleanup --- include/openPMD/LoadStoreChunk.hpp | 69 ++++++++++++++++++++++++------ include/openPMD/LoadStoreChunk.tpp | 6 ++- 2 files changed, 61 insertions(+), 14 deletions(-) diff --git a/include/openPMD/LoadStoreChunk.hpp b/include/openPMD/LoadStoreChunk.hpp index 864b7f9472..c9f2d30eda 100644 --- a/include/openPMD/LoadStoreChunk.hpp +++ b/include/openPMD/LoadStoreChunk.hpp @@ -32,6 +32,12 @@ namespace internal std::optional memorySelection; }; + /* + * Actual data members of `ConfigureLoadStore<>`. They don't depend on the + * template parameter of that class template, so by extracting the members + * to this struct, we can pass them around between different instances of + * the class template. + */ struct ConfigureLoadStoreData { ConfigureLoadStoreData(RecordComponent &); @@ -42,6 +48,13 @@ namespace internal }; } // namespace internal +/** Basic configuration for a Load/Store operation. + * + * @tparam ChildClass CRT pattern. + * The purpose is that in child classes `return *this` should return + * an instance of the child class, not of ConfigureLoadStore. + * Instantiate with void when using without subclass. + */ template class ConfigureLoadStore : protected internal::ConfigureLoadStoreData { @@ -53,7 +66,7 @@ class ConfigureLoadStore : protected internal::ConfigureLoadStoreData ConfigureLoadStore(RecordComponent &rc); ConfigureLoadStore(ConfigureLoadStoreData &&); - auto dim() const -> uint8_t; + [[nodiscard]] auto dim() const -> uint8_t; auto getOffset() -> Offset const &; auto getExtent() -> Extent const &; auto storeChunkConfig() -> internal::LoadStoreConfig; @@ -67,34 +80,37 @@ class ConfigureLoadStore : protected internal::ConfigureLoadStoreData auto offset(Offset) -> return_type &; auto extent(Extent) -> return_type &; + /* + * If the type is non-const, then the return type should be + * ConfigureLoadStoreFromBuffer<>, ... + */ template struct shared_ptr_return_type_impl { - using type = ConfigureLoadStoreFromBuffer< - std::shared_ptr>>; + using type = ConfigureLoadStoreFromBuffer>; }; + /* + * ..., but if it is a const type, Load operations make no sense, so the + * return type should be ConfigureStoreChunkFromBuffer<>. + */ template struct shared_ptr_return_type_impl { using type = ConfigureStoreChunkFromBuffer, void>; }; - template - struct shared_ptr_return_type_impl - { - using type = - ConfigureStoreChunkFromBuffer, void>; - }; template using shared_ptr_return_type = - typename shared_ptr_return_type_impl::type; + typename shared_ptr_return_type_impl>::type; - template - using normalize_dataset_type = std::remove_cv_t>; + /* + * As loading into unique pointer types makes no sense, the case is simpler + * for unique pointers. Just remove the array extents here. + */ template using unique_ptr_return_type = ConfigureStoreChunkFromBuffer< - UniquePtrWithLambda>, + UniquePtrWithLambda>, void>; // @todo rvalue references..? @@ -127,6 +143,19 @@ class ConfigureLoadStore : protected internal::ConfigureLoadStoreData [[nodiscard]] auto enqueueLoadVariant() -> shared_ptr_dataset_types; }; +/** Configuration for a Store operation with a buffer type. + * + * This class does intentionally not support Load operations since there are + * pointer types (const pointers, unique pointers) where Load operations make no + * sense. See the \ref ConfigureLoadStoreFromBuffer class template for both + * Load/Store operations. + * + * @tparam Ptr_Type The type of pointer used internally. + * @tparam ChildClass CRT pattern. + * The purpose is that in child classes `return *this` should return + * an instance of the child class, not of ConfigureStoreChunkFromBuffer. + * Instantiate with void when using without subclass. + */ template class ConfigureStoreChunkFromBuffer : public ConfigureLoadStore parent_t const &; auto enqueueStore() -> void; + + /** This intentionally shadows the parent class's enqueueLoad method in + * order to show a compile error when using enqueueLoad() on an object of + * this class. The parent method can still be accessed through as_parent() + * if needed. + */ template auto enqueueLoad() { @@ -171,6 +206,14 @@ class ConfigureStoreChunkFromBuffer } }; +/** Configuration for a Load/Store operation with a buffer type. + * + * Only instantiated for pointer types where Load operations make sense (e.g. no + * const pointers and no unique pointers). + * \ref ConfigureStoreChunkFromBuffer is used otherwise. + * + * @tparam Ptr_Type The type of pointer used internally. + */ template class ConfigureLoadStoreFromBuffer : public ConfigureStoreChunkFromBuffer< diff --git a/include/openPMD/LoadStoreChunk.tpp b/include/openPMD/LoadStoreChunk.tpp index f91f61b5e0..382d899deb 100644 --- a/include/openPMD/LoadStoreChunk.tpp +++ b/include/openPMD/LoadStoreChunk.tpp @@ -24,13 +24,17 @@ auto ConfigureLoadStore::withUniquePtr(UniquePtrWithLambda data) -> unique_ptr_return_type { + // should we support them? + static_assert( + !std::is_const_v, + "Unique pointers to const types not supported as storeChunk buffers."); if (!data) { throw std::runtime_error( "Unallocated pointer passed during chunk store."); } return unique_ptr_return_type( - std::move(data).template static_cast_>(), + std::move(data).template static_cast_>(), {std::move(*this)}); } template From 6a2a886a5eb472b54c301530821c88e3ba23aae4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Mon, 24 Jun 2024 16:53:53 +0200 Subject: [PATCH 24/39] Frontend support for unique pointers with const value types --- include/openPMD/LoadStoreChunk.tpp | 4 ---- src/LoadStoreChunk.cpp | 24 ++++++++++++++++++++++++ test/SerialIOTest.cpp | 6 +++++- 3 files changed, 29 insertions(+), 5 deletions(-) diff --git a/include/openPMD/LoadStoreChunk.tpp b/include/openPMD/LoadStoreChunk.tpp index 382d899deb..d6cb992896 100644 --- a/include/openPMD/LoadStoreChunk.tpp +++ b/include/openPMD/LoadStoreChunk.tpp @@ -24,10 +24,6 @@ auto ConfigureLoadStore::withUniquePtr(UniquePtrWithLambda data) -> unique_ptr_return_type { - // should we support them? - static_assert( - !std::is_const_v, - "Unique pointers to const types not supported as storeChunk buffers."); if (!data) { throw std::runtime_error( diff --git a/src/LoadStoreChunk.cpp b/src/LoadStoreChunk.cpp index f1b6e12b5d..f65fd8cde5 100644 --- a/src/LoadStoreChunk.cpp +++ b/src/LoadStoreChunk.cpp @@ -41,6 +41,22 @@ namespace return auxiliary::WriteBuffer( std::move(ptr).template static_cast_()); } + + /* + * There is no backend support currently for const unique pointers. + * We support these mostly for providing a clean API to users that have such + * pointers and want to store from them, but there will be no + * backend-specific optimizations for such buffers as there are for + * non-const unique pointers. + */ + template + auto + asWriteBuffer(UniquePtrWithLambda &&ptr) -> auxiliary::WriteBuffer + { + auto raw_ptr = ptr.get(); + return asWriteBuffer(std::shared_ptr{ + raw_ptr, [ptr_lambda = std::move(ptr)](auto const *) {}}); + } } // namespace template @@ -272,6 +288,14 @@ OPENPMD_FOREACH_DATASET_DATATYPE(INSTANTIATE_METHOD_TEMPLATES_FOR_BASE) INSTANTIATE_METHOD_TEMPLATES( \ ConfigureLoadStore< \ ConfigureStoreChunkFromBuffer>>, \ + dtype) \ + template class ConfigureStoreChunkFromBuffer< \ + UniquePtrWithLambda>; \ + template class ConfigureLoadStore< \ + ConfigureStoreChunkFromBuffer>>; \ + INSTANTIATE_METHOD_TEMPLATES( \ + ConfigureLoadStore< \ + ConfigureStoreChunkFromBuffer>>, \ dtype) // clang-format on diff --git a/test/SerialIOTest.cpp b/test/SerialIOTest.cpp index b88383e709..c50eebc539 100644 --- a/test/SerialIOTest.cpp +++ b/test/SerialIOTest.cpp @@ -928,7 +928,11 @@ inline void constant_scalar(std::string const &file_ending) new unsigned int[6], [](unsigned int const *p) { delete[] p; }); unsigned int e{0}; std::generate(E.get(), E.get() + 6, [&e] { return e++; }); - E_y.storeChunk(std::move(E), {0, 0, 0}, {1, 2, 3}); + // check that const-type unique pointers work in the builder pattern + E_y.prepareLoadStore() + .extent({1, 2, 3}) + .withUniquePtr(std::move(E).static_cast_()) + .enqueueStore(); // store a number of predefined attributes in E Mesh &E_mesh = s.iterations[1].meshes["E"]; From ab646483fd58e5027f625f5b581bbeff19674b68 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Tue, 25 Jun 2024 15:55:39 +0200 Subject: [PATCH 25/39] Workaround for annoying compilers that dont move --- src/LoadStoreChunk.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/LoadStoreChunk.cpp b/src/LoadStoreChunk.cpp index f65fd8cde5..056a0e713b 100644 --- a/src/LoadStoreChunk.cpp +++ b/src/LoadStoreChunk.cpp @@ -53,9 +53,12 @@ namespace auto asWriteBuffer(UniquePtrWithLambda &&ptr) -> auxiliary::WriteBuffer { - auto raw_ptr = ptr.get(); + auto raw_ptr = ptr.release(); return asWriteBuffer(std::shared_ptr{ - raw_ptr, [ptr_lambda = std::move(ptr)](auto const *) {}}); + raw_ptr, + [deleter = std::move(ptr.get_deleter())](auto const *delete_me) { + deleter(delete_me); + }}); } } // namespace From ef1c443e6b8d7bc961f9f88d4af01f32d8932c7f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Fri, 19 Jul 2024 14:11:43 +0200 Subject: [PATCH 26/39] Transform load calls to use std::future --- include/openPMD/LoadStoreChunk.hpp | 25 ++++++-- include/openPMD/RecordComponent.hpp | 1 + include/openPMD/RecordComponent.tpp | 10 +-- src/LoadStoreChunk.cpp | 94 +++++++++++++++++++++++------ test/ParallelIOTest.cpp | 7 ++- test/SerialIOTest.cpp | 2 +- 6 files changed, 107 insertions(+), 32 deletions(-) diff --git a/include/openPMD/LoadStoreChunk.hpp b/include/openPMD/LoadStoreChunk.hpp index c9f2d30eda..b893e7250a 100644 --- a/include/openPMD/LoadStoreChunk.hpp +++ b/include/openPMD/LoadStoreChunk.hpp @@ -4,6 +4,7 @@ #include "openPMD/auxiliary/ShareRawInternal.hpp" #include "openPMD/auxiliary/UniquePtr.hpp" +#include #include #include #include @@ -48,6 +49,18 @@ namespace internal }; } // namespace internal +namespace auxiliary::detail +{ + using future_to_shared_ptr_dataset_types = + map_variant::type; +} // namespace auxiliary::detail + +enum class EnqueuePolicy +{ + Defer, + Immediate +}; + /** Basic configuration for a Load/Store operation. * * @tparam ChildClass CRT pattern. @@ -136,11 +149,13 @@ class ConfigureLoadStore : protected internal::ConfigureLoadStoreData [[nodiscard]] auto enqueueStore(F &&createBuffer) -> DynamicMemoryView; template - [[nodiscard]] auto enqueueLoad() -> std::shared_ptr; + [[nodiscard]] auto enqueueLoad() -> std::future>; + + template + [[nodiscard]] auto load(EnqueuePolicy) -> std::shared_ptr; - using shared_ptr_dataset_types = auxiliary::detail:: - map_variant::type; - [[nodiscard]] auto enqueueLoadVariant() -> shared_ptr_dataset_types; + [[nodiscard]] auto enqueueLoadVariant() + -> std::future; }; /** Configuration for a Store operation with a buffer type. @@ -230,6 +245,8 @@ class ConfigureLoadStoreFromBuffer public: auto enqueueLoad() -> void; + + auto load(EnqueuePolicy) -> void; }; } // namespace openPMD diff --git a/include/openPMD/RecordComponent.hpp b/include/openPMD/RecordComponent.hpp index 7ad892b51c..5d8ac53740 100644 --- a/include/openPMD/RecordComponent.hpp +++ b/include/openPMD/RecordComponent.hpp @@ -142,6 +142,7 @@ class RecordComponent : public BaseRecordComponent friend class ConfigureStoreChunkFromBuffer; template friend class ConfigureLoadStoreFromBuffer; + friend struct VisitorEnqueueLoadVariant; public: enum class Allocation diff --git a/include/openPMD/RecordComponent.tpp b/include/openPMD/RecordComponent.tpp index 4786fd47cf..c91ccfcb7e 100644 --- a/include/openPMD/RecordComponent.tpp +++ b/include/openPMD/RecordComponent.tpp @@ -76,7 +76,7 @@ inline std::shared_ptr RecordComponent::loadChunk(Offset o, Extent e) operation.extent(std::move(e)); } - return operation.enqueueLoad(); + return operation.load(EnqueuePolicy::Defer); } template @@ -100,7 +100,7 @@ RecordComponent::loadChunkAllocate_impl(internal::LoadStoreConfig cfg) .offset(std::move(o)) .extent(std::move(e)) .withSharedPtr(newData) - .enqueueLoad(); + .load(EnqueuePolicy::Defer); return newData; #else auto newData = std::shared_ptr(new T[numPoints]); @@ -108,7 +108,7 @@ RecordComponent::loadChunkAllocate_impl(internal::LoadStoreConfig cfg) .offset(std::move(o)) .extent(std::move(e)) .withSharedPtr(newData) - .enqueueLoad(); + .load(EnqueuePolicy::Defer); return std::static_pointer_cast(std::move(newData)); #endif } @@ -134,7 +134,7 @@ inline void RecordComponent::loadChunk( operation.extent(std::move(e)); } - operation.withSharedPtr(std::move(data)).enqueueLoad(); + operation.withSharedPtr(std::move(data)).load(EnqueuePolicy::Defer); } template @@ -215,7 +215,7 @@ inline void RecordComponent::loadChunkRaw(T *ptr, Offset offset, Extent extent) .offset(std::move(offset)) .extent(std::move(extent)) .withRawPtr(ptr) - .enqueueLoad(); + .load(EnqueuePolicy::Defer); } template diff --git a/src/LoadStoreChunk.cpp b/src/LoadStoreChunk.cpp index 056a0e713b..85c4fc9aa1 100644 --- a/src/LoadStoreChunk.cpp +++ b/src/LoadStoreChunk.cpp @@ -12,6 +12,7 @@ // comment to keep clang-format from reordering #include "openPMD/DatatypeMacros.hpp" +#include #include #include #include @@ -147,32 +148,65 @@ auto ConfigureLoadStore::enqueueStore() -> DynamicMemoryView template template -auto ConfigureLoadStore::enqueueLoad() -> std::shared_ptr +auto ConfigureLoadStore::enqueueLoad() + -> std::future> { - return m_rc.loadChunkAllocate_impl(storeChunkConfig()); + auto res = m_rc.loadChunkAllocate_impl(storeChunkConfig()); + return std::async( + std::launch::deferred, + [res_lambda = std::move(res), rc = m_rc]() mutable { + rc.seriesFlush(); + return res_lambda; + }); } -namespace +template +template +auto ConfigureLoadStore::load(EnqueuePolicy ep) + -> std::shared_ptr { - template - struct VisitorEnqueueLoadVariant + auto res = m_rc.loadChunkAllocate_impl(storeChunkConfig()); + switch (ep) { - template - static auto call(RecordComponent const &, ConfigureLoadStore_t &cfg) -> - typename ConfigureLoadStore_t::shared_ptr_dataset_types - { - return cfg.template enqueueLoad(); - } - }; -} // namespace + case EnqueuePolicy::Defer: + break; + case EnqueuePolicy::Immediate: + m_rc.seriesFlush(); + break; + } + return res; +} + +struct VisitorEnqueueLoadVariant +{ + template + static auto call(RecordComponent &rc, internal::LoadStoreConfig cfg) + -> std::future + { + auto res = rc.loadChunkAllocate_impl(std::move(cfg)); + return std::async( + std::launch::deferred, + [res_lambda = std::move(res), rc_lambda = rc]() mutable + -> auxiliary::detail::future_to_shared_ptr_dataset_types { + rc_lambda.seriesFlush(); + return res_lambda; + }); + } + + static auto non_templated_implementation( + RecordComponent &rc, internal::LoadStoreConfig cfg) + -> std::future + { + return rc.visit(std::move(cfg)); + } +}; template auto ConfigureLoadStore::enqueueLoadVariant() - -> shared_ptr_dataset_types + -> std::future { - return m_rc - .visit>>( - *this); + return VisitorEnqueueLoadVariant::non_templated_implementation( + m_rc, this->storeChunkConfig()); } template @@ -245,9 +279,31 @@ auto ConfigureLoadStoreFromBuffer::enqueueLoad() -> void std::move(this->m_buffer), this->storeChunkConfig()); } +template +auto ConfigureLoadStoreFromBuffer::load(EnqueuePolicy ep) -> void +{ + this->m_rc.loadChunk_impl( + std::move(this->m_buffer), this->storeChunkConfig()); + switch (ep) + { + + case EnqueuePolicy::Defer: + break; + case EnqueuePolicy::Immediate: + this->m_rc.seriesFlush(); + break; + } +} + +/* clang-format would destroy the NOLINT comments */ +// clang-format off #define INSTANTIATE_METHOD_TEMPLATES(base_class, dtype) \ - template auto base_class::enqueueStore() -> DynamicMemoryView; \ - template auto base_class::enqueueLoad() -> std::shared_ptr; + template auto base_class::enqueueStore()->DynamicMemoryView; \ + template auto base_class::enqueueLoad() \ + /* NOLINTNEXTLINE(bugprone-macro-parentheses) */ \ + ->std::future>; \ + template auto base_class::load(EnqueuePolicy)->std::shared_ptr; +// clang-format on #define INSTANTIATE_METHOD_TEMPLATES_FOR_BASE(dtype) \ INSTANTIATE_METHOD_TEMPLATES(ConfigureLoadStore, dtype) diff --git a/test/ParallelIOTest.cpp b/test/ParallelIOTest.cpp index 8aeeb23dda..0ae244d46e 100644 --- a/test/ParallelIOTest.cpp +++ b/test/ParallelIOTest.cpp @@ -500,13 +500,14 @@ void available_chunks_test(std::string const &file_ending) auto E_y = it0.meshes["E"]["y"]; auto width = E_y.getExtent()[1]; auto first_row = - E_y.prepareLoadStore().extent({1, width}).enqueueLoad(); + E_y.prepareLoadStore().extent({1, width}).enqueueLoad().get(); auto middle_rows = E_y.prepareLoadStore() .offset({1, 0}) .extent({3, width}) - .enqueueLoad(); + .enqueueLoad() + .get(); auto last_row = - E_y.prepareLoadStore().offset({4, 0}).enqueueLoad(); + E_y.prepareLoadStore().offset({4, 0}).enqueueLoad().get(); read.flush(); for (auto row : [&]() -> std::vector *> { diff --git a/test/SerialIOTest.cpp b/test/SerialIOTest.cpp index c50eebc539..653ec55e9e 100644 --- a/test/SerialIOTest.cpp +++ b/test/SerialIOTest.cpp @@ -1680,7 +1680,7 @@ inline void write_test(const std::string &backend) auto opaqueTypeDataset = rc.visit(); auto variantTypeDataset = rc.loadChunkVariant(); - auto variantTypeDataset2 = rc.prepareLoadStore().enqueueLoadVariant(); + auto variantTypeDataset2 = rc.prepareLoadStore().enqueueLoadVariant().get(); rc.seriesFlush(); for (auto ptr : {&variantTypeDataset, &variantTypeDataset2}) { From a61325f138dca19785d6579027bda340101b02bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Fri, 19 Jul 2024 16:08:51 +0200 Subject: [PATCH 27/39] Extract non-template members to ConfigureStoreChunkCore --- include/openPMD/LoadStoreChunk.hpp | 80 +++++++++++++-------------- include/openPMD/LoadStoreChunk.tpp | 16 ++---- include/openPMD/RecordComponent.hpp | 1 + include/openPMD/RecordComponent.tpp | 3 +- src/LoadStoreChunk.cpp | 83 +++++++++-------------------- 5 files changed, 71 insertions(+), 112 deletions(-) diff --git a/include/openPMD/LoadStoreChunk.hpp b/include/openPMD/LoadStoreChunk.hpp index b893e7250a..6918b62534 100644 --- a/include/openPMD/LoadStoreChunk.hpp +++ b/include/openPMD/LoadStoreChunk.hpp @@ -33,20 +33,6 @@ namespace internal std::optional memorySelection; }; - /* - * Actual data members of `ConfigureLoadStore<>`. They don't depend on the - * template parameter of that class template, so by extracting the members - * to this struct, we can pass them around between different instances of - * the class template. - */ - struct ConfigureLoadStoreData - { - ConfigureLoadStoreData(RecordComponent &); - - RecordComponent &m_rc; - std::optional m_offset; - std::optional m_extent; - }; } // namespace internal namespace auxiliary::detail @@ -61,38 +47,27 @@ enum class EnqueuePolicy Immediate }; -/** Basic configuration for a Load/Store operation. - * - * @tparam ChildClass CRT pattern. - * The purpose is that in child classes `return *this` should return - * an instance of the child class, not of ConfigureLoadStore. - * Instantiate with void when using without subclass. +/* + * Actual data members of `ConfigureLoadStore<>` and methods that don't depend + * on the ChildClass template parameter. By extracting the members to this + * struct, we can pass them around between different instances of the class + * template. Numbers of method instantiations can be reduced. */ -template -class ConfigureLoadStore : protected internal::ConfigureLoadStoreData +struct ConfigureLoadStoreCore { - friend class RecordComponent; - template - friend class ConfigureLoadStore; + ConfigureLoadStoreCore(RecordComponent &); -protected: - ConfigureLoadStore(RecordComponent &rc); - ConfigureLoadStore(ConfigureLoadStoreData &&); + RecordComponent &m_rc; + std::optional m_offset; + std::optional m_extent; +protected: [[nodiscard]] auto dim() const -> uint8_t; auto getOffset() -> Offset const &; auto getExtent() -> Extent const &; auto storeChunkConfig() -> internal::LoadStoreConfig; public: - using return_type = std::conditional_t< - std::is_void_v, - /*then*/ ConfigureLoadStore, - /*else*/ ChildClass>; - - auto offset(Offset) -> return_type &; - auto extent(Extent) -> return_type &; - /* * If the type is non-const, then the return type should be * ConfigureLoadStoreFromBuffer<>, ... @@ -158,6 +133,33 @@ class ConfigureLoadStore : protected internal::ConfigureLoadStoreData -> std::future; }; +/** Basic configuration for a Load/Store operation. + * + * @tparam ChildClass CRT pattern. + * The purpose is that in child classes `return *this` should return + * an instance of the child class, not of ConfigureLoadStore. + * Instantiate with void when using without subclass. + */ +template +class ConfigureLoadStore : public ConfigureLoadStoreCore +{ + friend class RecordComponent; + friend struct ConfigureLoadStoreCore; + +protected: + ConfigureLoadStore(RecordComponent &rc); + ConfigureLoadStore(ConfigureLoadStoreCore &&); + +public: + using return_type = std::conditional_t< + std::is_void_v, + /*then*/ ConfigureLoadStore, + /*else*/ ChildClass>; + + auto offset(Offset) -> return_type &; + auto extent(Extent) -> return_type &; +}; + /** Configuration for a Store operation with a buffer type. * * This class does intentionally not support Load operations since there are @@ -186,8 +188,7 @@ class ConfigureStoreChunkFromBuffer using parent_t = ConfigureLoadStore; protected: - template - friend class ConfigureLoadStore; + friend struct ConfigureLoadStoreCore; Ptr_Type m_buffer; std::optional m_mem_select; @@ -238,8 +239,7 @@ class ConfigureLoadStoreFromBuffer using parent_t = ConfigureStoreChunkFromBuffer< Ptr_Type, ConfigureLoadStoreFromBuffer>; - template - friend class ConfigureLoadStore; + friend struct ConfigureLoadStoreCore; ConfigureLoadStoreFromBuffer( Ptr_Type buffer, typename parent_t::parent_t &&); diff --git a/include/openPMD/LoadStoreChunk.tpp b/include/openPMD/LoadStoreChunk.tpp index d6cb992896..a15e1ec40c 100644 --- a/include/openPMD/LoadStoreChunk.tpp +++ b/include/openPMD/LoadStoreChunk.tpp @@ -4,9 +4,8 @@ namespace openPMD { -template template -auto ConfigureLoadStore::withSharedPtr(std::shared_ptr data) +auto ConfigureLoadStoreCore::withSharedPtr(std::shared_ptr data) -> shared_ptr_return_type { if (!data) @@ -18,9 +17,8 @@ auto ConfigureLoadStore::withSharedPtr(std::shared_ptr data) std::static_pointer_cast>(std::move(data)), {std::move(*this)}); } -template template -auto ConfigureLoadStore::withUniquePtr(UniquePtrWithLambda data) +auto ConfigureLoadStoreCore::withUniquePtr(UniquePtrWithLambda data) -> unique_ptr_return_type { @@ -33,10 +31,8 @@ auto ConfigureLoadStore::withUniquePtr(UniquePtrWithLambda data) std::move(data).template static_cast_>(), {std::move(*this)}); } -template template -auto ConfigureLoadStore::withRawPtr(T *data) - -> shared_ptr_return_type +auto ConfigureLoadStoreCore::withRawPtr(T *data) -> shared_ptr_return_type { if (!data) { @@ -47,16 +43,14 @@ auto ConfigureLoadStore::withRawPtr(T *data) auxiliary::shareRaw(data), {std::move(*this)}); } -template template -auto ConfigureLoadStore::withUniquePtr(std::unique_ptr data) +auto ConfigureLoadStoreCore::withUniquePtr(std::unique_ptr data) -> unique_ptr_return_type { return withUniquePtr(UniquePtrWithLambda(std::move(data))); } -template template -auto ConfigureLoadStore::withContiguousContainer( +auto ConfigureLoadStoreCore::withContiguousContainer( T_ContiguousContainer &data) -> std::enable_if_t< auxiliary::IsContiguousContainer_v, diff --git a/include/openPMD/RecordComponent.hpp b/include/openPMD/RecordComponent.hpp index 5d8ac53740..02c453f0d5 100644 --- a/include/openPMD/RecordComponent.hpp +++ b/include/openPMD/RecordComponent.hpp @@ -136,6 +136,7 @@ class RecordComponent : public BaseRecordComponent friend class MeshRecordComponent; template friend T &internal::makeOwning(T &self, Series); + friend struct ConfigureLoadStoreCore; template friend class ConfigureLoadStore; template diff --git a/include/openPMD/RecordComponent.tpp b/include/openPMD/RecordComponent.tpp index c91ccfcb7e..f992dd7dc9 100644 --- a/include/openPMD/RecordComponent.tpp +++ b/include/openPMD/RecordComponent.tpp @@ -399,9 +399,8 @@ void RecordComponent::verifyChunk(Offset const &o, Extent const &e) const } // definitions for LoadStoreChunk.hpp -template template -auto ConfigureLoadStore::enqueueStore(F &&createBuffer) +auto ConfigureLoadStoreCore::enqueueStore(F &&createBuffer) -> DynamicMemoryView { return m_rc.storeChunkSpanCreateBuffer_impl( diff --git a/src/LoadStoreChunk.cpp b/src/LoadStoreChunk.cpp index 85c4fc9aa1..df7634a78e 100644 --- a/src/LoadStoreChunk.cpp +++ b/src/LoadStoreChunk.cpp @@ -19,14 +19,6 @@ namespace openPMD { - -namespace internal -{ - ConfigureLoadStoreData::ConfigureLoadStoreData(RecordComponent &rc) - : m_rc(rc) - {} -} // namespace internal - namespace { template @@ -63,25 +55,26 @@ namespace } } // namespace +ConfigureLoadStoreCore::ConfigureLoadStoreCore(RecordComponent &rc) : m_rc(rc) +{} + template ConfigureLoadStore::ConfigureLoadStore(RecordComponent &rc) - : ConfigureLoadStoreData(rc) + : ConfigureLoadStoreCore(rc) {} template ConfigureLoadStore::ConfigureLoadStore( - internal::ConfigureLoadStoreData &&data) - : ConfigureLoadStoreData(std::move(data)) + ConfigureLoadStoreCore &&data) + : ConfigureLoadStoreCore(std::move(data)) {} -template -auto ConfigureLoadStore::dim() const -> uint8_t +auto ConfigureLoadStoreCore::dim() const -> uint8_t { return m_rc.getDimensionality(); } -template -auto ConfigureLoadStore::getOffset() -> Offset const & +auto ConfigureLoadStoreCore::getOffset() -> Offset const & { if (!m_offset.has_value()) { @@ -97,8 +90,7 @@ auto ConfigureLoadStore::getOffset() -> Offset const & return *m_offset; } -template -auto ConfigureLoadStore::getExtent() -> Extent const & +auto ConfigureLoadStoreCore::getExtent() -> Extent const & { if (!m_extent.has_value()) { @@ -118,9 +110,7 @@ auto ConfigureLoadStore::getExtent() -> Extent const & return *m_extent; } -template -auto ConfigureLoadStore::storeChunkConfig() - -> internal::LoadStoreConfig +auto ConfigureLoadStoreCore::storeChunkConfig() -> internal::LoadStoreConfig { return internal::LoadStoreConfig{getOffset(), getExtent()}; } @@ -139,17 +129,14 @@ auto ConfigureLoadStore::offset(Offset offset) -> return_type & return *static_cast(this); } -template template -auto ConfigureLoadStore::enqueueStore() -> DynamicMemoryView +auto ConfigureLoadStoreCore::enqueueStore() -> DynamicMemoryView { return m_rc.storeChunkSpan_impl(storeChunkConfig()); } -template template -auto ConfigureLoadStore::enqueueLoad() - -> std::future> +auto ConfigureLoadStoreCore::enqueueLoad() -> std::future> { auto res = m_rc.loadChunkAllocate_impl(storeChunkConfig()); return std::async( @@ -160,10 +147,8 @@ auto ConfigureLoadStore::enqueueLoad() }); } -template template -auto ConfigureLoadStore::load(EnqueuePolicy ep) - -> std::shared_ptr +auto ConfigureLoadStoreCore::load(EnqueuePolicy ep) -> std::shared_ptr { auto res = m_rc.loadChunkAllocate_impl(storeChunkConfig()); switch (ep) @@ -201,8 +186,7 @@ struct VisitorEnqueueLoadVariant } }; -template -auto ConfigureLoadStore::enqueueLoadVariant() +auto ConfigureLoadStoreCore::enqueueLoadVariant() -> std::future { return VisitorEnqueueLoadVariant::non_templated_implementation( @@ -297,21 +281,20 @@ auto ConfigureLoadStoreFromBuffer::load(EnqueuePolicy ep) -> void /* clang-format would destroy the NOLINT comments */ // clang-format off -#define INSTANTIATE_METHOD_TEMPLATES(base_class, dtype) \ - template auto base_class::enqueueStore()->DynamicMemoryView; \ - template auto base_class::enqueueLoad() \ - /* NOLINTNEXTLINE(bugprone-macro-parentheses) */ \ +#define INSTANTIATE_METHOD_TEMPLATES(dtype) \ + template auto ConfigureLoadStoreCore::enqueueStore() \ + ->DynamicMemoryView; \ + template auto ConfigureLoadStoreCore::enqueueLoad() \ + /* NOLINTNEXTLINE(bugprone-macro-parentheses) */ \ ->std::future>; \ - template auto base_class::load(EnqueuePolicy)->std::shared_ptr; + template auto ConfigureLoadStoreCore::load(EnqueuePolicy) \ + ->std::shared_ptr; // clang-format on -#define INSTANTIATE_METHOD_TEMPLATES_FOR_BASE(dtype) \ - INSTANTIATE_METHOD_TEMPLATES(ConfigureLoadStore, dtype) - template class ConfigureLoadStore; -OPENPMD_FOREACH_DATASET_DATATYPE(INSTANTIATE_METHOD_TEMPLATES_FOR_BASE) +OPENPMD_FOREACH_DATASET_DATATYPE(INSTANTIATE_METHOD_TEMPLATES) -#undef INSTANTIATE_METHOD_TEMPLATES_FOR_BASE +#undef INSTANTIATE_METHOD_TEMPLATES /* clang-format would destroy the NOLINT comments */ // clang-format off @@ -325,37 +308,19 @@ OPENPMD_FOREACH_DATASET_DATATYPE(INSTANTIATE_METHOD_TEMPLATES_FOR_BASE) template class ConfigureLoadStore< \ /* NOLINTNEXTLINE(bugprone-macro-parentheses) */ \ ConfigureLoadStoreFromBuffer>>; \ - INSTANTIATE_METHOD_TEMPLATES( \ - ConfigureLoadStore< \ - /* NOLINTNEXTLINE(bugprone-macro-parentheses) */ \ - ConfigureLoadStoreFromBuffer>>, \ - dtype) \ /* NOLINTNEXTLINE(bugprone-macro-parentheses) */ \ template class ConfigureStoreChunkFromBuffer>; \ template class ConfigureLoadStore< \ /* NOLINTNEXTLINE(bugprone-macro-parentheses) */ \ ConfigureStoreChunkFromBuffer>>; \ - INSTANTIATE_METHOD_TEMPLATES( \ - ConfigureLoadStore< \ - /* NOLINTNEXTLINE(bugprone-macro-parentheses) */ \ - ConfigureStoreChunkFromBuffer>>, \ - dtype) \ template class ConfigureStoreChunkFromBuffer< \ std::shared_ptr>; \ template class ConfigureLoadStore< \ ConfigureStoreChunkFromBuffer>>; \ - INSTANTIATE_METHOD_TEMPLATES( \ - ConfigureLoadStore< \ - ConfigureStoreChunkFromBuffer>>, \ - dtype) \ template class ConfigureStoreChunkFromBuffer< \ UniquePtrWithLambda>; \ template class ConfigureLoadStore< \ - ConfigureStoreChunkFromBuffer>>; \ - INSTANTIATE_METHOD_TEMPLATES( \ - ConfigureLoadStore< \ - ConfigureStoreChunkFromBuffer>>, \ - dtype) + ConfigureStoreChunkFromBuffer>>; // clang-format on OPENPMD_FOREACH_DATASET_DATATYPE(INSTANTIATE_STORE_CHUNK_FROM_BUFFER) From 0cb6c8d89f28e3acaaf6898d98ce905ec6338c02 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Fri, 19 Jul 2024 18:58:44 +0200 Subject: [PATCH 28/39] Some cleanup needed, but change class structure --- include/openPMD/LoadStoreChunk.hpp | 188 ++++++++++++++------------- include/openPMD/RecordComponent.hpp | 10 +- src/LoadStoreChunk.cpp | 194 +++++++++++++++------------- src/RecordComponent.cpp | 4 +- 4 files changed, 210 insertions(+), 186 deletions(-) diff --git a/include/openPMD/LoadStoreChunk.hpp b/include/openPMD/LoadStoreChunk.hpp index 6918b62534..46ae334491 100644 --- a/include/openPMD/LoadStoreChunk.hpp +++ b/include/openPMD/LoadStoreChunk.hpp @@ -12,7 +12,7 @@ namespace openPMD { class RecordComponent; -template +template class ConfigureStoreChunkFromBuffer; template class ConfigureLoadStoreFromBuffer; @@ -41,6 +41,15 @@ namespace auxiliary::detail map_variant::type; } // namespace auxiliary::detail +namespace auxiliary +{ + template + using non_void_or = std::conditional_t< + !std::is_void_v, + /*then*/ possibly_void, + /*else*/ alternative>; +} // namespace auxiliary + enum class EnqueuePolicy { Defer, @@ -84,8 +93,7 @@ struct ConfigureLoadStoreCore template struct shared_ptr_return_type_impl { - using type = - ConfigureStoreChunkFromBuffer, void>; + using type = ConfigureStoreChunkFromBuffer>; }; template @@ -98,8 +106,7 @@ struct ConfigureLoadStoreCore */ template using unique_ptr_return_type = ConfigureStoreChunkFromBuffer< - UniquePtrWithLambda>, - void>; + UniquePtrWithLambda>>; // @todo rvalue references..? template @@ -133,77 +140,17 @@ struct ConfigureLoadStoreCore -> std::future; }; -/** Basic configuration for a Load/Store operation. - * - * @tparam ChildClass CRT pattern. - * The purpose is that in child classes `return *this` should return - * an instance of the child class, not of ConfigureLoadStore. - * Instantiate with void when using without subclass. - */ -template -class ConfigureLoadStore : public ConfigureLoadStoreCore -{ - friend class RecordComponent; - friend struct ConfigureLoadStoreCore; - -protected: - ConfigureLoadStore(RecordComponent &rc); - ConfigureLoadStore(ConfigureLoadStoreCore &&); - -public: - using return_type = std::conditional_t< - std::is_void_v, - /*then*/ ConfigureLoadStore, - /*else*/ ChildClass>; - - auto offset(Offset) -> return_type &; - auto extent(Extent) -> return_type &; -}; - -/** Configuration for a Store operation with a buffer type. - * - * This class does intentionally not support Load operations since there are - * pointer types (const pointers, unique pointers) where Load operations make no - * sense. See the \ref ConfigureLoadStoreFromBuffer class template for both - * Load/Store operations. - * - * @tparam Ptr_Type The type of pointer used internally. - * @tparam ChildClass CRT pattern. - * The purpose is that in child classes `return *this` should return - * an instance of the child class, not of ConfigureStoreChunkFromBuffer. - * Instantiate with void when using without subclass. - */ -template -class ConfigureStoreChunkFromBuffer - : public ConfigureLoadStore, - /*then*/ ConfigureStoreChunkFromBuffer, - /*else*/ ChildClass>> +template +class ConfigureStoreChunkFromBufferCore : public ConfigureLoadStoreCore { public: - using return_type = std::conditional_t< - std::is_void_v, - /*then*/ ConfigureStoreChunkFromBuffer, - /*else*/ ChildClass>; - using parent_t = ConfigureLoadStore; - -protected: - friend struct ConfigureLoadStoreCore; - Ptr_Type m_buffer; std::optional m_mem_select; - auto storeChunkConfig() -> internal::LoadStoreConfigWithBuffer; - -protected: - ConfigureStoreChunkFromBuffer(Ptr_Type buffer, parent_t &&); + ConfigureStoreChunkFromBufferCore( + Ptr_Type buffer, ConfigureLoadStoreCore &&); -public: - auto memorySelection(MemorySelection) -> return_type &; - - auto as_parent() && -> parent_t &&; - auto as_parent() & -> parent_t &; - auto as_parent() const & -> parent_t const &; + auto storeChunkConfig() -> internal::LoadStoreConfigWithBuffer; auto enqueueStore() -> void; @@ -222,32 +169,95 @@ class ConfigureStoreChunkFromBuffer } }; -/** Configuration for a Load/Store operation with a buffer type. - * - * Only instantiated for pointer types where Load operations make sense (e.g. no - * const pointers and no unique pointers). - * \ref ConfigureStoreChunkFromBuffer is used otherwise. - * - * @tparam Ptr_Type The type of pointer used internally. - */ template -class ConfigureLoadStoreFromBuffer - : public ConfigureStoreChunkFromBuffer< - Ptr_Type, - ConfigureLoadStoreFromBuffer> +class ConfigureLoadStoreFromBufferCore + : public ConfigureStoreChunkFromBufferCore { - using parent_t = ConfigureStoreChunkFromBuffer< - Ptr_Type, - ConfigureLoadStoreFromBuffer>; - friend struct ConfigureLoadStoreCore; - ConfigureLoadStoreFromBuffer( - Ptr_Type buffer, typename parent_t::parent_t &&); - public: + using ConfigureStoreChunkFromBufferCore< + Ptr_Type>::ConfigureStoreChunkFromBufferCore; + auto enqueueLoad() -> void; auto load(EnqueuePolicy) -> void; }; + +namespace compose +{ + /** Basic configuration for a Load/Store operation. + * + * @tparam ChildClass CRT pattern. + * The purpose is that in child classes `return *this` should return + * an instance of the child class, not of ConfigureLoadStore. + * Instantiate with void when using without subclass. + */ + template + class ConfigureLoadStore + { + public: + auto offset(Offset) -> ChildClass &; + auto extent(Extent) -> ChildClass &; + }; + + /** Configuration for a Store operation with a buffer type. + * + * This class does intentionally not support Load operations since there are + * pointer types (const pointers, unique pointers) where Load operations + * make no sense. See the \ref ConfigureLoadStoreFromBuffer class template + * for both Load/Store operations. + * + * @tparam Ptr_Type The type of pointer used internally. + * @tparam ChildClass CRT pattern. + * The purpose is that in child classes `return *this` should return + * an instance of the child class, not of + * ConfigureStoreChunkFromBuffer. Instantiate with void when using without + * subclass. + */ + template + class ConfigureStoreChunkFromBuffer + { + public: + auto memorySelection(MemorySelection) -> ChildClass &; + }; +} // namespace compose + +class ConfigureLoadStore + : public ConfigureLoadStoreCore + , public compose::ConfigureLoadStore +{ + friend class RecordComponent; + friend struct ConfigureLoadStoreCore; + + ConfigureLoadStore(RecordComponent &rc); + ConfigureLoadStore(ConfigureLoadStoreCore &&); +}; + +template +class ConfigureStoreChunkFromBuffer + : public ConfigureStoreChunkFromBufferCore + , public compose::ConfigureLoadStore< + ConfigureStoreChunkFromBuffer> + , public compose::ConfigureStoreChunkFromBuffer< + ConfigureStoreChunkFromBuffer> +{ + friend struct ConfigureLoadStoreCore; + + using ConfigureStoreChunkFromBufferCore< + Ptr_Type>::ConfigureStoreChunkFromBufferCore; +}; + +template +class ConfigureLoadStoreFromBuffer + : public ConfigureLoadStoreFromBufferCore + , public compose::ConfigureLoadStore> + , public compose::ConfigureStoreChunkFromBuffer< + ConfigureLoadStoreFromBuffer> +{ + friend struct ConfigureLoadStoreCore; + + using ConfigureLoadStoreFromBufferCore< + Ptr_Type>::ConfigureLoadStoreFromBufferCore; +}; } // namespace openPMD #include "openPMD/LoadStoreChunk.tpp" diff --git a/include/openPMD/RecordComponent.hpp b/include/openPMD/RecordComponent.hpp index 02c453f0d5..734b0dcbe0 100644 --- a/include/openPMD/RecordComponent.hpp +++ b/include/openPMD/RecordComponent.hpp @@ -137,12 +137,10 @@ class RecordComponent : public BaseRecordComponent template friend T &internal::makeOwning(T &self, Series); friend struct ConfigureLoadStoreCore; - template - friend class ConfigureLoadStore; - template - friend class ConfigureStoreChunkFromBuffer; template - friend class ConfigureLoadStoreFromBuffer; + friend class ConfigureLoadStoreFromBufferCore; + template + friend class ConfigureStoreChunkFromBufferCore; friend struct VisitorEnqueueLoadVariant; public: @@ -295,7 +293,7 @@ class RecordComponent : public BaseRecordComponent template void loadChunkRaw(T *data, Offset offset, Extent extent); - ConfigureLoadStore prepareLoadStore(); + ConfigureLoadStore prepareLoadStore(); /** Store a chunk of data from a chunk of memory. * diff --git a/src/LoadStoreChunk.cpp b/src/LoadStoreChunk.cpp index df7634a78e..d37cb8620d 100644 --- a/src/LoadStoreChunk.cpp +++ b/src/LoadStoreChunk.cpp @@ -58,17 +58,6 @@ namespace ConfigureLoadStoreCore::ConfigureLoadStoreCore(RecordComponent &rc) : m_rc(rc) {} -template -ConfigureLoadStore::ConfigureLoadStore(RecordComponent &rc) - : ConfigureLoadStoreCore(rc) -{} - -template -ConfigureLoadStore::ConfigureLoadStore( - ConfigureLoadStoreCore &&data) - : ConfigureLoadStoreCore(std::move(data)) -{} - auto ConfigureLoadStoreCore::dim() const -> uint8_t { return m_rc.getDimensionality(); @@ -115,19 +104,33 @@ auto ConfigureLoadStoreCore::storeChunkConfig() -> internal::LoadStoreConfig return internal::LoadStoreConfig{getOffset(), getExtent()}; } -template -auto ConfigureLoadStore::extent(Extent extent) -> return_type & +namespace compose { - m_extent = std::make_optional(std::move(extent)); - return *static_cast(this); -} + template + auto ConfigureLoadStore::extent(Extent extent) -> ChildClass & + { + static_cast(this)->m_extent = + std::make_optional(std::move(extent)); + return *static_cast(this); + } -template -auto ConfigureLoadStore::offset(Offset offset) -> return_type & -{ - m_offset = std::make_optional(std::move(offset)); - return *static_cast(this); -} + template + auto ConfigureLoadStore::offset(Offset offset) -> ChildClass & + { + static_cast(this)->m_offset = + std::make_optional(std::move(offset)); + return *static_cast(this); + } + + template + auto ConfigureStoreChunkFromBuffer::memorySelection( + MemorySelection sel) -> ChildClass & + { + static_cast(this)->m_mem_select = + std::make_optional(std::move(sel)); + return *static_cast(this); + } +} // namespace compose template auto ConfigureLoadStoreCore::enqueueStore() -> DynamicMemoryView @@ -193,49 +196,22 @@ auto ConfigureLoadStoreCore::enqueueLoadVariant() m_rc, this->storeChunkConfig()); } -template -ConfigureStoreChunkFromBuffer:: - ConfigureStoreChunkFromBuffer(Ptr_Type buffer, parent_t &&parent) - : parent_t(std::move(parent)), m_buffer(std::move(buffer)) +template +ConfigureStoreChunkFromBufferCore::ConfigureStoreChunkFromBufferCore( + Ptr_Type buffer, ConfigureLoadStoreCore &&core) + : ConfigureLoadStoreCore(std::move(core)), m_buffer(std::move(buffer)) {} -template -auto ConfigureStoreChunkFromBuffer::as_parent() - && -> parent_t && -{ - return std::move(*this); -} -template -auto ConfigureStoreChunkFromBuffer::as_parent() - & -> parent_t & -{ - return *this; -} -template -auto ConfigureStoreChunkFromBuffer::as_parent() - const & -> parent_t const & -{ - return *this; -} - -template -auto ConfigureStoreChunkFromBuffer::storeChunkConfig() +template +auto ConfigureStoreChunkFromBufferCore::storeChunkConfig() -> internal::LoadStoreConfigWithBuffer { return internal::LoadStoreConfigWithBuffer{ this->getOffset(), this->getExtent(), m_mem_select}; } -template -auto ConfigureStoreChunkFromBuffer::memorySelection( - MemorySelection sel) -> return_type & -{ - this->m_mem_select = std::make_optional(std::move(sel)); - return *static_cast(this); -} - -template -auto ConfigureStoreChunkFromBuffer::enqueueStore() -> void +template +auto ConfigureStoreChunkFromBufferCore::enqueueStore() -> void { this->m_rc.storeChunk_impl( asWriteBuffer(std::move(m_buffer)), @@ -243,28 +219,28 @@ auto ConfigureStoreChunkFromBuffer::enqueueStore() -> void storeChunkConfig()); } -template -ConfigureLoadStoreFromBuffer::ConfigureLoadStoreFromBuffer( - Ptr_Type buffer, typename parent_t::parent_t &&parent) - : parent_t(std::move(buffer), std::move(parent)) -{ - static_assert( - std::is_same_v< - Ptr_Type, - std::shared_ptr>, - "ConfigureLoadStoreFromBuffer must be instantiated with a shared_ptr " - "type."); -} +// template +// ConfigureLoadStoreFromBuffer::ConfigureLoadStoreFromBuffer( +// Ptr_Type buffer, typename parent_t::parent_t &&parent) +// : parent_t(std::move(buffer), std::move(parent)) +// { +// static_assert( +// std::is_same_v< +// Ptr_Type, +// std::shared_ptr>, +// "ConfigureLoadStoreFromBuffer must be instantiated with a shared_ptr +// " "type."); +// } template -auto ConfigureLoadStoreFromBuffer::enqueueLoad() -> void +auto ConfigureLoadStoreFromBufferCore::enqueueLoad() -> void { this->m_rc.loadChunk_impl( std::move(this->m_buffer), this->storeChunkConfig()); } template -auto ConfigureLoadStoreFromBuffer::load(EnqueuePolicy ep) -> void +auto ConfigureLoadStoreFromBufferCore::load(EnqueuePolicy ep) -> void { this->m_rc.loadChunk_impl( std::move(this->m_buffer), this->storeChunkConfig()); @@ -291,41 +267,81 @@ auto ConfigureLoadStoreFromBuffer::load(EnqueuePolicy ep) -> void ->std::shared_ptr; // clang-format on -template class ConfigureLoadStore; OPENPMD_FOREACH_DATASET_DATATYPE(INSTANTIATE_METHOD_TEMPLATES) #undef INSTANTIATE_METHOD_TEMPLATES /* clang-format would destroy the NOLINT comments */ -// clang-format off +//// clang-format off #define INSTANTIATE_STORE_CHUNK_FROM_BUFFER(dtype) \ - /* NOLINTNEXTLINE(bugprone-macro-parentheses) */ \ template class ConfigureLoadStoreFromBuffer>; \ - template class ConfigureStoreChunkFromBuffer< \ - std::shared_ptr, \ - /* NOLINTNEXTLINE(bugprone-macro-parentheses) */ \ - ConfigureLoadStoreFromBuffer>>; \ - template class ConfigureLoadStore< \ - /* NOLINTNEXTLINE(bugprone-macro-parentheses) */ \ - ConfigureLoadStoreFromBuffer>>; \ - /* NOLINTNEXTLINE(bugprone-macro-parentheses) */ \ + namespace compose \ + { \ + template class ConfigureLoadStore< \ + ConfigureLoadStoreFromBuffer>>; \ + template class ConfigureStoreChunkFromBuffer< \ + ConfigureLoadStoreFromBuffer>>; \ + } \ + template class ConfigureLoadStoreFromBufferCore>; \ + template class ConfigureStoreChunkFromBuffer>; \ + namespace compose \ + { \ + template class ConfigureLoadStore< \ + openPMD::ConfigureStoreChunkFromBuffer>>; \ + template class ConfigureStoreChunkFromBuffer< \ + openPMD::ConfigureStoreChunkFromBuffer>>; \ + } \ + template class ConfigureStoreChunkFromBufferCore>; \ template class ConfigureStoreChunkFromBuffer>; \ - template class ConfigureLoadStore< \ - /* NOLINTNEXTLINE(bugprone-macro-parentheses) */ \ - ConfigureStoreChunkFromBuffer>>; \ + namespace compose \ + { \ + template class ConfigureLoadStore< \ + openPMD::ConfigureStoreChunkFromBuffer< \ + UniquePtrWithLambda>>; \ + template class ConfigureStoreChunkFromBuffer< \ + openPMD::ConfigureStoreChunkFromBuffer< \ + UniquePtrWithLambda>>; \ + } \ + template class ConfigureStoreChunkFromBufferCore< \ + UniquePtrWithLambda>; \ template class ConfigureStoreChunkFromBuffer< \ std::shared_ptr>; \ - template class ConfigureLoadStore< \ - ConfigureStoreChunkFromBuffer>>; \ + namespace compose \ + { \ + template class ConfigureLoadStore< \ + openPMD::ConfigureStoreChunkFromBuffer< \ + std::shared_ptr>>; \ + template class ConfigureStoreChunkFromBuffer< \ + openPMD::ConfigureStoreChunkFromBuffer< \ + std::shared_ptr>>; \ + } \ + template class ConfigureStoreChunkFromBufferCore< \ + std::shared_ptr>; \ template class ConfigureStoreChunkFromBuffer< \ UniquePtrWithLambda>; \ - template class ConfigureLoadStore< \ - ConfigureStoreChunkFromBuffer>>; + namespace compose \ + { \ + template class ConfigureLoadStore< \ + openPMD::ConfigureStoreChunkFromBuffer< \ + UniquePtrWithLambda>>; \ + template class ConfigureStoreChunkFromBuffer< \ + openPMD::ConfigureStoreChunkFromBuffer< \ + UniquePtrWithLambda>>; \ + } \ + template class ConfigureStoreChunkFromBufferCore< \ + UniquePtrWithLambda>; // clang-format on +// /* NOLINTNEXTLINE(bugprone-macro-parentheses) */ OPENPMD_FOREACH_DATASET_DATATYPE(INSTANTIATE_STORE_CHUNK_FROM_BUFFER) #undef INSTANTIATE_STORE_CHUNK_FROM_BUFFER #undef INSTANTIATE_METHOD_TEMPLATES +ConfigureLoadStore::ConfigureLoadStore(RecordComponent &rc) + : ConfigureLoadStoreCore{rc} +{} +ConfigureLoadStore::ConfigureLoadStore(ConfigureLoadStoreCore &&core) + : ConfigureLoadStoreCore{std::move(core)} +{} } // namespace openPMD diff --git a/src/RecordComponent.cpp b/src/RecordComponent.cpp index 2bbc901041..9905801a7a 100644 --- a/src/RecordComponent.cpp +++ b/src/RecordComponent.cpp @@ -66,9 +66,9 @@ RecordComponent::RecordComponent() : BaseRecordComponent(NoInit()) setData(std::make_shared()); } -ConfigureLoadStore RecordComponent::prepareLoadStore() +ConfigureLoadStore RecordComponent::prepareLoadStore() { - return ConfigureLoadStore{*this}; + return ConfigureLoadStore{*this}; } namespace From 999e5be206e8421b381a711218e5685598a5cdaf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Mon, 22 Jul 2024 11:22:30 +0200 Subject: [PATCH 29/39] WIP: Cleanup --- include/openPMD/LoadStoreChunk.hpp | 284 +++++++++--------- include/openPMD/LoadStoreChunk.tpp | 15 +- include/openPMD/RecordComponent.hpp | 23 +- include/openPMD/RecordComponent.tpp | 2 +- src/LoadStoreChunk.cpp | 434 ++++++++++++++-------------- 5 files changed, 390 insertions(+), 368 deletions(-) diff --git a/include/openPMD/LoadStoreChunk.hpp b/include/openPMD/LoadStoreChunk.hpp index 46ae334491..01538c1a5e 100644 --- a/include/openPMD/LoadStoreChunk.hpp +++ b/include/openPMD/LoadStoreChunk.hpp @@ -37,18 +37,17 @@ namespace internal namespace auxiliary::detail { - using future_to_shared_ptr_dataset_types = + using shared_ptr_dataset_types = map_variant::type; } // namespace auxiliary::detail -namespace auxiliary +namespace compose { - template - using non_void_or = std::conditional_t< - !std::is_void_v, - /*then*/ possibly_void, - /*else*/ alternative>; -} // namespace auxiliary + template + class ConfigureLoadStore; + template + class ConfigureLoadStoreFromBuffer; +} // namespace compose enum class EnqueuePolicy { @@ -56,131 +55,154 @@ enum class EnqueuePolicy Immediate }; -/* - * Actual data members of `ConfigureLoadStore<>` and methods that don't depend - * on the ChildClass template parameter. By extracting the members to this - * struct, we can pass them around between different instances of the class - * template. Numbers of method instantiations can be reduced. - */ -struct ConfigureLoadStoreCore +namespace core { - ConfigureLoadStoreCore(RecordComponent &); - - RecordComponent &m_rc; - std::optional m_offset; - std::optional m_extent; - -protected: - [[nodiscard]] auto dim() const -> uint8_t; - auto getOffset() -> Offset const &; - auto getExtent() -> Extent const &; - auto storeChunkConfig() -> internal::LoadStoreConfig; - -public: - /* - * If the type is non-const, then the return type should be - * ConfigureLoadStoreFromBuffer<>, ... - */ - template - struct shared_ptr_return_type_impl - { - using type = ConfigureLoadStoreFromBuffer>; - }; /* - * ..., but if it is a const type, Load operations make no sense, so the - * return type should be ConfigureStoreChunkFromBuffer<>. + * Actual data members of `ConfigureLoadStore<>` and methods that don't + * depend on the ChildClass template parameter. By extracting the members to + * this struct, we can pass them around between different instances of the + * class template. Numbers of method instantiations can be reduced. */ - template - struct shared_ptr_return_type_impl + class ConfigureLoadStore { - using type = ConfigureStoreChunkFromBuffer>; - }; - - template - using shared_ptr_return_type = - typename shared_ptr_return_type_impl>::type; - - /* - * As loading into unique pointer types makes no sense, the case is simpler - * for unique pointers. Just remove the array extents here. - */ - template - using unique_ptr_return_type = ConfigureStoreChunkFromBuffer< - UniquePtrWithLambda>>; - - // @todo rvalue references..? - template - auto withSharedPtr(std::shared_ptr) -> shared_ptr_return_type; - template - auto withUniquePtr(UniquePtrWithLambda) -> unique_ptr_return_type; - template - auto withUniquePtr(std::unique_ptr) -> unique_ptr_return_type; - template - auto withRawPtr(T *data) -> shared_ptr_return_type; - template - auto withContiguousContainer(T_ContiguousContainer &data) - -> std::enable_if_t< - auxiliary::IsContiguousContainer_v, - shared_ptr_return_type>; - - template - [[nodiscard]] auto enqueueStore() -> DynamicMemoryView; - // definition for this one is in RecordComponent.tpp since it needs the - // definition of class RecordComponent. - template - [[nodiscard]] auto enqueueStore(F &&createBuffer) -> DynamicMemoryView; - - template - [[nodiscard]] auto enqueueLoad() -> std::future>; - - template - [[nodiscard]] auto load(EnqueuePolicy) -> std::shared_ptr; - - [[nodiscard]] auto enqueueLoadVariant() - -> std::future; -}; + template + friend class compose::ConfigureLoadStore; + template + friend class compose::ConfigureLoadStoreFromBuffer; -template -class ConfigureStoreChunkFromBufferCore : public ConfigureLoadStoreCore -{ -public: - Ptr_Type m_buffer; - std::optional m_mem_select; + protected: + ConfigureLoadStore(RecordComponent &); + RecordComponent &m_rc; - ConfigureStoreChunkFromBufferCore( - Ptr_Type buffer, ConfigureLoadStoreCore &&); + std::optional m_offset; + std::optional m_extent; - auto storeChunkConfig() -> internal::LoadStoreConfigWithBuffer; + [[nodiscard]] auto dim() const -> uint8_t; + auto storeChunkConfig() -> internal::LoadStoreConfig; - auto enqueueStore() -> void; + public: + auto getOffset() -> Offset const &; + auto getExtent() -> Extent const &; + /* + * If the type is non-const, then the return type should be + * ConfigureLoadStoreFromBuffer<>, ... + */ + template + struct shared_ptr_return_type_impl + { + using type = ConfigureLoadStoreFromBuffer>; + }; + /* + * ..., but if it is a const type, Load operations make no sense, so the + * return type should be ConfigureStoreChunkFromBuffer<>. + */ + template + struct shared_ptr_return_type_impl + { + using type = + ConfigureStoreChunkFromBuffer>; + }; + + template + using shared_ptr_return_type = + typename shared_ptr_return_type_impl>::type; + + /* + * As loading into unique pointer types makes no sense, the case is + * simpler for unique pointers. Just remove the array extents here. + */ + template + using unique_ptr_return_type = ConfigureStoreChunkFromBuffer< + UniquePtrWithLambda>>; + + // @todo rvalue references..? + template + auto withSharedPtr(std::shared_ptr) -> shared_ptr_return_type; + template + auto withUniquePtr(UniquePtrWithLambda) -> unique_ptr_return_type; + template + auto + withUniquePtr(std::unique_ptr) -> unique_ptr_return_type; + template + auto withRawPtr(T *data) -> shared_ptr_return_type; + template + auto withContiguousContainer(T_ContiguousContainer &data) + -> std::enable_if_t< + auxiliary::IsContiguousContainer_v, + shared_ptr_return_type< + typename T_ContiguousContainer::value_type>>; + + template + [[nodiscard]] auto enqueueStore() -> DynamicMemoryView; + // definition for this one is in RecordComponent.tpp since it needs the + // definition of class RecordComponent. + template + [[nodiscard]] auto + enqueueStore(F &&createBuffer) -> DynamicMemoryView; + + template + [[nodiscard]] auto enqueueLoad() -> std::future>; + + template + [[nodiscard]] auto load(EnqueuePolicy) -> std::shared_ptr; + + [[nodiscard]] auto enqueueLoadVariant() + -> std::future; + + [[nodiscard]] auto loadVariant(EnqueuePolicy) + -> auxiliary::detail::shared_ptr_dataset_types; + }; - /** This intentionally shadows the parent class's enqueueLoad method in - * order to show a compile error when using enqueueLoad() on an object of - * this class. The parent method can still be accessed through as_parent() - * if needed. - */ - template - auto enqueueLoad() + template + class ConfigureStoreChunkFromBuffer : public ConfigureLoadStore { - static_assert( - auxiliary::dependent_false_v, - "Cannot load chunk data into a buffer that is const or a " - "unique_ptr."); - } -}; + public: + Ptr_Type m_buffer; + std::optional m_mem_select; + + ConfigureStoreChunkFromBuffer(Ptr_Type buffer, ConfigureLoadStore &&); + + auto storeChunkConfig() -> internal::LoadStoreConfigWithBuffer; + + auto enqueueStore() -> void; + + /** This intentionally shadows the parent class's enqueueLoad methods in + * order to show a compile error when using enqueueLoad() on an object + * of this class. The parent method can still be accessed through + * as_parent() if needed. + */ + template + auto enqueueLoad() + { + static_assert( + auxiliary::dependent_false_v, + "Cannot load chunk data into a buffer that is const or a " + "unique_ptr."); + } + + template + auto load(EnqueuePolicy) + { + static_assert( + auxiliary::dependent_false_v, + "Cannot load chunk data into a buffer that is const or a " + "unique_ptr."); + } + }; -template -class ConfigureLoadStoreFromBufferCore - : public ConfigureStoreChunkFromBufferCore -{ -public: - using ConfigureStoreChunkFromBufferCore< - Ptr_Type>::ConfigureStoreChunkFromBufferCore; + template + class ConfigureLoadStoreFromBuffer + : public ConfigureStoreChunkFromBuffer + { + public: + using ConfigureStoreChunkFromBuffer< + Ptr_Type>::ConfigureStoreChunkFromBuffer; - auto enqueueLoad() -> void; + auto enqueueLoad() -> void; - auto load(EnqueuePolicy) -> void; -}; + auto load(EnqueuePolicy) -> void; + }; +} // namespace core namespace compose { @@ -222,41 +244,41 @@ namespace compose } // namespace compose class ConfigureLoadStore - : public ConfigureLoadStoreCore + : public core::ConfigureLoadStore , public compose::ConfigureLoadStore { friend class RecordComponent; - friend struct ConfigureLoadStoreCore; + friend class core::ConfigureLoadStore; ConfigureLoadStore(RecordComponent &rc); - ConfigureLoadStore(ConfigureLoadStoreCore &&); + ConfigureLoadStore(core::ConfigureLoadStore &&); }; template class ConfigureStoreChunkFromBuffer - : public ConfigureStoreChunkFromBufferCore + : public core::ConfigureStoreChunkFromBuffer , public compose::ConfigureLoadStore< ConfigureStoreChunkFromBuffer> , public compose::ConfigureStoreChunkFromBuffer< ConfigureStoreChunkFromBuffer> { - friend struct ConfigureLoadStoreCore; + friend class core::ConfigureLoadStore; - using ConfigureStoreChunkFromBufferCore< - Ptr_Type>::ConfigureStoreChunkFromBufferCore; + using core::ConfigureStoreChunkFromBuffer< + Ptr_Type>::ConfigureStoreChunkFromBuffer; }; template class ConfigureLoadStoreFromBuffer - : public ConfigureLoadStoreFromBufferCore + : public core::ConfigureLoadStoreFromBuffer , public compose::ConfigureLoadStore> , public compose::ConfigureStoreChunkFromBuffer< ConfigureLoadStoreFromBuffer> { - friend struct ConfigureLoadStoreCore; + friend class ConfigureLoadStoreCore; - using ConfigureLoadStoreFromBufferCore< - Ptr_Type>::ConfigureLoadStoreFromBufferCore; + using core::ConfigureLoadStoreFromBuffer< + Ptr_Type>::ConfigureLoadStoreFromBuffer; }; } // namespace openPMD diff --git a/include/openPMD/LoadStoreChunk.tpp b/include/openPMD/LoadStoreChunk.tpp index a15e1ec40c..c3c104cebd 100644 --- a/include/openPMD/LoadStoreChunk.tpp +++ b/include/openPMD/LoadStoreChunk.tpp @@ -2,10 +2,10 @@ #include "openPMD/LoadStoreChunk.hpp" -namespace openPMD +namespace openPMD::core { template -auto ConfigureLoadStoreCore::withSharedPtr(std::shared_ptr data) +auto ConfigureLoadStore::withSharedPtr(std::shared_ptr data) -> shared_ptr_return_type { if (!data) @@ -18,7 +18,7 @@ auto ConfigureLoadStoreCore::withSharedPtr(std::shared_ptr data) {std::move(*this)}); } template -auto ConfigureLoadStoreCore::withUniquePtr(UniquePtrWithLambda data) +auto ConfigureLoadStore::withUniquePtr(UniquePtrWithLambda data) -> unique_ptr_return_type { @@ -32,7 +32,7 @@ auto ConfigureLoadStoreCore::withUniquePtr(UniquePtrWithLambda data) {std::move(*this)}); } template -auto ConfigureLoadStoreCore::withRawPtr(T *data) -> shared_ptr_return_type +auto ConfigureLoadStore::withRawPtr(T *data) -> shared_ptr_return_type { if (!data) { @@ -44,14 +44,13 @@ auto ConfigureLoadStoreCore::withRawPtr(T *data) -> shared_ptr_return_type } template -auto ConfigureLoadStoreCore::withUniquePtr(std::unique_ptr data) +auto ConfigureLoadStore::withUniquePtr(std::unique_ptr data) -> unique_ptr_return_type { return withUniquePtr(UniquePtrWithLambda(std::move(data))); } template -auto ConfigureLoadStoreCore::withContiguousContainer( - T_ContiguousContainer &data) +auto ConfigureLoadStore::withContiguousContainer(T_ContiguousContainer &data) -> std::enable_if_t< auxiliary::IsContiguousContainer_v, shared_ptr_return_type> @@ -62,4 +61,4 @@ auto ConfigureLoadStoreCore::withContiguousContainer( } return withRawPtr(data.data()); } -} // namespace openPMD +} // namespace openPMD::core diff --git a/include/openPMD/RecordComponent.hpp b/include/openPMD/RecordComponent.hpp index 734b0dcbe0..b94b981b91 100644 --- a/include/openPMD/RecordComponent.hpp +++ b/include/openPMD/RecordComponent.hpp @@ -115,6 +115,17 @@ namespace internal class BaseRecordData; } // namespace internal +namespace core +{ + class ConfigureLoadStore; + template + class ConfigureLoadStoreFromBuffer; + template + class ConfigureStoreChunkFromBuffer; + struct VisitorEnqueueLoadVariant; + struct VisitorLoadVariant; +} // namespace core + template class BaseRecord; @@ -137,11 +148,13 @@ class RecordComponent : public BaseRecordComponent template friend T &internal::makeOwning(T &self, Series); friend struct ConfigureLoadStoreCore; + friend class core::ConfigureLoadStore; template - friend class ConfigureLoadStoreFromBufferCore; + friend class core::ConfigureLoadStoreFromBuffer; template - friend class ConfigureStoreChunkFromBufferCore; - friend struct VisitorEnqueueLoadVariant; + friend class core::ConfigureStoreChunkFromBuffer; + friend struct core::VisitorEnqueueLoadVariant; + friend struct core::VisitorLoadVariant; public: enum class Allocation @@ -236,8 +249,8 @@ class RecordComponent : public BaseRecordComponent template std::shared_ptr loadChunk(Offset = {0u}, Extent = {-1u}); - using shared_ptr_dataset_types = auxiliary::detail:: - map_variant::type; + using shared_ptr_dataset_types = + auxiliary::detail::shared_ptr_dataset_types; /** std::variant-based version of allocating loadChunk(Offset, Extent) * diff --git a/include/openPMD/RecordComponent.tpp b/include/openPMD/RecordComponent.tpp index f992dd7dc9..04e453e0cc 100644 --- a/include/openPMD/RecordComponent.tpp +++ b/include/openPMD/RecordComponent.tpp @@ -400,7 +400,7 @@ void RecordComponent::verifyChunk(Offset const &o, Extent const &e) const // definitions for LoadStoreChunk.hpp template -auto ConfigureLoadStoreCore::enqueueStore(F &&createBuffer) +auto core::ConfigureLoadStore::enqueueStore(F &&createBuffer) -> DynamicMemoryView { return m_rc.storeChunkSpanCreateBuffer_impl( diff --git a/src/LoadStoreChunk.cpp b/src/LoadStoreChunk.cpp index d37cb8620d..cd9cb347f9 100644 --- a/src/LoadStoreChunk.cpp +++ b/src/LoadStoreChunk.cpp @@ -55,54 +55,192 @@ namespace } } // namespace -ConfigureLoadStoreCore::ConfigureLoadStoreCore(RecordComponent &rc) : m_rc(rc) -{} - -auto ConfigureLoadStoreCore::dim() const -> uint8_t +namespace core { - return m_rc.getDimensionality(); -} + ConfigureLoadStore::ConfigureLoadStore(RecordComponent &rc) : m_rc(rc) + {} -auto ConfigureLoadStoreCore::getOffset() -> Offset const & -{ - if (!m_offset.has_value()) + auto ConfigureLoadStore::dim() const -> uint8_t { - if (m_rc.joinedDimension().has_value()) - { - m_offset = std::make_optional(); - } - else + return m_rc.getDimensionality(); + } + + auto ConfigureLoadStore::storeChunkConfig() -> internal::LoadStoreConfig + { + return internal::LoadStoreConfig{getOffset(), getExtent()}; + } + + auto ConfigureLoadStore::getOffset() -> Offset const & + { + if (!m_offset.has_value()) { - m_offset = std::make_optional(dim(), 0); + if (m_rc.joinedDimension().has_value()) + { + m_offset = std::make_optional(); + } + else + { + m_offset = std::make_optional(dim(), 0); + } } + return *m_offset; } - return *m_offset; -} -auto ConfigureLoadStoreCore::getExtent() -> Extent const & -{ - if (!m_extent.has_value()) + auto ConfigureLoadStore::getExtent() -> Extent const & { - m_extent = std::make_optional(m_rc.getExtent()); - if (m_offset.has_value()) + if (!m_extent.has_value()) { - auto it_o = m_offset->begin(); - auto end_o = m_offset->end(); - auto it_e = m_extent->begin(); - auto end_e = m_extent->end(); - for (; it_o != end_o && it_e != end_e; ++it_e, ++it_o) + m_extent = std::make_optional(m_rc.getExtent()); + if (m_offset.has_value()) { - *it_e -= *it_o; + auto it_o = m_offset->begin(); + auto end_o = m_offset->end(); + auto it_e = m_extent->begin(); + auto end_e = m_extent->end(); + for (; it_o != end_o && it_e != end_e; ++it_e, ++it_o) + { + *it_e -= *it_o; + } } } + return *m_extent; } - return *m_extent; -} -auto ConfigureLoadStoreCore::storeChunkConfig() -> internal::LoadStoreConfig -{ - return internal::LoadStoreConfig{getOffset(), getExtent()}; -} + template + auto ConfigureLoadStore::enqueueStore() -> DynamicMemoryView + { + return m_rc.storeChunkSpan_impl(storeChunkConfig()); + } + + template + auto ConfigureLoadStore::enqueueLoad() -> std::future> + { + auto res = m_rc.loadChunkAllocate_impl(storeChunkConfig()); + return std::async( + std::launch::deferred, + [res_lambda = std::move(res), rc = m_rc]() mutable { + rc.seriesFlush(); + return res_lambda; + }); + } + + template + auto ConfigureLoadStore::load(EnqueuePolicy ep) -> std::shared_ptr + { + auto res = m_rc.loadChunkAllocate_impl(storeChunkConfig()); + switch (ep) + { + case EnqueuePolicy::Defer: + break; + case EnqueuePolicy::Immediate: + m_rc.seriesFlush(); + break; + } + return res; + } + + struct VisitorEnqueueLoadVariant + { + template + static auto call(RecordComponent &rc, internal::LoadStoreConfig cfg) + -> std::future + { + auto res = rc.loadChunkAllocate_impl(std::move(cfg)); + return std::async( + std::launch::deferred, + [res_lambda = std::move(res), rc_lambda = rc]() mutable + -> auxiliary::detail::shared_ptr_dataset_types { + rc_lambda.seriesFlush(); + return res_lambda; + }); + } + }; + + auto ConfigureLoadStore::enqueueLoadVariant() + -> std::future + { + return m_rc.visit(this->storeChunkConfig()); + } + + struct VisitorLoadVariant + { + template + static auto call(RecordComponent &rc, internal::LoadStoreConfig cfg) + -> auxiliary::detail::shared_ptr_dataset_types + { + return rc.loadChunkAllocate_impl(std::move(cfg)); + } + }; + + auto ConfigureLoadStore::loadVariant(EnqueuePolicy ep) + -> auxiliary::detail::shared_ptr_dataset_types + { + auto res = m_rc.visit(this->storeChunkConfig()); + switch (ep) + { + case EnqueuePolicy::Defer: + break; + case EnqueuePolicy::Immediate: + m_rc.seriesFlush(); + break; + } + return res; + } + + template + ConfigureStoreChunkFromBuffer::ConfigureStoreChunkFromBuffer( + Ptr_Type buffer, ConfigureLoadStore &&core) + : ConfigureLoadStore(std::move(core)), m_buffer(std::move(buffer)) + {} + + template + auto ConfigureStoreChunkFromBuffer::storeChunkConfig() + -> internal::LoadStoreConfigWithBuffer + { + return internal::LoadStoreConfigWithBuffer{ + this->getOffset(), this->getExtent(), m_mem_select}; + } + + template + auto ConfigureStoreChunkFromBuffer::enqueueStore() -> void + { + this->m_rc.storeChunk_impl( + asWriteBuffer(std::move(m_buffer)), + determineDatatype>(), + storeChunkConfig()); + } + + template + auto ConfigureLoadStoreFromBuffer::enqueueLoad() -> void + { + static_assert( + std::is_same_v< + Ptr_Type, + std::shared_ptr< + std::remove_cv_t>>, + "ConfigureLoadStoreFromBuffer must be instantiated with a " + "non-const " + "shared_ptr type."); + this->m_rc.loadChunk_impl( + std::move(this->m_buffer), this->storeChunkConfig()); + } + + template + auto ConfigureLoadStoreFromBuffer::load(EnqueuePolicy ep) -> void + { + this->m_rc.loadChunk_impl( + std::move(this->m_buffer), this->storeChunkConfig()); + switch (ep) + { + + case EnqueuePolicy::Defer: + break; + case EnqueuePolicy::Immediate: + this->m_rc.seriesFlush(); + break; + } + } +} // namespace core namespace compose { @@ -132,138 +270,16 @@ namespace compose } } // namespace compose -template -auto ConfigureLoadStoreCore::enqueueStore() -> DynamicMemoryView -{ - return m_rc.storeChunkSpan_impl(storeChunkConfig()); -} - -template -auto ConfigureLoadStoreCore::enqueueLoad() -> std::future> -{ - auto res = m_rc.loadChunkAllocate_impl(storeChunkConfig()); - return std::async( - std::launch::deferred, - [res_lambda = std::move(res), rc = m_rc]() mutable { - rc.seriesFlush(); - return res_lambda; - }); -} - -template -auto ConfigureLoadStoreCore::load(EnqueuePolicy ep) -> std::shared_ptr -{ - auto res = m_rc.loadChunkAllocate_impl(storeChunkConfig()); - switch (ep) - { - case EnqueuePolicy::Defer: - break; - case EnqueuePolicy::Immediate: - m_rc.seriesFlush(); - break; - } - return res; -} - -struct VisitorEnqueueLoadVariant -{ - template - static auto call(RecordComponent &rc, internal::LoadStoreConfig cfg) - -> std::future - { - auto res = rc.loadChunkAllocate_impl(std::move(cfg)); - return std::async( - std::launch::deferred, - [res_lambda = std::move(res), rc_lambda = rc]() mutable - -> auxiliary::detail::future_to_shared_ptr_dataset_types { - rc_lambda.seriesFlush(); - return res_lambda; - }); - } - - static auto non_templated_implementation( - RecordComponent &rc, internal::LoadStoreConfig cfg) - -> std::future - { - return rc.visit(std::move(cfg)); - } -}; - -auto ConfigureLoadStoreCore::enqueueLoadVariant() - -> std::future -{ - return VisitorEnqueueLoadVariant::non_templated_implementation( - m_rc, this->storeChunkConfig()); -} - -template -ConfigureStoreChunkFromBufferCore::ConfigureStoreChunkFromBufferCore( - Ptr_Type buffer, ConfigureLoadStoreCore &&core) - : ConfigureLoadStoreCore(std::move(core)), m_buffer(std::move(buffer)) -{} - -template -auto ConfigureStoreChunkFromBufferCore::storeChunkConfig() - -> internal::LoadStoreConfigWithBuffer -{ - return internal::LoadStoreConfigWithBuffer{ - this->getOffset(), this->getExtent(), m_mem_select}; -} - -template -auto ConfigureStoreChunkFromBufferCore::enqueueStore() -> void -{ - this->m_rc.storeChunk_impl( - asWriteBuffer(std::move(m_buffer)), - determineDatatype>(), - storeChunkConfig()); -} - -// template -// ConfigureLoadStoreFromBuffer::ConfigureLoadStoreFromBuffer( -// Ptr_Type buffer, typename parent_t::parent_t &&parent) -// : parent_t(std::move(buffer), std::move(parent)) -// { -// static_assert( -// std::is_same_v< -// Ptr_Type, -// std::shared_ptr>, -// "ConfigureLoadStoreFromBuffer must be instantiated with a shared_ptr -// " "type."); -// } - -template -auto ConfigureLoadStoreFromBufferCore::enqueueLoad() -> void -{ - this->m_rc.loadChunk_impl( - std::move(this->m_buffer), this->storeChunkConfig()); -} - -template -auto ConfigureLoadStoreFromBufferCore::load(EnqueuePolicy ep) -> void -{ - this->m_rc.loadChunk_impl( - std::move(this->m_buffer), this->storeChunkConfig()); - switch (ep) - { - - case EnqueuePolicy::Defer: - break; - case EnqueuePolicy::Immediate: - this->m_rc.seriesFlush(); - break; - } -} +template class compose::ConfigureLoadStore; /* clang-format would destroy the NOLINT comments */ -// clang-format off +//// clang-format off #define INSTANTIATE_METHOD_TEMPLATES(dtype) \ - template auto ConfigureLoadStoreCore::enqueueStore() \ - ->DynamicMemoryView; \ - template auto ConfigureLoadStoreCore::enqueueLoad() \ - /* NOLINTNEXTLINE(bugprone-macro-parentheses) */ \ - ->std::future>; \ - template auto ConfigureLoadStoreCore::load(EnqueuePolicy) \ + template auto core::ConfigureLoadStore::enqueueStore() \ + -> DynamicMemoryView; \ + template auto core::ConfigureLoadStore::enqueueLoad() \ + -> std::future>; \ + template auto core::ConfigureLoadStore::load(EnqueuePolicy) \ ->std::shared_ptr; // clang-format on @@ -273,75 +289,47 @@ OPENPMD_FOREACH_DATASET_DATATYPE(INSTANTIATE_METHOD_TEMPLATES) /* clang-format would destroy the NOLINT comments */ //// clang-format off -#define INSTANTIATE_STORE_CHUNK_FROM_BUFFER(dtype) \ - template class ConfigureLoadStoreFromBuffer>; \ - namespace compose \ - { \ - template class ConfigureLoadStore< \ - ConfigureLoadStoreFromBuffer>>; \ - template class ConfigureStoreChunkFromBuffer< \ - ConfigureLoadStoreFromBuffer>>; \ - } \ - template class ConfigureLoadStoreFromBufferCore>; \ - template class ConfigureStoreChunkFromBuffer>; \ - namespace compose \ - { \ - template class ConfigureLoadStore< \ - openPMD::ConfigureStoreChunkFromBuffer>>; \ - template class ConfigureStoreChunkFromBuffer< \ - openPMD::ConfigureStoreChunkFromBuffer>>; \ - } \ - template class ConfigureStoreChunkFromBufferCore>; \ - template class ConfigureStoreChunkFromBuffer>; \ - namespace compose \ - { \ - template class ConfigureLoadStore< \ - openPMD::ConfigureStoreChunkFromBuffer< \ - UniquePtrWithLambda>>; \ - template class ConfigureStoreChunkFromBuffer< \ - openPMD::ConfigureStoreChunkFromBuffer< \ - UniquePtrWithLambda>>; \ - } \ - template class ConfigureStoreChunkFromBufferCore< \ - UniquePtrWithLambda>; \ - template class ConfigureStoreChunkFromBuffer< \ - std::shared_ptr>; \ - namespace compose \ - { \ - template class ConfigureLoadStore< \ - openPMD::ConfigureStoreChunkFromBuffer< \ - std::shared_ptr>>; \ - template class ConfigureStoreChunkFromBuffer< \ - openPMD::ConfigureStoreChunkFromBuffer< \ - std::shared_ptr>>; \ - } \ - template class ConfigureStoreChunkFromBufferCore< \ - std::shared_ptr>; \ - template class ConfigureStoreChunkFromBuffer< \ - UniquePtrWithLambda>; \ - namespace compose \ - { \ - template class ConfigureLoadStore< \ - openPMD::ConfigureStoreChunkFromBuffer< \ - UniquePtrWithLambda>>; \ - template class ConfigureStoreChunkFromBuffer< \ - openPMD::ConfigureStoreChunkFromBuffer< \ - UniquePtrWithLambda>>; \ - } \ - template class ConfigureStoreChunkFromBufferCore< \ - UniquePtrWithLambda>; +#define INSTANTIATE_HALF(pointer_type) \ + template class ConfigureStoreChunkFromBuffer; \ + template class core::ConfigureStoreChunkFromBuffer; \ + template class compose::ConfigureLoadStore< \ + ConfigureStoreChunkFromBuffer>; \ + template class compose::ConfigureStoreChunkFromBuffer< \ + ConfigureStoreChunkFromBuffer>; // clang-format on + +/* clang-format would destroy the NOLINT comments */ +// clang-format off +#define INSTANTIATE_FULL(pointer_type) \ + INSTANTIATE_HALF(pointer_type) \ + template class ConfigureLoadStoreFromBuffer; \ + template class core::ConfigureLoadStoreFromBuffer; \ + template class compose::ConfigureLoadStore< \ + /* NOLINTNEXTLINE(bugprone-macro-parentheses) */ \ + ConfigureLoadStoreFromBuffer>; \ + template class compose::ConfigureStoreChunkFromBuffer< \ + /* NOLINTNEXTLINE(bugprone-macro-parentheses) */ \ + ConfigureLoadStoreFromBuffer>; +// clang-format on + +#define INSTANTIATE_STORE_CHUNK_FROM_BUFFER(dtype) \ + INSTANTIATE_FULL(std::shared_ptr) \ + INSTANTIATE_HALF(std::shared_ptr) \ + INSTANTIATE_HALF(UniquePtrWithLambda) \ + INSTANTIATE_HALF(UniquePtrWithLambda) // /* NOLINTNEXTLINE(bugprone-macro-parentheses) */ OPENPMD_FOREACH_DATASET_DATATYPE(INSTANTIATE_STORE_CHUNK_FROM_BUFFER) #undef INSTANTIATE_STORE_CHUNK_FROM_BUFFER #undef INSTANTIATE_METHOD_TEMPLATES +#undef INSTANTIATE_FULL +#undef INSTANTIATE_HALF ConfigureLoadStore::ConfigureLoadStore(RecordComponent &rc) - : ConfigureLoadStoreCore{rc} + : core::ConfigureLoadStore{rc} {} -ConfigureLoadStore::ConfigureLoadStore(ConfigureLoadStoreCore &&core) - : ConfigureLoadStoreCore{std::move(core)} +ConfigureLoadStore::ConfigureLoadStore(core::ConfigureLoadStore &&core) + : core::ConfigureLoadStore{std::move(core)} {} } // namespace openPMD From 2269f3ea616af07184d08206de73415cd60a7612 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Mon, 22 Jul 2024 12:18:48 +0200 Subject: [PATCH 30/39] Same for store API --- include/openPMD/LoadStoreChunk.hpp | 4 +++- src/LoadStoreChunk.cpp | 24 +++++++++++++++++++++++- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/include/openPMD/LoadStoreChunk.hpp b/include/openPMD/LoadStoreChunk.hpp index 01538c1a5e..d25f2d3510 100644 --- a/include/openPMD/LoadStoreChunk.hpp +++ b/include/openPMD/LoadStoreChunk.hpp @@ -164,7 +164,9 @@ namespace core auto storeChunkConfig() -> internal::LoadStoreConfigWithBuffer; - auto enqueueStore() -> void; + auto enqueueStore() -> std::future; + + auto store(EnqueuePolicy) -> void; /** This intentionally shadows the parent class's enqueueLoad methods in * order to show a compile error when using enqueueLoad() on an object diff --git a/src/LoadStoreChunk.cpp b/src/LoadStoreChunk.cpp index cd9cb347f9..52ef46b189 100644 --- a/src/LoadStoreChunk.cpp +++ b/src/LoadStoreChunk.cpp @@ -202,12 +202,34 @@ namespace core } template - auto ConfigureStoreChunkFromBuffer::enqueueStore() -> void + auto + ConfigureStoreChunkFromBuffer::enqueueStore() -> std::future + { + this->m_rc.storeChunk_impl( + asWriteBuffer(std::move(m_buffer)), + determineDatatype>(), + storeChunkConfig()); + return std::async( + std::launch::deferred, + [rc_lambda = m_rc]() mutable -> void { rc_lambda.seriesFlush(); }); + } + + template + auto + ConfigureStoreChunkFromBuffer::store(EnqueuePolicy ep) -> void { this->m_rc.storeChunk_impl( asWriteBuffer(std::move(m_buffer)), determineDatatype>(), storeChunkConfig()); + switch (ep) + { + case EnqueuePolicy::Defer: + break; + case EnqueuePolicy::Immediate: + m_rc.seriesFlush(); + break; + } } template From c824501a467e3e046d78001abe1232eeb481b304 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon, 22 Jul 2024 10:21:14 +0000 Subject: [PATCH 31/39] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- src/IO/ADIOS/ADIOS2File.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/IO/ADIOS/ADIOS2File.cpp b/src/IO/ADIOS/ADIOS2File.cpp index f3d9500f0d..00af183642 100644 --- a/src/IO/ADIOS/ADIOS2File.cpp +++ b/src/IO/ADIOS/ADIOS2File.cpp @@ -22,13 +22,13 @@ #include "openPMD/IO/ADIOS/ADIOS2File.hpp" #include "openPMD/Error.hpp" #include "openPMD/IO/ADIOS/ADIOS2IOHandler.hpp" -#include "openPMD/IO/AbstractIOHandler.hpp" #include "openPMD/IO/ADIOS/macros.hpp" +#include "openPMD/IO/AbstractIOHandler.hpp" #include "openPMD/auxiliary/Environment.hpp" #include "openPMD/auxiliary/StringManip.hpp" -#include #include +#include #if openPMD_USE_VERIFY #define VERIFY(CONDITION, TEXT) \ From eec6aede1c8ff498f3801d97ee37a8ad2a575706 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Mon, 22 Jul 2024 16:11:45 +0200 Subject: [PATCH 32/39] Use own wrapper class DeferredFuture --- CMakeLists.txt | 1 + include/openPMD/LoadStoreChunk.hpp | 11 +++--- include/openPMD/auxiliary/Future.hpp | 26 ++++++++++++++ src/LoadStoreChunk.cpp | 36 ++++++++++---------- src/auxiliary/Future.cpp | 51 ++++++++++++++++++++++++++++ 5 files changed, 104 insertions(+), 21 deletions(-) create mode 100644 include/openPMD/auxiliary/Future.hpp create mode 100644 src/auxiliary/Future.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 77643696c8..6cc08143c4 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -468,6 +468,7 @@ set(CORE_SOURCE src/WriteIterations.cpp src/auxiliary/Date.cpp src/auxiliary/Filesystem.cpp + src/auxiliary/Future.cpp src/auxiliary/JSON.cpp src/auxiliary/Mpi.cpp src/backend/Attributable.cpp diff --git a/include/openPMD/LoadStoreChunk.hpp b/include/openPMD/LoadStoreChunk.hpp index d25f2d3510..043d185994 100644 --- a/include/openPMD/LoadStoreChunk.hpp +++ b/include/openPMD/LoadStoreChunk.hpp @@ -1,6 +1,7 @@ #pragma once #include "openPMD/Dataset.hpp" +#include "openPMD/auxiliary/Future.hpp" #include "openPMD/auxiliary/ShareRawInternal.hpp" #include "openPMD/auxiliary/UniquePtr.hpp" @@ -141,13 +142,15 @@ namespace core enqueueStore(F &&createBuffer) -> DynamicMemoryView; template - [[nodiscard]] auto enqueueLoad() -> std::future>; + [[nodiscard]] auto + enqueueLoad() -> auxiliary::DeferredFuture>; template [[nodiscard]] auto load(EnqueuePolicy) -> std::shared_ptr; - [[nodiscard]] auto enqueueLoadVariant() - -> std::future; + [[nodiscard]] auto + enqueueLoadVariant() -> auxiliary::DeferredFuture< + auxiliary::detail::shared_ptr_dataset_types>; [[nodiscard]] auto loadVariant(EnqueuePolicy) -> auxiliary::detail::shared_ptr_dataset_types; @@ -164,7 +167,7 @@ namespace core auto storeChunkConfig() -> internal::LoadStoreConfigWithBuffer; - auto enqueueStore() -> std::future; + auto enqueueStore() -> auxiliary::DeferredFuture; auto store(EnqueuePolicy) -> void; diff --git a/include/openPMD/auxiliary/Future.hpp b/include/openPMD/auxiliary/Future.hpp new file mode 100644 index 0000000000..6c5cebd822 --- /dev/null +++ b/include/openPMD/auxiliary/Future.hpp @@ -0,0 +1,26 @@ +#pragma once + +#include "openPMD/auxiliary/TypeTraits.hpp" + +#include + +namespace openPMD::auxiliary +{ +template +class DeferredFuture +{ + using task_type = std::packaged_task; + using future_type = std::future; + future_type m_future; + task_type m_task; + +public: + DeferredFuture(task_type); + + auto get() -> T; + + [[nodiscard]] auto valid() const noexcept -> bool; + + auto wait() -> void; +}; +} // namespace openPMD::auxiliary diff --git a/src/LoadStoreChunk.cpp b/src/LoadStoreChunk.cpp index 52ef46b189..f3080bb08e 100644 --- a/src/LoadStoreChunk.cpp +++ b/src/LoadStoreChunk.cpp @@ -113,15 +113,15 @@ namespace core } template - auto ConfigureLoadStore::enqueueLoad() -> std::future> + auto ConfigureLoadStore::enqueueLoad() + -> auxiliary::DeferredFuture> { auto res = m_rc.loadChunkAllocate_impl(storeChunkConfig()); - return std::async( - std::launch::deferred, + return auxiliary::DeferredFuture(std::packaged_task( [res_lambda = std::move(res), rc = m_rc]() mutable { rc.seriesFlush(); return res_lambda; - }); + })); } template @@ -143,21 +143,22 @@ namespace core { template static auto call(RecordComponent &rc, internal::LoadStoreConfig cfg) - -> std::future + -> auxiliary::DeferredFuture< + auxiliary::detail::shared_ptr_dataset_types> { auto res = rc.loadChunkAllocate_impl(std::move(cfg)); - return std::async( - std::launch::deferred, + return auxiliary::DeferredFuture(std::packaged_task( [res_lambda = std::move(res), rc_lambda = rc]() mutable -> auxiliary::detail::shared_ptr_dataset_types { rc_lambda.seriesFlush(); return res_lambda; - }); + })); } }; auto ConfigureLoadStore::enqueueLoadVariant() - -> std::future + -> auxiliary::DeferredFuture< + auxiliary::detail::shared_ptr_dataset_types> { return m_rc.visit(this->storeChunkConfig()); } @@ -202,16 +203,15 @@ namespace core } template - auto - ConfigureStoreChunkFromBuffer::enqueueStore() -> std::future + auto ConfigureStoreChunkFromBuffer::enqueueStore() + -> auxiliary::DeferredFuture { this->m_rc.storeChunk_impl( asWriteBuffer(std::move(m_buffer)), determineDatatype>(), storeChunkConfig()); - return std::async( - std::launch::deferred, - [rc_lambda = m_rc]() mutable -> void { rc_lambda.seriesFlush(); }); + return auxiliary::DeferredFuture(std::packaged_task( + [rc_lambda = m_rc]() mutable -> void { rc_lambda.seriesFlush(); })); } template @@ -300,7 +300,7 @@ template class compose::ConfigureLoadStore; template auto core::ConfigureLoadStore::enqueueStore() \ -> DynamicMemoryView; \ template auto core::ConfigureLoadStore::enqueueLoad() \ - -> std::future>; \ + -> auxiliary::DeferredFuture>; \ template auto core::ConfigureLoadStore::load(EnqueuePolicy) \ ->std::shared_ptr; // clang-format on @@ -310,13 +310,15 @@ OPENPMD_FOREACH_DATASET_DATATYPE(INSTANTIATE_METHOD_TEMPLATES) #undef INSTANTIATE_METHOD_TEMPLATES /* clang-format would destroy the NOLINT comments */ -//// clang-format off +// clang-format off #define INSTANTIATE_HALF(pointer_type) \ template class ConfigureStoreChunkFromBuffer; \ template class core::ConfigureStoreChunkFromBuffer; \ template class compose::ConfigureLoadStore< \ + /* NOLINTNEXTLINE(bugprone-macro-parentheses) */ \ ConfigureStoreChunkFromBuffer>; \ template class compose::ConfigureStoreChunkFromBuffer< \ + /* NOLINTNEXTLINE(bugprone-macro-parentheses) */ \ ConfigureStoreChunkFromBuffer>; // clang-format on @@ -325,7 +327,7 @@ OPENPMD_FOREACH_DATASET_DATATYPE(INSTANTIATE_METHOD_TEMPLATES) #define INSTANTIATE_FULL(pointer_type) \ INSTANTIATE_HALF(pointer_type) \ template class ConfigureLoadStoreFromBuffer; \ - template class core::ConfigureLoadStoreFromBuffer; \ + template class core::ConfigureLoadStoreFromBuffer; \ template class compose::ConfigureLoadStore< \ /* NOLINTNEXTLINE(bugprone-macro-parentheses) */ \ ConfigureLoadStoreFromBuffer>; \ diff --git a/src/auxiliary/Future.cpp b/src/auxiliary/Future.cpp new file mode 100644 index 0000000000..5886126527 --- /dev/null +++ b/src/auxiliary/Future.cpp @@ -0,0 +1,51 @@ +#include "openPMD/auxiliary/Future.hpp" +#include "openPMD/LoadStoreChunk.hpp" + +#include + +// comment + +#include "openPMD/DatatypeMacros.hpp" + +namespace openPMD::auxiliary +{ + +template +DeferredFuture::DeferredFuture(task_type task) + : m_future(task.get_future()), m_task(std::move(task)) +{} + +template +auto DeferredFuture::get() -> T +{ + if (m_future.valid()) + { + m_task(); + } // else get() was already called, propagate the std::future behavior + return m_future.get(); +} + +template +auto DeferredFuture::valid() const noexcept -> bool +{ + return m_future.valid(); +} + +template +auto DeferredFuture::wait() -> void +{ + if (!m_task.valid()) + { + m_task(); + } +} + +template class DeferredFuture; +template class DeferredFuture; +#define INSTANTIATE_FUTURE(dtype) \ + template class DeferredFuture>; +OPENPMD_FOREACH_DATASET_DATATYPE(INSTANTIATE_FUTURE) +#undef INSTANTIATE_FUTURE +} // namespace openPMD::auxiliary + +#include "openPMD/UndefDatatypeMacros.hpp" From f0c8652c794e32cda6c3d493049228467dc2295d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Mon, 22 Jul 2024 16:13:12 +0200 Subject: [PATCH 33/39] Avoid wrong warnings --- src/LoadStoreChunk.cpp | 3 ++- src/auxiliary/Future.cpp | 3 +++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/LoadStoreChunk.cpp b/src/LoadStoreChunk.cpp index f3080bb08e..96e8ed0f0e 100644 --- a/src/LoadStoreChunk.cpp +++ b/src/LoadStoreChunk.cpp @@ -295,11 +295,12 @@ namespace compose template class compose::ConfigureLoadStore; /* clang-format would destroy the NOLINT comments */ -//// clang-format off +// clang-format off #define INSTANTIATE_METHOD_TEMPLATES(dtype) \ template auto core::ConfigureLoadStore::enqueueStore() \ -> DynamicMemoryView; \ template auto core::ConfigureLoadStore::enqueueLoad() \ + /* NOLINTNEXTLINE(bugprone-macro-parentheses) */ \ -> auxiliary::DeferredFuture>; \ template auto core::ConfigureLoadStore::load(EnqueuePolicy) \ ->std::shared_ptr; diff --git a/src/auxiliary/Future.cpp b/src/auxiliary/Future.cpp index 5886126527..5699bb894f 100644 --- a/src/auxiliary/Future.cpp +++ b/src/auxiliary/Future.cpp @@ -42,10 +42,13 @@ auto DeferredFuture::wait() -> void template class DeferredFuture; template class DeferredFuture; +// clang-format off #define INSTANTIATE_FUTURE(dtype) \ + /* NOLINTNEXTLINE(bugprone-macro-parentheses) */ \ template class DeferredFuture>; OPENPMD_FOREACH_DATASET_DATATYPE(INSTANTIATE_FUTURE) #undef INSTANTIATE_FUTURE +// clang-format on } // namespace openPMD::auxiliary #include "openPMD/UndefDatatypeMacros.hpp" From a1382f492ce65413f465fddc4a6406baca44166d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Mon, 22 Jul 2024 16:27:44 +0200 Subject: [PATCH 34/39] Type hints for old compilers' benefit --- src/LoadStoreChunk.cpp | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/src/LoadStoreChunk.cpp b/src/LoadStoreChunk.cpp index 96e8ed0f0e..cb1a922e24 100644 --- a/src/LoadStoreChunk.cpp +++ b/src/LoadStoreChunk.cpp @@ -117,11 +117,12 @@ namespace core -> auxiliary::DeferredFuture> { auto res = m_rc.loadChunkAllocate_impl(storeChunkConfig()); - return auxiliary::DeferredFuture(std::packaged_task( - [res_lambda = std::move(res), rc = m_rc]() mutable { - rc.seriesFlush(); - return res_lambda; - })); + return auxiliary::DeferredFuture>( + std::packaged_task()>( + [res_lambda = std::move(res), rc = m_rc]() mutable { + rc.seriesFlush(); + return res_lambda; + })); } template @@ -147,12 +148,15 @@ namespace core auxiliary::detail::shared_ptr_dataset_types> { auto res = rc.loadChunkAllocate_impl(std::move(cfg)); - return auxiliary::DeferredFuture(std::packaged_task( - [res_lambda = std::move(res), rc_lambda = rc]() mutable - -> auxiliary::detail::shared_ptr_dataset_types { - rc_lambda.seriesFlush(); - return res_lambda; - })); + return auxiliary::DeferredFuture< + auxiliary::detail::shared_ptr_dataset_types>( + std::packaged_task< + auxiliary::detail::shared_ptr_dataset_types()>( + [res_lambda = std::move(res), rc_lambda = rc]() mutable + -> auxiliary::detail::shared_ptr_dataset_types { + rc_lambda.seriesFlush(); + return res_lambda; + })); } }; @@ -210,7 +214,7 @@ namespace core asWriteBuffer(std::move(m_buffer)), determineDatatype>(), storeChunkConfig()); - return auxiliary::DeferredFuture(std::packaged_task( + return auxiliary::DeferredFuture(std::packaged_task( [rc_lambda = m_rc]() mutable -> void { rc_lambda.seriesFlush(); })); } From e0f728518d1b12ecfdbf80e3cdaf6402ee131ddb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Mon, 22 Jul 2024 16:40:07 +0200 Subject: [PATCH 35/39] Fixes --- include/openPMD/RecordComponent.tpp | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/include/openPMD/RecordComponent.tpp b/include/openPMD/RecordComponent.tpp index 04e453e0cc..f297222606 100644 --- a/include/openPMD/RecordComponent.tpp +++ b/include/openPMD/RecordComponent.tpp @@ -226,7 +226,7 @@ RecordComponent::storeChunk(std::shared_ptr data, Offset o, Extent e) .offset(std::move(o)) .extent(std::move(e)) .withSharedPtr(std::move(data)) - .enqueueStore(); + .store(EnqueuePolicy::Defer); } template @@ -237,7 +237,7 @@ RecordComponent::storeChunk(UniquePtrWithLambda data, Offset o, Extent e) .offset(std::move(o)) .extent(std::move(e)) .withUniquePtr(std::move(data)) - .enqueueStore(); + .store(EnqueuePolicy::Defer); } template @@ -248,7 +248,7 @@ RecordComponent::storeChunk(std::unique_ptr data, Offset o, Extent e) .offset(std::move(o)) .extent(std::move(e)) .withUniquePtr(std::move(data)) - .enqueueStore(); + .store(EnqueuePolicy::Defer); } template @@ -258,7 +258,7 @@ void RecordComponent::storeChunkRaw(T *ptr, Offset offset, Extent extent) .offset(std::move(offset)) .extent(std::move(extent)) .withRawPtr(ptr) - .enqueueStore(); + .store(EnqueuePolicy::Defer); } template @@ -278,7 +278,9 @@ RecordComponent::storeChunk(T_ContiguousContainer &data, Offset o, Extent e) storeChunkConfig.extent(std::move(e)); } - std::move(storeChunkConfig).withContiguousContainer(data).enqueueStore(); + std::move(storeChunkConfig) + .withContiguousContainer(data) + .store(EnqueuePolicy::Defer); } template From 1fe453499d8bb36114a5ce12e433a85475dc7e43 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Tue, 23 Jul 2024 16:05:03 +0200 Subject: [PATCH 36/39] Fix rebasing error --- include/openPMD/RecordComponent.hpp | 1 - 1 file changed, 1 deletion(-) diff --git a/include/openPMD/RecordComponent.hpp b/include/openPMD/RecordComponent.hpp index b94b981b91..0a0eacace0 100644 --- a/include/openPMD/RecordComponent.hpp +++ b/include/openPMD/RecordComponent.hpp @@ -147,7 +147,6 @@ class RecordComponent : public BaseRecordComponent friend class MeshRecordComponent; template friend T &internal::makeOwning(T &self, Series); - friend struct ConfigureLoadStoreCore; friend class core::ConfigureLoadStore; template friend class core::ConfigureLoadStoreFromBuffer; From bd7b37801bbe1e6de0a0bca3d2fa8bcdd051c99e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Thu, 1 Aug 2024 15:33:59 +0200 Subject: [PATCH 37/39] Don't use std::packaged_task Intel compilers having trouble --- include/openPMD/LoadStoreChunk.hpp | 6 ++-- include/openPMD/auxiliary/Future.hpp | 13 ++++----- src/LoadStoreChunk.cpp | 42 +++++++++++++-------------- src/auxiliary/Future.cpp | 43 ++++++++++++++-------------- 4 files changed, 50 insertions(+), 54 deletions(-) diff --git a/include/openPMD/LoadStoreChunk.hpp b/include/openPMD/LoadStoreChunk.hpp index 043d185994..18af717c0a 100644 --- a/include/openPMD/LoadStoreChunk.hpp +++ b/include/openPMD/LoadStoreChunk.hpp @@ -143,13 +143,13 @@ namespace core template [[nodiscard]] auto - enqueueLoad() -> auxiliary::DeferredFuture>; + enqueueLoad() -> auxiliary::DeferredComputation>; template [[nodiscard]] auto load(EnqueuePolicy) -> std::shared_ptr; [[nodiscard]] auto - enqueueLoadVariant() -> auxiliary::DeferredFuture< + enqueueLoadVariant() -> auxiliary::DeferredComputation< auxiliary::detail::shared_ptr_dataset_types>; [[nodiscard]] auto loadVariant(EnqueuePolicy) @@ -167,7 +167,7 @@ namespace core auto storeChunkConfig() -> internal::LoadStoreConfigWithBuffer; - auto enqueueStore() -> auxiliary::DeferredFuture; + auto enqueueStore() -> auxiliary::DeferredComputation; auto store(EnqueuePolicy) -> void; diff --git a/include/openPMD/auxiliary/Future.hpp b/include/openPMD/auxiliary/Future.hpp index 6c5cebd822..d40f7372fb 100644 --- a/include/openPMD/auxiliary/Future.hpp +++ b/include/openPMD/auxiliary/Future.hpp @@ -2,25 +2,22 @@ #include "openPMD/auxiliary/TypeTraits.hpp" -#include +#include namespace openPMD::auxiliary { template -class DeferredFuture +class DeferredComputation { - using task_type = std::packaged_task; - using future_type = std::future; - future_type m_future; + using task_type = std::function; task_type m_task; + bool m_valid = false; public: - DeferredFuture(task_type); + DeferredComputation(task_type); auto get() -> T; [[nodiscard]] auto valid() const noexcept -> bool; - - auto wait() -> void; }; } // namespace openPMD::auxiliary diff --git a/src/LoadStoreChunk.cpp b/src/LoadStoreChunk.cpp index cb1a922e24..6d6d57386f 100644 --- a/src/LoadStoreChunk.cpp +++ b/src/LoadStoreChunk.cpp @@ -114,15 +114,14 @@ namespace core template auto ConfigureLoadStore::enqueueLoad() - -> auxiliary::DeferredFuture> + -> auxiliary::DeferredComputation> { auto res = m_rc.loadChunkAllocate_impl(storeChunkConfig()); - return auxiliary::DeferredFuture>( - std::packaged_task()>( - [res_lambda = std::move(res), rc = m_rc]() mutable { - rc.seriesFlush(); - return res_lambda; - })); + return auxiliary::DeferredComputation>( + [res_lambda = std::move(res), rc = m_rc]() mutable { + rc.seriesFlush(); + return res_lambda; + }); } template @@ -144,24 +143,25 @@ namespace core { template static auto call(RecordComponent &rc, internal::LoadStoreConfig cfg) - -> auxiliary::DeferredFuture< + -> auxiliary::DeferredComputation< auxiliary::detail::shared_ptr_dataset_types> { auto res = rc.loadChunkAllocate_impl(std::move(cfg)); - return auxiliary::DeferredFuture< + return auxiliary::DeferredComputation< auxiliary::detail::shared_ptr_dataset_types>( - std::packaged_task< - auxiliary::detail::shared_ptr_dataset_types()>( - [res_lambda = std::move(res), rc_lambda = rc]() mutable - -> auxiliary::detail::shared_ptr_dataset_types { - rc_lambda.seriesFlush(); - return res_lambda; - })); + + [res_lambda = std::move(res), rc_lambda = rc]() mutable + -> auxiliary::detail::shared_ptr_dataset_types { + std::cout << "Flushing Series from Future" << std::endl; + rc_lambda.seriesFlush(); + std::cout << "Flushed Series from Future" << std::endl; + return res_lambda; + }); } }; auto ConfigureLoadStore::enqueueLoadVariant() - -> auxiliary::DeferredFuture< + -> auxiliary::DeferredComputation< auxiliary::detail::shared_ptr_dataset_types> { return m_rc.visit(this->storeChunkConfig()); @@ -208,14 +208,14 @@ namespace core template auto ConfigureStoreChunkFromBuffer::enqueueStore() - -> auxiliary::DeferredFuture + -> auxiliary::DeferredComputation { this->m_rc.storeChunk_impl( asWriteBuffer(std::move(m_buffer)), determineDatatype>(), storeChunkConfig()); - return auxiliary::DeferredFuture(std::packaged_task( - [rc_lambda = m_rc]() mutable -> void { rc_lambda.seriesFlush(); })); + return auxiliary::DeferredComputation( + [rc_lambda = m_rc]() mutable -> void { rc_lambda.seriesFlush(); }); } template @@ -305,7 +305,7 @@ template class compose::ConfigureLoadStore; -> DynamicMemoryView; \ template auto core::ConfigureLoadStore::enqueueLoad() \ /* NOLINTNEXTLINE(bugprone-macro-parentheses) */ \ - -> auxiliary::DeferredFuture>; \ + -> auxiliary::DeferredComputation>; \ template auto core::ConfigureLoadStore::load(EnqueuePolicy) \ ->std::shared_ptr; // clang-format on diff --git a/src/auxiliary/Future.cpp b/src/auxiliary/Future.cpp index 5699bb894f..944fe63504 100644 --- a/src/auxiliary/Future.cpp +++ b/src/auxiliary/Future.cpp @@ -1,7 +1,9 @@ #include "openPMD/auxiliary/Future.hpp" #include "openPMD/LoadStoreChunk.hpp" +#include #include +#include // comment @@ -11,41 +13,38 @@ namespace openPMD::auxiliary { template -DeferredFuture::DeferredFuture(task_type task) - : m_future(task.get_future()), m_task(std::move(task)) +DeferredComputation::DeferredComputation(task_type task) + : m_task([wrapped_task = std::move(task), this]() { + if (!this->m_valid) + { + throw std::runtime_error( + "[DeferredComputation] No valid state. Probably already " + "computed."); + } + this->m_valid = false; + return std::move(wrapped_task)(); + }) + , m_valid(true) {} template -auto DeferredFuture::get() -> T +auto DeferredComputation::get() -> T { - if (m_future.valid()) - { - m_task(); - } // else get() was already called, propagate the std::future behavior - return m_future.get(); + return m_task(); } template -auto DeferredFuture::valid() const noexcept -> bool +auto DeferredComputation::valid() const noexcept -> bool { - return m_future.valid(); + return m_valid; } -template -auto DeferredFuture::wait() -> void -{ - if (!m_task.valid()) - { - m_task(); - } -} - -template class DeferredFuture; -template class DeferredFuture; +template class DeferredComputation; +template class DeferredComputation; // clang-format off #define INSTANTIATE_FUTURE(dtype) \ /* NOLINTNEXTLINE(bugprone-macro-parentheses) */ \ - template class DeferredFuture>; + template class DeferredComputation>; OPENPMD_FOREACH_DATASET_DATATYPE(INSTANTIATE_FUTURE) #undef INSTANTIATE_FUTURE // clang-format on From e5e06e5f1034ccb99b86cf2340fb2ba6d6a5a4fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Wed, 4 Sep 2024 16:50:16 +0200 Subject: [PATCH 38/39] Add a bit more verbose output --- src/IO/AbstractIOHandlerImpl.cpp | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/src/IO/AbstractIOHandlerImpl.cpp b/src/IO/AbstractIOHandlerImpl.cpp index 8993816f48..b4986d560c 100644 --- a/src/IO/AbstractIOHandlerImpl.cpp +++ b/src/IO/AbstractIOHandlerImpl.cpp @@ -286,7 +286,26 @@ std::future AbstractIOHandlerImpl::flush() i.writable->parent, "->", i.writable, - "] WRITE_DATASET"); + "] WRITE_DATASET: ", + [&]() { + std::stringstream stream; + stream << "offset: " << vec_as_string(parameter.offset) + << " extent: " << vec_as_string(parameter.extent) + << " mem-selection: "; + if (parameter.memorySelection.has_value()) + { + stream << vec_as_string( + parameter.memorySelection->offset) + << "--" + << vec_as_string( + parameter.memorySelection->extent); + } + else + { + stream << "NONE"; + } + return stream.str(); + }); writeDataset(i.writable, parameter); break; } From 208c381f091a618c9eb3838db0eda22be8c0d549 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Wed, 4 Sep 2024 16:50:50 +0200 Subject: [PATCH 39/39] Add missing future return type --- include/openPMD/LoadStoreChunk.hpp | 2 +- src/LoadStoreChunk.cpp | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/include/openPMD/LoadStoreChunk.hpp b/include/openPMD/LoadStoreChunk.hpp index 18af717c0a..451d6cd0cf 100644 --- a/include/openPMD/LoadStoreChunk.hpp +++ b/include/openPMD/LoadStoreChunk.hpp @@ -203,7 +203,7 @@ namespace core using ConfigureStoreChunkFromBuffer< Ptr_Type>::ConfigureStoreChunkFromBuffer; - auto enqueueLoad() -> void; + auto enqueueLoad() -> auxiliary::DeferredComputation; auto load(EnqueuePolicy) -> void; }; diff --git a/src/LoadStoreChunk.cpp b/src/LoadStoreChunk.cpp index 6d6d57386f..4571cf71e5 100644 --- a/src/LoadStoreChunk.cpp +++ b/src/LoadStoreChunk.cpp @@ -237,7 +237,8 @@ namespace core } template - auto ConfigureLoadStoreFromBuffer::enqueueLoad() -> void + auto ConfigureLoadStoreFromBuffer::enqueueLoad() + -> auxiliary::DeferredComputation { static_assert( std::is_same_v< @@ -249,6 +250,10 @@ namespace core "shared_ptr type."); this->m_rc.loadChunk_impl( std::move(this->m_buffer), this->storeChunkConfig()); + return auxiliary::DeferredComputation( + [rc_lambda = this->m_rc]() mutable -> void { + rc_lambda.seriesFlush(); + }); } template