From 2fd3027cea656d247b8c0ae9afbc923c153bb02b Mon Sep 17 00:00:00 2001 From: Keita Iwabuchi Date: Tue, 8 Nov 2022 10:07:12 -0800 Subject: [PATCH 1/8] Change not to propagate allocator. --- include/metall/stl_allocator.hpp | 6 +++--- test/container/stl_allocator_test.cpp | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/include/metall/stl_allocator.hpp b/include/metall/stl_allocator.hpp index 20f2fcf8..74dd6deb 100644 --- a/include/metall/stl_allocator.hpp +++ b/include/metall/stl_allocator.hpp @@ -35,9 +35,9 @@ class stl_allocator { using const_void_pointer = typename std::pointer_traits::template rebind; using difference_type = typename std::pointer_traits::difference_type; using size_type = typename std::make_unsigned::type; - using propagate_on_container_copy_assignment = std::true_type; - using propagate_on_container_move_assignment = std::true_type; - using propagate_on_container_swap = std::true_type; + using propagate_on_container_copy_assignment = std::false_type; + using propagate_on_container_move_assignment = std::false_type; + using propagate_on_container_swap = std::false_type; using is_always_equal = std::false_type; using manager_kernel_type = metall_manager_kernel_type; diff --git a/test/container/stl_allocator_test.cpp b/test/container/stl_allocator_test.cpp index 7e4aaf76..f2cd6f39 100644 --- a/test/container/stl_allocator_test.cpp +++ b/test/container/stl_allocator_test.cpp @@ -53,16 +53,16 @@ TEST(StlAllocatorTest, Types) { GTEST_ASSERT_EQ(typeid(std::allocator_traits::propagate_on_container_copy_assignment), typeid(alloc_t::propagate_on_container_copy_assignment)); GTEST_ASSERT_EQ(typeid(std::allocator_traits::propagate_on_container_copy_assignment), - typeid(std::true_type)); + typeid(std::false_type)); GTEST_ASSERT_EQ(typeid(std::allocator_traits::propagate_on_container_move_assignment), typeid(alloc_t::propagate_on_container_move_assignment)); GTEST_ASSERT_EQ(typeid(std::allocator_traits::propagate_on_container_move_assignment), - typeid(std::true_type)); + typeid(std::false_type)); GTEST_ASSERT_EQ(typeid(std::allocator_traits::propagate_on_container_swap), typeid(alloc_t::propagate_on_container_swap)); - GTEST_ASSERT_EQ(typeid(std::allocator_traits::propagate_on_container_swap), typeid(std::true_type)); + GTEST_ASSERT_EQ(typeid(std::allocator_traits::propagate_on_container_swap), typeid(std::false_type)); GTEST_ASSERT_EQ(typeid(std::allocator_traits::is_always_equal), typeid(alloc_t::is_always_equal)); From d2c17bbae0ddf9a11e6d25256923a0c365b3eee2 Mon Sep 17 00:00:00 2001 From: Keita Iwabuchi Date: Thu, 10 Nov 2022 22:31:31 -0800 Subject: [PATCH 2/8] Call copy assignment explicitly. --- .../metall/container/experimental/json/value.hpp | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/include/metall/container/experimental/json/value.hpp b/include/metall/container/experimental/json/value.hpp index e4599b00..34724cc9 100644 --- a/include/metall/container/experimental/json/value.hpp +++ b/include/metall/container/experimental/json/value.hpp @@ -138,7 +138,20 @@ class value { std::true_type>) { m_allocator = other.m_allocator; } - m_data = other.m_data; + + // Cannot do `m_data = other.m_data` + // because std::variant calls the allocator-extended copy constructor of the holding data, + // which ignores propagate_on_container_copy_assignment value. + if (other.is_object()) { + emplace_object() = other.as_object(); + } else if (other.is_array()) { + emplace_array() = other.as_array(); + } else if (other.is_string()) { + emplace_string() = other.as_string(); + } else { + m_data = other.m_data; + } + return *this; } From 66ed25479d71ec2bc25eb0f45f7cc6d5391f2512 Mon Sep 17 00:00:00 2001 From: Keita Iwabuchi Date: Fri, 11 Nov 2022 14:51:38 -0800 Subject: [PATCH 3/8] Add warning comments about the new behavior --- include/metall/stl_allocator.hpp | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/include/metall/stl_allocator.hpp b/include/metall/stl_allocator.hpp index 74dd6deb..dca2fc51 100644 --- a/include/metall/stl_allocator.hpp +++ b/include/metall/stl_allocator.hpp @@ -17,9 +17,15 @@ namespace metall { -/// \brief A STL compatible allocator -/// \tparam T The type of the object -/// \tparam metall_manager_kernel_type The type of the manager kernel +/// \brief A STL compatible allocator. +/// \tparam T A object type. +/// \tparam metall_manager_kernel_type A manager kernel type. +/// \warning +/// This allocator is not propagated during containers' copy assignment, move assignment, or swap +/// (propagate_on_* types are std::false_type), as same as Boost.Interprocess. +/// Therefore, performing the move assignment between two objects allocated by different Metall managers invokes +/// copy operations instead of move operations. +/// Also, swapping objects allocated by different Metall managers will result in undefined behavior. template class stl_allocator { From a25de6421d6fbb8ec87a682b403b6bdd124055f0 Mon Sep 17 00:00:00 2001 From: Keita Iwabuchi Date: Fri, 11 Nov 2022 15:15:40 -0800 Subject: [PATCH 4/8] Use the default values in std::allocator_traits --- include/metall/stl_allocator.hpp | 22 +++++++--------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/include/metall/stl_allocator.hpp b/include/metall/stl_allocator.hpp index dca2fc51..c6b2db40 100644 --- a/include/metall/stl_allocator.hpp +++ b/include/metall/stl_allocator.hpp @@ -21,11 +21,13 @@ namespace metall { /// \tparam T A object type. /// \tparam metall_manager_kernel_type A manager kernel type. /// \warning -/// This allocator is not propagated during containers' copy assignment, move assignment, or swap -/// (propagate_on_* types are std::false_type), as same as Boost.Interprocess. -/// Therefore, performing the move assignment between two objects allocated by different Metall managers invokes -/// copy operations instead of move operations. -/// Also, swapping objects allocated by different Metall managers will result in undefined behavior. +/// This allocator does not define propagate_on_* types, as same as Boost.Interprocess. +/// Those types are going to be std::false_type in std::allocator_traits. +/// Therefore, this allocator is not propagated during containers' copy assignment, move assignment, or swap. +/// This configuration enables users to copy containers between different Metall managers easier. +/// On the other hand, performing the move assignment between two containers allocated by different Metall managers +/// invokes copy operations instead of move operations. +/// Also, swapping containers allocated by different Metall managers will result in undefined behavior. template class stl_allocator { @@ -41,10 +43,6 @@ class stl_allocator { using const_void_pointer = typename std::pointer_traits::template rebind; using difference_type = typename std::pointer_traits::difference_type; using size_type = typename std::make_unsigned::type; - using propagate_on_container_copy_assignment = std::false_type; - using propagate_on_container_move_assignment = std::false_type; - using propagate_on_container_swap = std::false_type; - using is_always_equal = std::false_type; using manager_kernel_type = metall_manager_kernel_type; /// \brief Makes another allocator type for type T2 @@ -139,12 +137,6 @@ class stl_allocator { void destroy(const pointer &ptr) const { priv_destroy(ptr); } - /// \brief Obtains the copy-constructed version of the allocator a. - /// \param a Allocator used by a standard container passed as an argument to a container copy constructor. - /// \return The allocator to use by the copy-constructed standard containers. - stl_allocator select_on_container_copy_construction(const stl_allocator &a) { - return stl_allocator(a); - } // ----------------------------------- This class's unique public functions ----------------------------------- // /// \brief Returns a pointer that points to manager kernel From 98bba458c72ab3f413c89294f5c27d74b28c5da4 Mon Sep 17 00:00:00 2001 From: Keita Iwabuchi Date: Fri, 11 Nov 2022 16:45:55 -0800 Subject: [PATCH 5/8] Adjust to the new model --- test/container/stl_allocator_test.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/container/stl_allocator_test.cpp b/test/container/stl_allocator_test.cpp index f2cd6f39..320e7344 100644 --- a/test/container/stl_allocator_test.cpp +++ b/test/container/stl_allocator_test.cpp @@ -51,21 +51,21 @@ TEST(StlAllocatorTest, Types) { GTEST_ASSERT_EQ(typeid(std::allocator_traits::size_type), typeid(alloc_t::size_type)); GTEST_ASSERT_EQ(typeid(std::allocator_traits::propagate_on_container_copy_assignment), - typeid(alloc_t::propagate_on_container_copy_assignment)); + typeid(std::false_type)); GTEST_ASSERT_EQ(typeid(std::allocator_traits::propagate_on_container_copy_assignment), typeid(std::false_type)); GTEST_ASSERT_EQ(typeid(std::allocator_traits::propagate_on_container_move_assignment), - typeid(alloc_t::propagate_on_container_move_assignment)); + typeid(std::false_type)); GTEST_ASSERT_EQ(typeid(std::allocator_traits::propagate_on_container_move_assignment), typeid(std::false_type)); GTEST_ASSERT_EQ(typeid(std::allocator_traits::propagate_on_container_swap), - typeid(alloc_t::propagate_on_container_swap)); + typeid(std::false_type)); GTEST_ASSERT_EQ(typeid(std::allocator_traits::propagate_on_container_swap), typeid(std::false_type)); GTEST_ASSERT_EQ(typeid(std::allocator_traits::is_always_equal), - typeid(alloc_t::is_always_equal)); + typeid(std::false_type)); GTEST_ASSERT_EQ(typeid(std::allocator_traits::is_always_equal), typeid(std::false_type)); using otherT = int; From 75a6f7296c4b2a43a42d0c253cd2545f6ca3c597 Mon Sep 17 00:00:00 2001 From: Keita Iwabuchi Date: Thu, 12 Jan 2023 17:01:29 -0800 Subject: [PATCH 6/8] Show error when copying non-consistent datastore --- include/metall/kernel/manager_kernel_impl.ipp | 7 +++++++ include/metall/utility/metall_mpi_adaptor.hpp | 10 ++++++++++ 2 files changed, 17 insertions(+) diff --git a/include/metall/kernel/manager_kernel_impl.ipp b/include/metall/kernel/manager_kernel_impl.ipp index f8d0957c..b5d28ec6 100644 --- a/include/metall/kernel/manager_kernel_impl.ipp +++ b/include/metall/kernel/manager_kernel_impl.ipp @@ -1173,6 +1173,13 @@ bool manager_kernel::priv_copy_data_store(const std::string &s const std::string &dst_base_dir_path, const bool use_clone, const int num_max_copy_threads) { + if (!consistent(src_base_dir_path.data())) { + std::string s( + "Source directory is not consistnt (may not have closed properly or may still be open): " + src_base_dir_path); + logger::out(logger::level::error, __FILE__, __LINE__, s.c_str()); + return false; + } + const std::string src_top_dir = priv_make_top_dir_path(src_base_dir_path); if (!mdtl::directory_exist(src_top_dir)) { std::string s("Source directory does not exist: " + src_top_dir); diff --git a/include/metall/utility/metall_mpi_adaptor.hpp b/include/metall/utility/metall_mpi_adaptor.hpp index a090ead3..6b84b63c 100644 --- a/include/metall/utility/metall_mpi_adaptor.hpp +++ b/include/metall/utility/metall_mpi_adaptor.hpp @@ -135,6 +135,7 @@ class metall_mpi_adaptor { } /// \brief Copies a Metall datastore to another location. + /// The behavior of copying a data store that is open without the read-only mode is undefined. /// \param source_dir_path A path to a source datastore. /// \param destination_dir_path A path to a destination datastore. /// \param comm A MPI communicator. @@ -143,6 +144,15 @@ class metall_mpi_adaptor { static bool copy(const char *source_dir_path, const char *destination_dir_path, const MPI_Comm &comm = MPI_COMM_WORLD) { + if (!consistent(source_dir_path, comm)) { + if (priv_mpi_comm_rank(comm) == 0) { + std::stringstream ss; + ss << "Source directory is not consistnt (may not have closed properly or may still be open): " + << source_dir_path; + logger::out(logger::level::error, __FILE__, __LINE__, ss.str().c_str()); + } + return false; + } priv_setup_root_dir(destination_dir_path, comm); const int rank = priv_mpi_comm_rank(comm); return priv_global_and(manager_type::copy(ds::make_local_dir_path(source_dir_path, rank).c_str(), From c56db7bdf32fafdbabbd601d20cc7a66b024c1ed Mon Sep 17 00:00:00 2001 From: Keita Iwabuchi Date: Tue, 21 Feb 2023 19:09:24 -0800 Subject: [PATCH 7/8] Bugfix in iterators in bench containers. --- bench/data_structure/multithread_adjacency_list.hpp | 4 ++-- .../data_structure/partitioned_multithread_adjacency_list.hpp | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/bench/data_structure/multithread_adjacency_list.hpp b/bench/data_structure/multithread_adjacency_list.hpp index 876bf92e..eed559fd 100644 --- a/bench/data_structure/multithread_adjacency_list.hpp +++ b/bench/data_structure/multithread_adjacency_list.hpp @@ -193,9 +193,9 @@ class multithread_adjacency_list<_key_type, _value_type, _base_allocator_type>:: return !(*this == other); } - reference operator++() { + impl_const_key_iterator& operator++() { next(); - return *m_local_iterator; + return *this; } impl_const_key_iterator operator++(int) { diff --git a/bench/data_structure/partitioned_multithread_adjacency_list.hpp b/bench/data_structure/partitioned_multithread_adjacency_list.hpp index c0b16fc1..091c9016 100644 --- a/bench/data_structure/partitioned_multithread_adjacency_list.hpp +++ b/bench/data_structure/partitioned_multithread_adjacency_list.hpp @@ -174,9 +174,9 @@ class partitioned_multithread_adjacency_list Date: Tue, 21 Feb 2023 19:31:47 -0800 Subject: [PATCH 8/8] Release v0.24 --- CMakeLists.txt | 2 +- docs/Doxyfile.in | 2 +- include/metall/version.hpp | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 60a90db7..e2295637 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -17,7 +17,7 @@ endif() # Metall general configuration # -------------------------------------------------------------------------------- # project(Metall - VERSION 0.23.1 + VERSION 0.24 DESCRIPTION "A persistent memory allocator for data-centric analytics" HOMEPAGE_URL "https://github.com/LLNL/metall") diff --git a/docs/Doxyfile.in b/docs/Doxyfile.in index eebd3653..389327da 100644 --- a/docs/Doxyfile.in +++ b/docs/Doxyfile.in @@ -38,7 +38,7 @@ PROJECT_NAME = "Metall" # could be handy for archiving the generated documentation or if some version # control system is used. -PROJECT_NUMBER = v0.23.1 +PROJECT_NUMBER = v0.24 # Using the PROJECT_BRIEF tag one can provide an optional one line description # for a project that appears at the top of each page and should give viewer a diff --git a/include/metall/version.hpp b/include/metall/version.hpp index 90bb19cd..66710a28 100644 --- a/include/metall/version.hpp +++ b/include/metall/version.hpp @@ -14,7 +14,7 @@ /// METALL_VERSION / 100 % 1000 // the minor version. /// METALL_VERSION % 100 // the patch level. /// \endcode -#define METALL_VERSION 2301 +#define METALL_VERSION 2400 namespace metall { /// \brief Variable type to handle a version data.