From 2959eec090e1582749c391e4a1e4df82010e430e Mon Sep 17 00:00:00 2001 From: Florian Weik Date: Fri, 25 Oct 2019 15:52:58 +0200 Subject: [PATCH 01/11] core: ghosts: Split loading and saving archiver --- src/core/ghosts.cpp | 42 ++++++++++++++++++++++++++++++++---------- 1 file changed, 32 insertions(+), 10 deletions(-) diff --git a/src/core/ghosts.cpp b/src/core/ghosts.cpp index 6710b76e020..0a71bcb17ac 100644 --- a/src/core/ghosts.cpp +++ b/src/core/ghosts.cpp @@ -81,28 +81,50 @@ class CommBuf { * Note: The underlying buffer (span) must be large enough. This class * does *not* resize the buffer. */ -struct Archiver { +struct LoadArchiver { private: Utils::Span buf; char *insert; public: - explicit Archiver(Utils::Span buf) : buf(buf), insert(buf.data()) {} - ~Archiver() { assert(insert - buf.data() == buf.size()); } + explicit LoadArchiver(Utils::Span buf) : buf(buf), insert(buf.data()) {} + ~LoadArchiver() { assert(insert - buf.data() == buf.size()); } template ::value>::type> - void operator<<(const T &value) { - std::memcpy(insert, &value, sizeof(T)); + void operator>>(T &value) { + std::memcpy(&value, insert, sizeof(T)); insert += sizeof(T); } + template auto operator&(T &value) { + operator>>(value); + + return *this; + } +}; + +struct SaveArchiver { +private: + Utils::Span buf; + char *insert; + +public: + explicit SaveArchiver(Utils::Span buf) : buf(buf), insert(buf.data()) {} + ~SaveArchiver() { assert(insert - buf.data() == buf.size()); } + template ::value>::type> - void operator>>(T &value) { - std::memcpy(&value, insert, sizeof(T)); + void operator<<(const T &value) { + std::memcpy(insert, &value, sizeof(T)); insert += sizeof(T); } + + template auto operator&(const T &value) { + operator<<(value); + + return *this; + } }; /** @@ -198,7 +220,7 @@ static void prepare_send_buffer(CommBuf &send_buffer, send_buffer.resize(calc_transmit_size(ghost_comm, data_parts)); send_buffer.bonds().clear(); - auto archiver = Archiver{Utils::make_span(send_buffer)}; + auto archiver = SaveArchiver{Utils::make_span(send_buffer)}; auto bond_archiver = BondArchiver{send_buffer.bonds()}; /* put in data */ @@ -270,7 +292,7 @@ static void put_recv_buffer(CommBuf &recv_buffer, GhostCommunication &ghost_comm, unsigned int data_parts) { /* put back data */ - auto archiver = Archiver{Utils::make_span(recv_buffer)}; + auto archiver = LoadArchiver{Utils::make_span(recv_buffer)}; auto bond_archiver = BondArchiver{recv_buffer.bonds()}; for (auto part_list : ghost_comm.part_lists) { @@ -311,7 +333,7 @@ static void put_recv_buffer(CommBuf &recv_buffer, static void add_forces_from_recv_buffer(CommBuf &recv_buffer, GhostCommunication &ghost_comm) { /* put back data */ - auto archiver = Archiver{Utils::make_span(recv_buffer)}; + auto archiver = LoadArchiver{Utils::make_span(recv_buffer)}; for (auto &part_list : ghost_comm.part_lists) { for (Particle &part : part_list->particles()) { ParticleForce pf; From 5192f35fdb8799a5882bdaee5670a885f40372a8 Mon Sep 17 00:00:00 2001 From: Florian Weik Date: Fri, 25 Oct 2019 16:02:45 +0200 Subject: [PATCH 02/11] core: ghosts: Split off bond archiving --- src/core/ghosts.cpp | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/core/ghosts.cpp b/src/core/ghosts.cpp index 0a71bcb17ac..f376aaba9c5 100644 --- a/src/core/ghosts.cpp +++ b/src/core/ghosts.cpp @@ -232,10 +232,6 @@ static void prepare_send_buffer(CommBuf &send_buffer, for (Particle const &part : part_list->particles()) { if (data_parts & GHOSTTRANS_PROPRTS) { archiver << part.p; - if (ghosts_have_bonds) { - archiver << static_cast(part.bl.n); - bond_archiver << Utils::make_const_span(part.bl); - } } if (data_parts & GHOSTTRANS_POSITION) { /* ok, this is not nice, but perhaps fast */ @@ -249,6 +245,10 @@ static void prepare_send_buffer(CommBuf &send_buffer, if (data_parts & GHOSTTRANS_FORCE) { archiver << part.f; } + if (ghosts_have_bonds && (data_parts & GHOSTTRANS_PROPRTS)) { + archiver << static_cast(part.bl.n); + bond_archiver << Utils::make_const_span(part.bl); + } } } } @@ -304,12 +304,6 @@ static void put_recv_buffer(CommBuf &recv_buffer, for (Particle &part : part_list->particles()) { if (data_parts & GHOSTTRANS_PROPRTS) { archiver >> part.p; - if (ghosts_have_bonds) { - int n_bonds; - archiver >> n_bonds; - part.bl.resize(n_bonds); - bond_archiver >> Utils::make_span(part.bl); - } if (local_particles[part.p.identity] == nullptr) { local_particles[part.p.identity] = ∂ } @@ -323,6 +317,12 @@ static void put_recv_buffer(CommBuf &recv_buffer, if (data_parts & GHOSTTRANS_FORCE) { archiver >> part.f; } + if (ghosts_have_bonds && (data_parts & GHOSTTRANS_PROPRTS)) { + int n_bonds; + archiver >> n_bonds; + part.bl.resize(n_bonds); + bond_archiver >> Utils::make_span(part.bl); + } } } } From 61d49e73009fd5c4ec214bd9aff178ac16adb106 Mon Sep 17 00:00:00 2001 From: Florian Weik Date: Fri, 25 Oct 2019 16:16:52 +0200 Subject: [PATCH 03/11] core: ghosts: Treat bonds as data_part --- src/core/ghosts.cpp | 20 +++++++++++--------- src/core/ghosts.hpp | 3 ++- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/src/core/ghosts.cpp b/src/core/ghosts.cpp index f376aaba9c5..3ad785a054c 100644 --- a/src/core/ghosts.cpp +++ b/src/core/ghosts.cpp @@ -193,10 +193,9 @@ static int calc_transmit_size(GhostCommunication &ghost_comm, n_buffer_new = 0; if (data_parts & GHOSTTRANS_PROPRTS) { n_buffer_new += sizeof(ParticleProperties); - // sending size of bond lists - if (ghosts_have_bonds) { - n_buffer_new += sizeof(int); - } + } + if (data_parts & GHOSTTRANS_BONDS) { + n_buffer_new += sizeof(int); } if (data_parts & GHOSTTRANS_POSITION) n_buffer_new += sizeof(ParticlePosition); @@ -245,7 +244,7 @@ static void prepare_send_buffer(CommBuf &send_buffer, if (data_parts & GHOSTTRANS_FORCE) { archiver << part.f; } - if (ghosts_have_bonds && (data_parts & GHOSTTRANS_PROPRTS)) { + if (data_parts & GHOSTTRANS_BONDS) { archiver << static_cast(part.bl.n); bond_archiver << Utils::make_const_span(part.bl); } @@ -317,7 +316,7 @@ static void put_recv_buffer(CommBuf &recv_buffer, if (data_parts & GHOSTTRANS_FORCE) { archiver >> part.f; } - if (ghosts_have_bonds && (data_parts & GHOSTTRANS_PROPRTS)) { + if (data_parts & GHOSTTRANS_BONDS) { int n_bonds; archiver >> n_bonds; part.bl.resize(n_bonds); @@ -360,9 +359,9 @@ static void cell_cell_transfer(GhostCommunication &ghost_comm, Particle &part2 = dst_list->part[p]; if (data_parts & GHOSTTRANS_PROPRTS) { part2.p = part1.p; - if (ghosts_have_bonds) { - part2.bl = part1.bl; - } + } + if (data_parts & GHOSTTRANS_BONDS) { + part2.bl = part1.bl; } if (data_parts & GHOSTTRANS_POSITION) { /* ok, this is not nice, but perhaps fast */ @@ -412,6 +411,9 @@ void ghost_communicator(GhostCommunicator *gcr, unsigned int data_parts) { if (ghosts_have_v && (data_parts & GHOSTTRANS_POSITION)) data_parts |= GHOSTTRANS_MOMENTUM; + if (ghosts_have_bonds && (data_parts & GHOSTTRANS_PROPRTS)) + data_parts |= GHOSTTRANS_BONDS; + for (auto it = gcr->comm.begin(); it != gcr->comm.end(); ++it) { GhostCommunication &ghost_comm = *it; int const comm_type = ghost_comm.type & GHOST_JOBMASK; diff --git a/src/core/ghosts.hpp b/src/core/ghosts.hpp index dc2c4f58d6c..f97f6d78128 100644 --- a/src/core/ghosts.hpp +++ b/src/core/ghosts.hpp @@ -134,7 +134,8 @@ enum : unsigned { /// transfer \ref ParticleForce GHOSTTRANS_FORCE = 16u, /// resize the receiver particle arrays to the size of the senders - GHOSTTRANS_PARTNUM = 64u + GHOSTTRANS_PARTNUM = 64u, + GHOSTTRANS_BONDS = 128u }; /** \name Data Types */ From 324deed24739e46c9f6364c60750e5812df8a33f Mon Sep 17 00:00:00 2001 From: Florian Weik Date: Mon, 28 Oct 2019 15:51:09 +0100 Subject: [PATCH 04/11] utils: memcpy serialization archive + test --- .../utils/serialization/memcpy_archive.hpp | 231 ++++++++++++++++++ src/utils/tests/CMakeLists.txt | 1 + src/utils/tests/memcpy_archive_test.cpp | 109 +++++++++ 3 files changed, 341 insertions(+) create mode 100644 src/utils/include/utils/serialization/memcpy_archive.hpp create mode 100644 src/utils/tests/memcpy_archive_test.cpp diff --git a/src/utils/include/utils/serialization/memcpy_archive.hpp b/src/utils/include/utils/serialization/memcpy_archive.hpp new file mode 100644 index 00000000000..9a98d8cd45f --- /dev/null +++ b/src/utils/include/utils/serialization/memcpy_archive.hpp @@ -0,0 +1,231 @@ +/* + * Copyright (C) 2019 The ESPResSo project + * + * This file is part of ESPResSo. + * + * ESPResSo is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * ESPResSo is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +#ifndef ESPRESSO_MEMCPY_ARCHIVE_HPP +#define ESPRESSO_MEMCPY_ARCHIVE_HPP + +#include + +#include +#include +#include + +#include + +namespace Utils { +/** @brief Type trait to indicate that a type is + * serializable with a static size, e.g. is + * suitable for memcpy serialization. Only + * specialize this to std::true_type if it is + * guarantueed that serializing this type always + * returns the same number of bytes, independent + * of object state. + * + * @tparam T type under consideration. + */ +template struct is_statically_serializable : std::false_type {}; + +namespace detail { +/* Use serialize function only if the type is opt-in but not + * trivially copyable, in which case memcpy is more efficient. */ +template +using use_serialize = + std::integral_constant::value and + is_statically_serializable::value>; + +struct SizeArchive { + using is_saving = boost::mpl::true_; + using is_loading = boost::mpl::false_; + + size_t size = {}; + + template + constexpr auto operator<<(T const &) + -> std::enable_if_t::value> { + size += sizeof(T); + } + + template + constexpr auto operator<<(T &v) + -> std::enable_if_t::value> { + boost::serialization::serialize(*this, v, 0); + } + + template constexpr auto operator<<(boost::optional &) { + operator<<(bool()); + operator<<(T{}); + } + + template constexpr SizeArchive &operator&(T &t) { + operator<<(t); + + return *this; + } +}; + +class BasicMemcpyArchive { + /** Buffer to write to */ + Utils::Span buf; + /** Current position in the buffer */ + char *insert; + +public: + explicit BasicMemcpyArchive(Utils::Span buf) + : buf(buf), insert(buf.data()) {} + + size_t bytes_processed() const { return insert - buf.data(); } + + template + auto operator>>(T &value) + -> std::enable_if_t::value> { + /* check that there is enough space left in the buffer */ + assert((insert + sizeof(T)) <= buf.end()); + + std::memcpy(&value, insert, sizeof(T)); + insert += sizeof(T); + } + + template + auto operator<<(T const &value) + -> std::enable_if_t::value> { + /* check that there is enough space left in the buffer */ + assert((insert + sizeof(T)) <= buf.end()); + std::memcpy(insert, &value, sizeof(T)); + insert += sizeof(T); + } + + /** + * @brief Determine the static packing size of a type. + * @tparam T Type to consider. + * @return Packed size in bytes. + */ + template static size_t packing_size() { + detail::SizeArchive sa{}; + T t; + return (sa & t).size; + } +}; +} // namespace detail + +/** + * @brief Archive that deserializes from a buffer via memcpy. + * + * Can only process types that have a static serialization size, + * e.g. that serialize to the same number of bytes independent of + * the state of the object. This can either be automatically detected + * for types that are trivially copyable, or by explicitly assuring + * this by specializing @c is_statically_serializable to std::true_type. + */ +class MemcpyIArchive : public detail::BasicMemcpyArchive { +private: + using base_type = detail::BasicMemcpyArchive; + +public: + using is_loading = boost::mpl::true_; + using is_saving = boost::mpl::false_; + + /** + * @param buf Buffer to read from. + */ + explicit MemcpyIArchive(Utils::Span buf) + : detail::BasicMemcpyArchive(buf) {} + + /** + * @brief Number of bytes read from the buffer. + * @return Number of bytes read. + */ + size_t bytes_read() const { return bytes_processed(); } + + /** @copydoc base_type::packing_size */ + using base_type::packing_size; + using base_type::operator>>; + + template + auto operator>>(T &value) + -> std::enable_if_t::value> { + boost::serialization::serialize(*this, value, 0); + } + + template void operator>>(boost::optional &o) { + bool initialized{}; + operator>>(initialized); + T val{}; + operator>>(val); + + if (initialized) { + o = val; + } else { + o = boost::none; + } + } + + template MemcpyIArchive &operator&(T &value) { + operator>>(value); + + return *this; + } +}; + +/** + * @brief Archive that serializes to a buffer via memcpy. + * + * @copydetails MemcpyIArchive + */ +class MemcpyOArchive : public detail::BasicMemcpyArchive { + using base_type = detail::BasicMemcpyArchive; + +public: + using is_loading = boost::mpl::false_; + using is_saving = boost::mpl::true_; + + /** + * @param buf Buffer to write to. + */ + explicit MemcpyOArchive(Utils::Span buf) + : detail::BasicMemcpyArchive(buf) {} + + /** + * @brief Number of bytes written to the buffer. + * @return Number of bytes written. + */ + size_t bytes_written() const { return bytes_processed(); } + + /** @copydoc base_type::packing_size */ + using base_type::packing_size; + using base_type::operator<<; + + template + auto operator<<(T &value) + -> std::enable_if_t::value> { + boost::serialization::serialize(*this, value, 0); + } + + template void operator<<(boost::optional &o) { + operator<<(o.is_initialized()); + operator<<(o.value_or(T{})); + } + + template MemcpyOArchive &operator&(T &value) { + operator<<(value); + + return *this; + } +}; +} // namespace Utils + +#endif // ESPRESSO_MEMCPY_ARCHIVE_HPP diff --git a/src/utils/tests/CMakeLists.txt b/src/utils/tests/CMakeLists.txt index 2fbe6cbd52e..d3c7380ce3e 100644 --- a/src/utils/tests/CMakeLists.txt +++ b/src/utils/tests/CMakeLists.txt @@ -40,6 +40,7 @@ unit_test(NAME quaternion_test SRC quaternion_test.cpp DEPENDS utils) unit_test(NAME mask_test SRC mask_test.cpp DEPENDS utils) unit_test(NAME type_traits_test SRC type_traits_test.cpp DEPENDS utils) unit_test(NAME uniform_test SRC uniform_test.cpp DEPENDS utils) +unit_test(NAME memcpy_archive_test SRC memcpy_archive_test.cpp DEPENDS utils) unit_test(NAME gather_buffer_test SRC gather_buffer_test.cpp DEPENDS utils Boost::mpi MPI::MPI_CXX NUM_PROC 4) unit_test(NAME scatter_buffer_test SRC scatter_buffer_test.cpp DEPENDS utils Boost::mpi MPI::MPI_CXX NUM_PROC 4) diff --git a/src/utils/tests/memcpy_archive_test.cpp b/src/utils/tests/memcpy_archive_test.cpp new file mode 100644 index 00000000000..4a0441e0d9d --- /dev/null +++ b/src/utils/tests/memcpy_archive_test.cpp @@ -0,0 +1,109 @@ +/* + * Copyright (C) 2019 The ESPResSo project + * + * This file is part of ESPResSo. + * + * ESPResSo is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * ESPResSo is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#define BOOST_TEST_MODULE memcpy archive test +#define BOOST_TEST_DYN_LINK +#include + +#include "utils/serialization/memcpy_archive.hpp" + +#include +#include + +#include + +struct NonTrivial { + boost::optional ov; + + template void serialize(Archive &ar, long int) { ar &ov; } +}; + +using OpVec = boost::optional; + +namespace Utils { +template <> struct is_statically_serializable : std::true_type {}; +} // namespace Utils + +BOOST_AUTO_TEST_CASE(packing_size_test) { + BOOST_CHECK_EQUAL(Utils::MemcpyIArchive::packing_size(), sizeof(int)); + + { + BOOST_CHECK_EQUAL(Utils::MemcpyIArchive::packing_size(), + Utils::MemcpyIArchive::packing_size()); + } +} + +BOOST_AUTO_TEST_CASE(optional_packing) { + std::vector buf(2 * Utils::MemcpyOArchive::packing_size()); + + const OpVec active = Utils::Vector3d{1., 2., 3.}; + const OpVec inactive = boost::none; + { + auto oa = Utils::MemcpyOArchive{Utils::make_span(buf)}; + auto in1 = active; + auto in2 = inactive; + oa << in1; + oa << in2; + + BOOST_CHECK_EQUAL(oa.bytes_written(), buf.size()); + } + + { + auto ia = Utils::MemcpyIArchive{Utils::make_span(buf)}; + OpVec out1, out2; + ia >> out1; + ia >> out2; + + BOOST_CHECK_EQUAL(ia.bytes_read(), buf.size()); + + BOOST_CHECK(out1 == active); + BOOST_CHECK(out2 == inactive); + } +} + +BOOST_AUTO_TEST_CASE(non_trivial_packing) { + static_assert(not std::is_trivially_copyable::value, ""); + + std::vector buf(2 * Utils::MemcpyOArchive::packing_size()); + + auto const active = NonTrivial{Utils::Vector3d{1., 2., 3.}}; + auto const inactive = NonTrivial{boost::none}; + + { + auto oa = Utils::MemcpyOArchive{Utils::make_span(buf)}; + auto in1 = active; + auto in2 = inactive; + oa << in1; + oa << in2; + + BOOST_CHECK_EQUAL(oa.bytes_written(), buf.size()); + } + + { + auto ia = Utils::MemcpyIArchive{Utils::make_span(buf)}; + NonTrivial out1, out2; + ia >> out1; + ia >> out2; + + BOOST_CHECK_EQUAL(ia.bytes_read(), buf.size()); + + BOOST_CHECK(out1.ov == active.ov); + BOOST_CHECK(out2.ov == inactive.ov); + } +} \ No newline at end of file From ffc3c58212bb4596cd5ce8dfb01ef6e24a4a91e3 Mon Sep 17 00:00:00 2001 From: Florian Weik Date: Mon, 28 Oct 2019 15:51:52 +0100 Subject: [PATCH 05/11] core: ghosts: Use memcpy serialization from Utils --- src/core/ghosts.cpp | 78 +++++++-------------------- src/core/unit_tests/Particle_test.cpp | 36 +++++++++++++ 2 files changed, 55 insertions(+), 59 deletions(-) diff --git a/src/core/ghosts.cpp b/src/core/ghosts.cpp index 3ad785a054c..e91ea0f2be0 100644 --- a/src/core/ghosts.cpp +++ b/src/core/ghosts.cpp @@ -34,9 +34,11 @@ #include "errorhandling.hpp" #include "particle_data.hpp" -#include #include #include +#include + +#include #include #include @@ -47,6 +49,10 @@ /** Tag for ghosts communications. */ #define REQ_GHOST_SEND 100 +template <> +struct Utils::is_statically_serializable : std::true_type { +}; + /** * Class that stores marshalled data for ghost communications. * To store and retrieve data, use the adapter classes below. @@ -76,57 +82,6 @@ class CommBuf { std::vector bondbuf; //< Buffer for bond lists }; -/** - * Adapter to CommBuf that allows putting in and getting out data. - * Note: The underlying buffer (span) must be large enough. This class - * does *not* resize the buffer. - */ -struct LoadArchiver { -private: - Utils::Span buf; - char *insert; - -public: - explicit LoadArchiver(Utils::Span buf) : buf(buf), insert(buf.data()) {} - ~LoadArchiver() { assert(insert - buf.data() == buf.size()); } - - template ::value>::type> - void operator>>(T &value) { - std::memcpy(&value, insert, sizeof(T)); - insert += sizeof(T); - } - - template auto operator&(T &value) { - operator>>(value); - - return *this; - } -}; - -struct SaveArchiver { -private: - Utils::Span buf; - char *insert; - -public: - explicit SaveArchiver(Utils::Span buf) : buf(buf), insert(buf.data()) {} - ~SaveArchiver() { assert(insert - buf.data() == buf.size()); } - - template ::value>::type> - void operator<<(const T &value) { - std::memcpy(insert, &value, sizeof(T)); - insert += sizeof(T); - } - - template auto operator&(const T &value) { - operator<<(value); - - return *this; - } -}; - /** * Adapter to CommBuf that allows putting in and getting back bond data. * Note, this class inserts the data handed to operator<< at the end @@ -192,7 +147,7 @@ static int calc_transmit_size(GhostCommunication &ghost_comm, else { n_buffer_new = 0; if (data_parts & GHOSTTRANS_PROPRTS) { - n_buffer_new += sizeof(ParticleProperties); + n_buffer_new += Utils::MemcpyOArchive::packing_size(); } if (data_parts & GHOSTTRANS_BONDS) { n_buffer_new += sizeof(int); @@ -219,7 +174,7 @@ static void prepare_send_buffer(CommBuf &send_buffer, send_buffer.resize(calc_transmit_size(ghost_comm, data_parts)); send_buffer.bonds().clear(); - auto archiver = SaveArchiver{Utils::make_span(send_buffer)}; + auto archiver = Utils::MemcpyOArchive{Utils::make_span(send_buffer)}; auto bond_archiver = BondArchiver{send_buffer.bonds()}; /* put in data */ @@ -228,7 +183,7 @@ static void prepare_send_buffer(CommBuf &send_buffer, int np = part_list->n; archiver << np; } else { - for (Particle const &part : part_list->particles()) { + for (Particle &part : part_list->particles()) { if (data_parts & GHOSTTRANS_PROPRTS) { archiver << part.p; } @@ -245,12 +200,14 @@ static void prepare_send_buffer(CommBuf &send_buffer, archiver << part.f; } if (data_parts & GHOSTTRANS_BONDS) { - archiver << static_cast(part.bl.n); + archiver << part.bl.n; bond_archiver << Utils::make_const_span(part.bl); } } } } + + assert(archiver.bytes_written() == send_buffer.size()); } static void prepare_ghost_cell(Cell *cell, int size) { @@ -291,7 +248,7 @@ static void put_recv_buffer(CommBuf &recv_buffer, GhostCommunication &ghost_comm, unsigned int data_parts) { /* put back data */ - auto archiver = LoadArchiver{Utils::make_span(recv_buffer)}; + auto archiver = Utils::MemcpyIArchive{Utils::make_span(recv_buffer)}; auto bond_archiver = BondArchiver{recv_buffer.bonds()}; for (auto part_list : ghost_comm.part_lists) { @@ -317,7 +274,7 @@ static void put_recv_buffer(CommBuf &recv_buffer, archiver >> part.f; } if (data_parts & GHOSTTRANS_BONDS) { - int n_bonds; + decltype(part.bl.n) n_bonds; archiver >> n_bonds; part.bl.resize(n_bonds); bond_archiver >> Utils::make_span(part.bl); @@ -326,13 +283,16 @@ static void put_recv_buffer(CommBuf &recv_buffer, } } + if (archiver.bytes_read() != recv_buffer.size()) { + throw std::runtime_error("packing size mismatch receiver"); + } recv_buffer.bonds().clear(); } static void add_forces_from_recv_buffer(CommBuf &recv_buffer, GhostCommunication &ghost_comm) { /* put back data */ - auto archiver = LoadArchiver{Utils::make_span(recv_buffer)}; + auto archiver = Utils::MemcpyIArchive{Utils::make_span(recv_buffer)}; for (auto &part_list : ghost_comm.part_lists) { for (Particle &part : part_list->particles()) { ParticleForce pf; diff --git a/src/core/unit_tests/Particle_test.cpp b/src/core/unit_tests/Particle_test.cpp index bf22d8f1ba7..59cbd0cc3b7 100644 --- a/src/core/unit_tests/Particle_test.cpp +++ b/src/core/unit_tests/Particle_test.cpp @@ -29,6 +29,8 @@ #include #include +#include + #include "Particle.hpp" #include "serialization/Particle.hpp" @@ -81,3 +83,37 @@ BOOST_AUTO_TEST_CASE(serialization) { BOOST_CHECK(q.el == el); #endif } + +namespace Utils { +template <> +struct is_statically_serializable : std::true_type {}; +} // namespace Utils + +BOOST_AUTO_TEST_CASE(properties_serialization) { + auto const expected_size = + Utils::MemcpyOArchive::packing_size(); + + BOOST_CHECK_LE(expected_size, sizeof(ParticleProperties)); + + std::vector buf(expected_size); + + auto prop = ParticleProperties{}; + prop.identity = 1234; + + { + auto oa = Utils::MemcpyOArchive{Utils::make_span(buf)}; + + oa << prop; + + BOOST_CHECK_EQUAL(oa.bytes_written(), expected_size); + } + + { + auto ia = Utils::MemcpyIArchive{Utils::make_span(buf)}; + ParticleProperties out; + + ia >> out; + BOOST_CHECK_EQUAL(ia.bytes_read(), expected_size); + BOOST_CHECK_EQUAL(out.identity, prop.identity); + } +} \ No newline at end of file From 54eb0bba9e29e550e1a9098cc9bcefbb4d00c156 Mon Sep 17 00:00:00 2001 From: Florian Weik Date: Mon, 28 Oct 2019 17:04:54 +0100 Subject: [PATCH 06/11] core: ghosts: calculate transmit size in size_t --- src/core/ghosts.cpp | 53 +++++++++++++++++++++++---------------------- 1 file changed, 27 insertions(+), 26 deletions(-) diff --git a/src/core/ghosts.cpp b/src/core/ghosts.cpp index e91ea0f2be0..aa2472179a5 100644 --- a/src/core/ghosts.cpp +++ b/src/core/ghosts.cpp @@ -41,6 +41,7 @@ #include #include +#include #include #include #include @@ -138,33 +139,34 @@ void free_comm(GhostCommunicator *gcr) { ghost_comm.part_lists.clear(); } -static int calc_transmit_size(GhostCommunication &ghost_comm, - unsigned int data_parts) { - int n_buffer_new; +static size_t calc_transmit_size(unsigned data_parts) { + size_t size = {}; + if (data_parts & GHOSTTRANS_PROPRTS) { + size += Utils::MemcpyOArchive::packing_size(); + } + if (data_parts & GHOSTTRANS_BONDS) { + size += sizeof(int); + } + if (data_parts & GHOSTTRANS_POSITION) + size += sizeof(ParticlePosition); + if (data_parts & GHOSTTRANS_MOMENTUM) + size += sizeof(ParticleMomentum); + if (data_parts & GHOSTTRANS_FORCE) + size += sizeof(ParticleForce); + + return size; +} +static size_t calc_transmit_size(GhostCommunication &ghost_comm, + unsigned int data_parts) { if (data_parts & GHOSTTRANS_PARTNUM) - n_buffer_new = sizeof(int) * ghost_comm.part_lists.size(); + return sizeof(int) * ghost_comm.part_lists.size(); else { - n_buffer_new = 0; - if (data_parts & GHOSTTRANS_PROPRTS) { - n_buffer_new += Utils::MemcpyOArchive::packing_size(); - } - if (data_parts & GHOSTTRANS_BONDS) { - n_buffer_new += sizeof(int); - } - if (data_parts & GHOSTTRANS_POSITION) - n_buffer_new += sizeof(ParticlePosition); - if (data_parts & GHOSTTRANS_MOMENTUM) - n_buffer_new += sizeof(ParticleMomentum); - if (data_parts & GHOSTTRANS_FORCE) - n_buffer_new += sizeof(ParticleForce); - - int count = 0; - for (auto const &pl : ghost_comm.part_lists) - count += pl->n; - n_buffer_new *= count; + auto const n_part = boost::accumulate( + ghost_comm.part_lists, 0ul, + [](size_t sum, auto part_list) { return sum + part_list->n; }); + return n_part * calc_transmit_size(data_parts); } - return n_buffer_new; } static void prepare_send_buffer(CommBuf &send_buffer, @@ -283,9 +285,8 @@ static void put_recv_buffer(CommBuf &recv_buffer, } } - if (archiver.bytes_read() != recv_buffer.size()) { - throw std::runtime_error("packing size mismatch receiver"); - } + assert(archiver.bytes_read() == recv_buffer.size()); + recv_buffer.bonds().clear(); } From 344d8de332dbe15e18dc87b6277900dd1b7fac67 Mon Sep 17 00:00:00 2001 From: Florian Weik Date: Wed, 30 Oct 2019 22:53:49 +0100 Subject: [PATCH 07/11] utils: memcpy_archive: Improved cooperation with boost.serialization, removed special overload for boost::optional --- .../utils/serialization/memcpy_archive.hpp | 143 ++++++++---------- src/utils/tests/memcpy_archive_test.cpp | 80 ++++++---- 2 files changed, 110 insertions(+), 113 deletions(-) diff --git a/src/utils/include/utils/serialization/memcpy_archive.hpp b/src/utils/include/utils/serialization/memcpy_archive.hpp index 9a98d8cd45f..ded276cdd99 100644 --- a/src/utils/include/utils/serialization/memcpy_archive.hpp +++ b/src/utils/include/utils/serialization/memcpy_archive.hpp @@ -22,7 +22,8 @@ #include #include -#include +#include +#include #include #include @@ -38,47 +39,26 @@ namespace Utils { * * @tparam T type under consideration. */ -template struct is_statically_serializable : std::false_type {}; +template +struct is_statically_serializable + : std::integral_constant< + bool, std::is_trivially_copyable::value or + boost::serialization::is_bitwise_serializable::value> {}; namespace detail { +/* Use memcpy for packing */ +template +using use_memcpy = std::integral_constant< + bool, std::is_trivially_copyable::value or + boost::serialization::is_bitwise_serializable::value>; /* Use serialize function only if the type is opt-in but not * trivially copyable, in which case memcpy is more efficient. */ template using use_serialize = - std::integral_constant::value and + std::integral_constant::value and is_statically_serializable::value>; -struct SizeArchive { - using is_saving = boost::mpl::true_; - using is_loading = boost::mpl::false_; - - size_t size = {}; - - template - constexpr auto operator<<(T const &) - -> std::enable_if_t::value> { - size += sizeof(T); - } - - template - constexpr auto operator<<(T &v) - -> std::enable_if_t::value> { - boost::serialization::serialize(*this, v, 0); - } - - template constexpr auto operator<<(boost::optional &) { - operator<<(bool()); - operator<<(T{}); - } - - template constexpr SizeArchive &operator&(T &t) { - operator<<(t); - - return *this; - } -}; - -class BasicMemcpyArchive { +template class BasicMemcpyArchive { /** Buffer to write to */ Utils::Span buf; /** Current position in the buffer */ @@ -88,11 +68,16 @@ class BasicMemcpyArchive { explicit BasicMemcpyArchive(Utils::Span buf) : buf(buf), insert(buf.data()) {} + size_t get_library_version() const { return 4; } + size_t bytes_processed() const { return insert - buf.data(); } + void skip(size_t bytes) { + assert((insert + bytes) <= buf.end()); + insert += bytes; + } template - auto operator>>(T &value) - -> std::enable_if_t::value> { + auto operator>>(T &value) -> std::enable_if_t::value> { /* check that there is enough space left in the buffer */ assert((insert + sizeof(T)) <= buf.end()); @@ -101,23 +86,53 @@ class BasicMemcpyArchive { } template - auto operator<<(T const &value) - -> std::enable_if_t::value> { + auto operator<<(T const &value) -> std::enable_if_t::value> { /* check that there is enough space left in the buffer */ assert((insert + sizeof(T)) <= buf.end()); std::memcpy(insert, &value, sizeof(T)); insert += sizeof(T); } +private: + template void process(T &value) { + auto const old_pos = insert; + boost::serialization::serialize_adl(*static_cast(this), value, + 4); + auto const new_pos = insert; + assert((new_pos - old_pos) <= sizeof(T)); + + auto const padding_size = sizeof(T) - (new_pos - old_pos); + skip(padding_size); + } + +public: + template + auto operator>>(T &value) + -> std::enable_if_t::value> { + process(value); + } + + template + auto operator<<(T &value) + -> std::enable_if_t::value> { + process(value); + } + + template void operator<<(const boost::serialization::nvp &nvp) { + operator<<(nvp.const_value()); + } + + template void operator>>(const boost::serialization::nvp &nvp) { + operator>>(nvp.value()); + } + /** * @brief Determine the static packing size of a type. * @tparam T Type to consider. * @return Packed size in bytes. */ - template static size_t packing_size() { - detail::SizeArchive sa{}; - T t; - return (sa & t).size; + template static constexpr size_t packing_size() { + return sizeof(T); } }; } // namespace detail @@ -131,9 +146,9 @@ class BasicMemcpyArchive { * for types that are trivially copyable, or by explicitly assuring * this by specializing @c is_statically_serializable to std::true_type. */ -class MemcpyIArchive : public detail::BasicMemcpyArchive { +class MemcpyIArchive : public detail::BasicMemcpyArchive { private: - using base_type = detail::BasicMemcpyArchive; + using base_type = detail::BasicMemcpyArchive; public: using is_loading = boost::mpl::true_; @@ -142,8 +157,7 @@ class MemcpyIArchive : public detail::BasicMemcpyArchive { /** * @param buf Buffer to read from. */ - explicit MemcpyIArchive(Utils::Span buf) - : detail::BasicMemcpyArchive(buf) {} + explicit MemcpyIArchive(Utils::Span buf) : base_type(buf) {} /** * @brief Number of bytes read from the buffer. @@ -155,25 +169,6 @@ class MemcpyIArchive : public detail::BasicMemcpyArchive { using base_type::packing_size; using base_type::operator>>; - template - auto operator>>(T &value) - -> std::enable_if_t::value> { - boost::serialization::serialize(*this, value, 0); - } - - template void operator>>(boost::optional &o) { - bool initialized{}; - operator>>(initialized); - T val{}; - operator>>(val); - - if (initialized) { - o = val; - } else { - o = boost::none; - } - } - template MemcpyIArchive &operator&(T &value) { operator>>(value); @@ -186,8 +181,8 @@ class MemcpyIArchive : public detail::BasicMemcpyArchive { * * @copydetails MemcpyIArchive */ -class MemcpyOArchive : public detail::BasicMemcpyArchive { - using base_type = detail::BasicMemcpyArchive; +class MemcpyOArchive : public detail::BasicMemcpyArchive { + using base_type = detail::BasicMemcpyArchive; public: using is_loading = boost::mpl::false_; @@ -196,8 +191,7 @@ class MemcpyOArchive : public detail::BasicMemcpyArchive { /** * @param buf Buffer to write to. */ - explicit MemcpyOArchive(Utils::Span buf) - : detail::BasicMemcpyArchive(buf) {} + explicit MemcpyOArchive(Utils::Span buf) : base_type(buf) {} /** * @brief Number of bytes written to the buffer. @@ -209,17 +203,6 @@ class MemcpyOArchive : public detail::BasicMemcpyArchive { using base_type::packing_size; using base_type::operator<<; - template - auto operator<<(T &value) - -> std::enable_if_t::value> { - boost::serialization::serialize(*this, value, 0); - } - - template void operator<<(boost::optional &o) { - operator<<(o.is_initialized()); - operator<<(o.value_or(T{})); - } - template MemcpyOArchive &operator&(T &value) { operator<<(value); diff --git a/src/utils/tests/memcpy_archive_test.cpp b/src/utils/tests/memcpy_archive_test.cpp index 4a0441e0d9d..713b12f34ea 100644 --- a/src/utils/tests/memcpy_archive_test.cpp +++ b/src/utils/tests/memcpy_archive_test.cpp @@ -21,10 +21,14 @@ #define BOOST_TEST_DYN_LINK #include -#include "utils/serialization/memcpy_archive.hpp" +#include -#include #include +#include + +#include +#include +#include #include @@ -38,6 +42,14 @@ using OpVec = boost::optional; namespace Utils { template <> struct is_statically_serializable : std::true_type {}; + +template +struct is_statically_serializable> + : is_statically_serializable {}; + +template +struct is_statically_serializable> + : Utils::conjunction...> {}; } // namespace Utils BOOST_AUTO_TEST_CASE(packing_size_test) { @@ -49,42 +61,44 @@ BOOST_AUTO_TEST_CASE(packing_size_test) { } } -BOOST_AUTO_TEST_CASE(optional_packing) { - std::vector buf(2 * Utils::MemcpyOArchive::packing_size()); +BOOST_AUTO_TEST_CASE(type_traits) { + static_assert(Utils::is_statically_serializable::value, ""); + static_assert(not Utils::detail::use_memcpy::value, ""); + static_assert(Utils::detail::use_serialize::value, ""); +} - const OpVec active = Utils::Vector3d{1., 2., 3.}; - const OpVec inactive = boost::none; - { - auto oa = Utils::MemcpyOArchive{Utils::make_span(buf)}; - auto in1 = active; - auto in2 = inactive; - oa << in1; - oa << in2; +BOOST_AUTO_TEST_CASE(skiping_and_position) { + std::vector buf(10); - BOOST_CHECK_EQUAL(oa.bytes_written(), buf.size()); - } + auto ar = Utils::MemcpyOArchive(Utils::make_span(buf)); - { - auto ia = Utils::MemcpyIArchive{Utils::make_span(buf)}; - OpVec out1, out2; - ia >> out1; - ia >> out2; + BOOST_CHECK_EQUAL(0, ar.bytes_processed()); + ar.skip(5); + BOOST_CHECK_EQUAL(5, ar.bytes_processed()); +} - BOOST_CHECK_EQUAL(ia.bytes_read(), buf.size()); +BOOST_AUTO_TEST_CASE(memcpy_processing) { + std::vector buf(10); - BOOST_CHECK(out1 == active); - BOOST_CHECK(out2 == inactive); + auto const test_number = 5; + auto oa = Utils::MemcpyOArchive(Utils::make_span(buf)); + oa << test_number; + BOOST_CHECK_EQUAL(oa.bytes_written(), sizeof(test_number)); + + { + auto ia = Utils::MemcpyIArchive(Utils::make_span(buf)); + int out; + ia >> out; + BOOST_CHECK_EQUAL(out, test_number); + BOOST_CHECK_EQUAL(ia.bytes_read(), sizeof(test_number)); } } -BOOST_AUTO_TEST_CASE(non_trivial_packing) { - static_assert(not std::is_trivially_copyable::value, ""); - - std::vector buf(2 * Utils::MemcpyOArchive::packing_size()); - - auto const active = NonTrivial{Utils::Vector3d{1., 2., 3.}}; - auto const inactive = NonTrivial{boost::none}; +BOOST_AUTO_TEST_CASE(serializaton_processing) { + std::vector buf(2 * Utils::MemcpyOArchive::packing_size()); + const OpVec active = Utils::Vector3d{1., 2., 3.}; + const OpVec inactive = boost::none; { auto oa = Utils::MemcpyOArchive{Utils::make_span(buf)}; auto in1 = active; @@ -97,13 +111,13 @@ BOOST_AUTO_TEST_CASE(non_trivial_packing) { { auto ia = Utils::MemcpyIArchive{Utils::make_span(buf)}; - NonTrivial out1, out2; + OpVec out1, out2; ia >> out1; ia >> out2; BOOST_CHECK_EQUAL(ia.bytes_read(), buf.size()); - BOOST_CHECK(out1.ov == active.ov); - BOOST_CHECK(out2.ov == inactive.ov); + BOOST_CHECK(out1 == active); + BOOST_CHECK(out2 == inactive); } -} \ No newline at end of file +} From 9aec36531e5f3639ccf88d72f2d10dcbcfc9bbbe Mon Sep 17 00:00:00 2001 From: Florian Weik Date: Mon, 4 Nov 2019 10:52:57 +0100 Subject: [PATCH 08/11] core: Cleanup memcpy_archive test --- src/core/ghosts.cpp | 71 +++++-------------- .../utils/serialization/memcpy_archive.hpp | 4 +- src/utils/tests/memcpy_archive_test.cpp | 17 +++-- 3 files changed, 26 insertions(+), 66 deletions(-) diff --git a/src/core/ghosts.cpp b/src/core/ghosts.cpp index aa2472179a5..f27f1ce222a 100644 --- a/src/core/ghosts.cpp +++ b/src/core/ghosts.cpp @@ -38,6 +38,7 @@ #include #include +#include #include #include @@ -50,10 +51,6 @@ /** Tag for ghosts communications. */ #define REQ_GHOST_SEND 100 -template <> -struct Utils::is_statically_serializable : std::true_type { -}; - /** * Class that stores marshalled data for ghost communications. * To store and retrieve data, use the adapter classes below. @@ -83,42 +80,6 @@ class CommBuf { std::vector bondbuf; //< Buffer for bond lists }; -/** - * Adapter to CommBuf that allows putting in and getting back bond data. - * Note, this class inserts the data handed to operator<< at the end - * of the underlying buffer. It does resize the buffer. - */ -struct BondArchiver { -private: - /* underlying buffer type */ - using buffer_type = std::vector; - buffer_type &bondbuf; - - /* iterator into the underlying buffer */ - using const_iterator = buffer_type::const_iterator; - const_iterator bond_retrieve; - - // Need to save these because send buffer will invalidate cb.bondbuffer. - const const_iterator initial_begin; - const size_t initial_size; - -public: - explicit BondArchiver(buffer_type &bondbuf) - : bondbuf(bondbuf), bond_retrieve(bondbuf.cbegin()), - initial_begin(bondbuf.cbegin()), initial_size(bondbuf.size()) {} - - ~BondArchiver() { assert(bond_retrieve == initial_begin + initial_size); } - - template inline void operator<<(const Utils::Span data) { - bondbuf.insert(bondbuf.end(), data.cbegin(), data.cend()); - } - - template inline void operator>>(Utils::Span data) { - std::copy_n(bond_retrieve, data.size(), data.begin()); - bond_retrieve += data.size(); - } -}; - /** whether the ghosts should have velocity information, e.g. for DPD or RATTLE. * You need this whenever you need the relative velocity of two particles. * NO CHANGES OF THIS VALUE OUTSIDE OF \ref on_ghost_flags_change !!!! @@ -145,28 +106,27 @@ static size_t calc_transmit_size(unsigned data_parts) { size += Utils::MemcpyOArchive::packing_size(); } if (data_parts & GHOSTTRANS_BONDS) { - size += sizeof(int); + size += Utils::MemcpyOArchive::packing_size(); } if (data_parts & GHOSTTRANS_POSITION) - size += sizeof(ParticlePosition); + size += Utils::MemcpyOArchive::packing_size(); if (data_parts & GHOSTTRANS_MOMENTUM) - size += sizeof(ParticleMomentum); + size += Utils::MemcpyOArchive::packing_size(); if (data_parts & GHOSTTRANS_FORCE) - size += sizeof(ParticleForce); + size += Utils::MemcpyOArchive::packing_size(); return size; } static size_t calc_transmit_size(GhostCommunication &ghost_comm, - unsigned int data_parts) { + unsigned int data_parts) { if (data_parts & GHOSTTRANS_PARTNUM) return sizeof(int) * ghost_comm.part_lists.size(); - else { - auto const n_part = boost::accumulate( - ghost_comm.part_lists, 0ul, - [](size_t sum, auto part_list) { return sum + part_list->n; }); - return n_part * calc_transmit_size(data_parts); - } + + auto const n_part = boost::accumulate( + ghost_comm.part_lists, 0ul, + [](size_t sum, auto part_list) { return sum + part_list->n; }); + return n_part * calc_transmit_size(data_parts); } static void prepare_send_buffer(CommBuf &send_buffer, @@ -177,7 +137,7 @@ static void prepare_send_buffer(CommBuf &send_buffer, send_buffer.bonds().clear(); auto archiver = Utils::MemcpyOArchive{Utils::make_span(send_buffer)}; - auto bond_archiver = BondArchiver{send_buffer.bonds()}; + auto bond_buffer = std::back_inserter(send_buffer.bonds()); /* put in data */ for (auto part_list : ghost_comm.part_lists) { @@ -203,7 +163,7 @@ static void prepare_send_buffer(CommBuf &send_buffer, } if (data_parts & GHOSTTRANS_BONDS) { archiver << part.bl.n; - bond_archiver << Utils::make_const_span(part.bl); + boost::copy(part.bl, bond_buffer); } } } @@ -251,7 +211,7 @@ static void put_recv_buffer(CommBuf &recv_buffer, unsigned int data_parts) { /* put back data */ auto archiver = Utils::MemcpyIArchive{Utils::make_span(recv_buffer)}; - auto bond_archiver = BondArchiver{recv_buffer.bonds()}; + auto bond_buffer = recv_buffer.bonds().begin(); for (auto part_list : ghost_comm.part_lists) { if (data_parts & GHOSTTRANS_PARTNUM) { @@ -279,7 +239,8 @@ static void put_recv_buffer(CommBuf &recv_buffer, decltype(part.bl.n) n_bonds; archiver >> n_bonds; part.bl.resize(n_bonds); - bond_archiver >> Utils::make_span(part.bl); + std::copy_n(bond_buffer, n_bonds, part.bl.begin()); + bond_buffer += n_bonds; } } } diff --git a/src/utils/include/utils/serialization/memcpy_archive.hpp b/src/utils/include/utils/serialization/memcpy_archive.hpp index ded276cdd99..61e452d6d1b 100644 --- a/src/utils/include/utils/serialization/memcpy_archive.hpp +++ b/src/utils/include/utils/serialization/memcpy_archive.hpp @@ -143,8 +143,8 @@ template class BasicMemcpyArchive { * Can only process types that have a static serialization size, * e.g. that serialize to the same number of bytes independent of * the state of the object. This can either be automatically detected - * for types that are trivially copyable, or by explicitly assuring - * this by specializing @c is_statically_serializable to std::true_type. + * for types that are trivially copyable, or by explicitly assured + * by specializing @c is_statically_serializable to std::true_type. */ class MemcpyIArchive : public detail::BasicMemcpyArchive { private: diff --git a/src/utils/tests/memcpy_archive_test.cpp b/src/utils/tests/memcpy_archive_test.cpp index 713b12f34ea..2ecacb3f807 100644 --- a/src/utils/tests/memcpy_archive_test.cpp +++ b/src/utils/tests/memcpy_archive_test.cpp @@ -28,9 +28,8 @@ #include #include -#include -#include +#include struct NonTrivial { boost::optional ov; @@ -46,10 +45,6 @@ template <> struct is_statically_serializable : std::true_type {}; template struct is_statically_serializable> : is_statically_serializable {}; - -template -struct is_statically_serializable> - : Utils::conjunction...> {}; } // namespace Utils BOOST_AUTO_TEST_CASE(packing_size_test) { @@ -62,13 +57,17 @@ BOOST_AUTO_TEST_CASE(packing_size_test) { } BOOST_AUTO_TEST_CASE(type_traits) { + static_assert(Utils::is_statically_serializable::value, ""); + static_assert(Utils::detail::use_memcpy::value, ""); + static_assert(not Utils::detail::use_serialize::value, ""); + static_assert(Utils::is_statically_serializable::value, ""); static_assert(not Utils::detail::use_memcpy::value, ""); static_assert(Utils::detail::use_serialize::value, ""); } BOOST_AUTO_TEST_CASE(skiping_and_position) { - std::vector buf(10); + std::array buf; auto ar = Utils::MemcpyOArchive(Utils::make_span(buf)); @@ -78,7 +77,7 @@ BOOST_AUTO_TEST_CASE(skiping_and_position) { } BOOST_AUTO_TEST_CASE(memcpy_processing) { - std::vector buf(10); + std::array buf; auto const test_number = 5; auto oa = Utils::MemcpyOArchive(Utils::make_span(buf)); @@ -95,7 +94,7 @@ BOOST_AUTO_TEST_CASE(memcpy_processing) { } BOOST_AUTO_TEST_CASE(serializaton_processing) { - std::vector buf(2 * Utils::MemcpyOArchive::packing_size()); + std::array buf; const OpVec active = Utils::Vector3d{1., 2., 3.}; const OpVec inactive = boost::none; From 0684d17d6b73e648e572fdba191f3c4fe8e1b29b Mon Sep 17 00:00:00 2001 From: Steffen Hirschmann Date: Mon, 4 Nov 2019 14:27:47 +0100 Subject: [PATCH 09/11] Use && instead of test -a POSIX does not specify test for > 4 arguments: https://pubs.opengroup.org/onlinepubs/9699919799/utilities/test.html --- src/python/pypresso.cmakein | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/python/pypresso.cmakein b/src/python/pypresso.cmakein index 8476da0fa64..7ebc19294b6 100755 --- a/src/python/pypresso.cmakein +++ b/src/python/pypresso.cmakein @@ -14,13 +14,13 @@ else fi export PYTHONPATH -if [ "@CMAKE_CXX_COMPILER_ID@" != "GNU" -a "@WITH_ASAN@" = "ON" ]; then +if [ "@CMAKE_CXX_COMPILER_ID@" != "GNU" ] && [ "@WITH_ASAN@" = "ON" ]; then asan_lib=$("@CMAKE_CXX_COMPILER@" /dev/null -### -o /dev/null -fsanitize=address 2>&1 | grep -o '[" ][^" ]*libclang_rt.asan[^" ]*[^s][" ]' | sed 's/[" ]//g' | sed 's/\.a$/.so/g') for lib in $asan_lib; do test -f $lib && LD_PRELOAD="$lib $LD_PRELOAD" done fi -if [ "@CMAKE_CXX_COMPILER_ID@" != "GNU" -a "@WITH_UBSAN@" = "ON" -a "@WITH_ASAN@" != "ON" ]; then +if [ "@CMAKE_CXX_COMPILER_ID@" != "GNU" ] && [ "@WITH_UBSAN@" = "ON" ] && [ "@WITH_ASAN@" != "ON" ]; then ubsan_lib=$("@CMAKE_CXX_COMPILER@" /dev/null -### -o /dev/null -fsanitize=undefined 2>&1 | grep -o '[" ][^" ]*libclang_rt.ubsan[^" ]*[^s][" ]' | sed 's/[" ]//g' | sed 's/\.a$/.so/g') for lib in $ubsan_lib; do test -f $lib && LD_PRELOAD="$lib $LD_PRELOAD" @@ -47,7 +47,7 @@ if [ "@WITH_ASAN@" = "ON" ]; then fi export ASAN_OPTIONS fi -if [ "@WITH_MSAN@" = "ON" -a "@WARNINGS_ARE_ERRORS@" = "ON" ]; then +if [ "@WITH_MSAN@" = "ON" ] && [ "@WARNINGS_ARE_ERRORS@" = "ON" ]; then export MSAN_OPTIONS="halt_on_error=1 $MSAN_OPTIONS" fi From 3574e203e72ae3a5c62a4a21608c5b85f1b3c9b5 Mon Sep 17 00:00:00 2001 From: Steffen Hirschmann Date: Mon, 4 Nov 2019 14:29:55 +0100 Subject: [PATCH 10/11] Avoid re-splitting of arguments --- src/python/pypresso.cmakein | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/python/pypresso.cmakein b/src/python/pypresso.cmakein index 7ebc19294b6..019af45c477 100755 --- a/src/python/pypresso.cmakein +++ b/src/python/pypresso.cmakein @@ -54,50 +54,50 @@ fi case $1 in --gdb) shift - [ "@PYTHON_FRONTEND@" = "@IPYTHON_EXECUTABLE@" ] && exec gdb -ex "set print thread-events off" -ex "set exec-wrapper sh -c 'exec \"@IPYTHON_EXECUTABLE@\" \"\$@\"'" --args "@PYTHON_EXECUTABLE@" $@ - exec gdb --args @PYTHON_FRONTEND@ $@ + [ "@PYTHON_FRONTEND@" = "@IPYTHON_EXECUTABLE@" ] && exec gdb -ex "set print thread-events off" -ex "set exec-wrapper sh -c 'exec \"@IPYTHON_EXECUTABLE@\" \"\$@\"'" --args "@PYTHON_EXECUTABLE@" "$@" + exec gdb --args @PYTHON_FRONTEND@ "$@" ;; --lldb) shift - exec lldb -- @PYTHON_FRONTEND@ $@ + exec lldb -- @PYTHON_FRONTEND@ "$@" ;; --valgrind) shift - exec valgrind --leak-check=full @PYTHON_FRONTEND@ $@ + exec valgrind --leak-check=full @PYTHON_FRONTEND@ "$@" ;; --cuda-gdb) shift - exec cuda-gdb --args @PYTHON_FRONTEND@ $@ + exec cuda-gdb --args @PYTHON_FRONTEND@ "$@" ;; --cuda-memcheck) shift - exec cuda-memcheck @PYTHON_FRONTEND@ $@ + exec cuda-memcheck @PYTHON_FRONTEND@ "$@" ;; --gdb=*) options="${1#*=}" shift - [ "@PYTHON_FRONTEND@" = "@IPYTHON_EXECUTABLE@" ] && exec gdb -ex "set print thread-events off" -ex "set exec-wrapper sh -c 'exec \"@IPYTHON_EXECUTABLE@\" \"\$@\"'" ${options} --args "@PYTHON_EXECUTABLE@" $@ - exec gdb ${options} --args @PYTHON_FRONTEND@ $@ + [ "@PYTHON_FRONTEND@" = "@IPYTHON_EXECUTABLE@" ] && exec gdb -ex "set print thread-events off" -ex "set exec-wrapper sh -c 'exec \"@IPYTHON_EXECUTABLE@\" \"\$@\"'" ${options} --args "@PYTHON_EXECUTABLE@" "$@" + exec gdb ${options} --args @PYTHON_FRONTEND@ "$@" ;; --lldb=*) options="${1#*=}" shift - exec lldb ${options} -- @PYTHON_FRONTEND@ $@ + exec lldb ${options} -- @PYTHON_FRONTEND@ "$@" ;; --valgrind=*) options="${1#*=}" shift - exec valgrind ${options} @PYTHON_FRONTEND@ $@ + exec valgrind ${options} @PYTHON_FRONTEND@ "$@" ;; --cuda-gdb=*) options="${1#*=}" shift - exec cuda-gdb ${options} --args @PYTHON_FRONTEND@ $@ + exec cuda-gdb ${options} --args @PYTHON_FRONTEND@ "$@" ;; --cuda-memcheck=*) options="${1#*=}" shift - exec cuda-memcheck ${options} @PYTHON_FRONTEND@ $@ + exec cuda-memcheck ${options} @PYTHON_FRONTEND@ "$@" ;; *) exec @PYTHON_FRONTEND@ "$@" From 8ee16865a808a15e6e829baacf249eb6aa26d9c9 Mon Sep 17 00:00:00 2001 From: Florian Weik Date: Mon, 4 Nov 2019 15:07:41 +0100 Subject: [PATCH 11/11] utils: memcpy_archive: Work around some odd types in boost.serialization --- .../utils/serialization/memcpy_archive.hpp | 27 ++++++++++++------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/src/utils/include/utils/serialization/memcpy_archive.hpp b/src/utils/include/utils/serialization/memcpy_archive.hpp index 61e452d6d1b..4a414bb9735 100644 --- a/src/utils/include/utils/serialization/memcpy_archive.hpp +++ b/src/utils/include/utils/serialization/memcpy_archive.hpp @@ -76,21 +76,30 @@ template class BasicMemcpyArchive { insert += bytes; } - template - auto operator>>(T &value) -> std::enable_if_t::value> { +private: + void read(void *data, size_t bytes) { /* check that there is enough space left in the buffer */ - assert((insert + sizeof(T)) <= buf.end()); + assert((insert + bytes) <= buf.end()); + std::memcpy(data, insert, bytes); + insert += bytes; + } - std::memcpy(&value, insert, sizeof(T)); - insert += sizeof(T); + void write(const void *data, size_t bytes) { + /* check that there is enough space left in the buffer */ + assert((insert + bytes) <= buf.end()); + std::memcpy(insert, data, bytes); + insert += bytes; + } + +public: + template + auto operator>>(T &value) -> std::enable_if_t::value> { + read(&value, sizeof(T)); } template auto operator<<(T const &value) -> std::enable_if_t::value> { - /* check that there is enough space left in the buffer */ - assert((insert + sizeof(T)) <= buf.end()); - std::memcpy(insert, &value, sizeof(T)); - insert += sizeof(T); + write(&value, sizeof(T)); } private: