Merge pull request #1156 from lukaszstolarczuk/ext-radix

Extend radix_tree tests and disable operator-- (for MtMode)

Author: igchor

include/libpmemobj++/experimental/radix_tree.hpp

@@ -98,11 +98,11 @@ struct bytes_view;
  * swap() invalidates all references and iterators.
  *
  * MtMode enables single-writer multiple-readers concurrency with read
- * uncommitted isolation. In this, mode user HAS to call runtime_initialize_mt
+ * uncommitted isolation. In this, mode user HAS TO call runtime_initialize_mt
  * after each application restart and runtime_finalize_mt before destroying
  * radix tree.
  *
- * This has the following effects:
+ * Enabling MtMode has the following effects:
  * - erase and clear does not free nodes/leaves immediately, instead they are
  * added to a garbage list which can be freed by calling garbage_collect()
  * - insert_or_assign and iterator.assign_val do not perform an in-place update,
@@ -617,7 +617,9 @@ struct radix_tree<Key, Value, BytesView, MtMode>::radix_tree_iterator {
 						    value_type &>::type;
 	using pointer = typename std::conditional<IsConst, value_type const *,
 						  value_type *>::type;
-	using iterator_category = std::bidirectional_iterator_tag;
+	using iterator_category = typename std::conditional<
+		MtMode, std::forward_iterator_tag,
+		std::bidirectional_iterator_tag>::type;
 
 	radix_tree_iterator() = default;
 	radix_tree_iterator(leaf_ptr leaf_, tree_ptr tree);
@@ -646,9 +648,13 @@ struct radix_tree<Key, Value, BytesView, MtMode>::radix_tree_iterator {
 	void assign_val(T &&rhs);
 
 	radix_tree_iterator &operator++();
+	template <bool Mt = MtMode,
+		  typename Enable = typename std::enable_if<!Mt>::type>
 	radix_tree_iterator &operator--();
 
 	radix_tree_iterator operator++(int);
+	template <bool Mt = MtMode,
+		  typename Enable = typename std::enable_if<!Mt>::type>
 	radix_tree_iterator operator--(int);
 
 	template <bool C>
@@ -3200,8 +3206,15 @@ radix_tree<Key, Value, BytesView,
 	return true;
 }
 
+/*
+ * If MtMode == true it's not safe to use this operator (iterator may end up
+ * invalid if concurrent erase happen).
+ * XXX: it's not enabled in MtMode due to:
+ * https://github.com/pmem/libpmemobj-cpp/issues/1159
+ */
 template <typename Key, typename Value, typename BytesView, bool MtMode>
 template <bool IsConst>
+template <bool Mt, typename Enable>
 typename radix_tree<Key, Value, BytesView,
 		    MtMode>::template radix_tree_iterator<IsConst> &
 radix_tree<Key, Value, BytesView,
@@ -3278,8 +3291,15 @@ radix_tree<Key, Value, BytesView,
 	return tmp;
 }
 
+/*
+ * If MtMode == true it's not safe to use this operator (iterator may end up
+ * invalid if concurrent erase happen).
+ * XXX: it's not enabled in MtMode due to:
+ * https://github.com/pmem/libpmemobj-cpp/issues/1159
+ */
 template <typename Key, typename Value, typename BytesView, bool MtMode>
 template <bool IsConst>
+template <bool Mt, typename Enable>
 typename radix_tree<Key, Value, BytesView,
 		    MtMode>::template radix_tree_iterator<IsConst>
 radix_tree<Key, Value, BytesView,

tests/radix_tree/radix.hpp

@@ -1,91 +1,91 @@
 // SPDX-License-Identifier: BSD-3-Clause
 /* Copyright 2020-2021, Intel Corporation */
 
+#include <functional>
+#include <libpmemobj.h>
+#include <random>
+
 #include "transaction_helpers.hpp"
 #include "unittest.hpp"
 
 #include <libpmemobj++/experimental/inline_string.hpp>
 #include <libpmemobj++/experimental/radix_tree.hpp>
 
-#include <functional>
-
-#include <libpmemobj.h>
+static std::mt19937_64 generator;
 
 namespace nvobj = pmem::obj;
 namespace nvobjex = pmem::obj::experimental;
 
-using container_int =
+using cntr_int =
 	nvobjex::radix_tree<nvobjex::inline_string, nvobj::p<unsigned>>;
-using container_string =
+using cntr_string =
 	nvobjex::radix_tree<nvobjex::inline_string, nvobjex::inline_string>;
 
-using container_int_int =
-	nvobjex::radix_tree<unsigned, nvobj::p<unsigned>,
-			    nvobjex::bytes_view<unsigned>, false>;
-using container_int_string =
-	nvobjex::radix_tree<unsigned, nvobjex::inline_string>;
+using cntr_int_int = nvobjex::radix_tree<unsigned, nvobj::p<unsigned>,
+					 nvobjex::bytes_view<unsigned>, false>;
+using cntr_int_string = nvobjex::radix_tree<unsigned, nvobjex::inline_string>;
 
-using container_inline_s_wchart =
+using cntr_inline_s_wchart =
 	nvobjex::radix_tree<nvobjex::basic_inline_string<wchar_t>,
 			    nvobj::p<unsigned>>;
-using container_inline_s_wchart_wchart =
+using cntr_inline_s_wchart_wchart =
 	nvobjex::radix_tree<nvobjex::basic_inline_string<wchar_t>,
 			    nvobjex::basic_inline_string<wchar_t>>;
-using container_inline_s_u8t =
+using cntr_inline_s_u8t =
 	nvobjex::radix_tree<nvobjex::basic_inline_string<uint8_t>,
 			    nvobjex::basic_inline_string<uint8_t>>;
 
-using container_int_mt =
+using cntr_int_mt =
 	nvobjex::radix_tree<nvobjex::inline_string, nvobj::p<unsigned>,
 			    nvobjex::bytes_view<nvobjex::inline_string>, true>;
-using container_string_mt =
+using cntr_string_mt =
 	nvobjex::radix_tree<nvobjex::inline_string, nvobjex::inline_string,
 			    nvobjex::bytes_view<nvobjex::inline_string>, true>;
 
-using container_int_int_mt =
+using cntr_int_int_mt =
 	nvobjex::radix_tree<unsigned, nvobj::p<unsigned>,
 			    nvobjex::bytes_view<unsigned>, true>;
-using container_int_string_mt =
+using cntr_int_string_mt =
 	nvobjex::radix_tree<unsigned, nvobjex::inline_string,
 			    nvobjex::bytes_view<unsigned>, true>;
 
-using container_inline_s_wchart_mt = nvobjex::radix_tree<
+using cntr_inline_s_wchart_mt = nvobjex::radix_tree<
 	nvobjex::basic_inline_string<wchar_t>, nvobj::p<unsigned>,
 	nvobjex::bytes_view<nvobjex::basic_inline_string<wchar_t>>, true>;
-using container_inline_s_wchart_wchart_mt = nvobjex::radix_tree<
+using cntr_inline_s_wchart_wchart_mt = nvobjex::radix_tree<
 	nvobjex::basic_inline_string<wchar_t>,
 	nvobjex::basic_inline_string<wchar_t>,
 	nvobjex::bytes_view<nvobjex::basic_inline_string<wchar_t>>, true>;
-using container_inline_s_u8t_mt = nvobjex::radix_tree<
+using cntr_inline_s_u8t_mt = nvobjex::radix_tree<
 	nvobjex::basic_inline_string<uint8_t>,
 	nvobjex::basic_inline_string<uint8_t>,
 	nvobjex::bytes_view<nvobjex::basic_inline_string<uint8_t>>, true>;
 
 struct root {
-	nvobj::persistent_ptr<container_int> radix_int;
-	nvobj::persistent_ptr<container_string> radix_str;
+	nvobj::persistent_ptr<cntr_int> radix_int;
+	nvobj::persistent_ptr<cntr_string> radix_str;
 
-	nvobj::persistent_ptr<container_int_int> radix_int_int;
-	nvobj::persistent_ptr<container_int_string> radix_int_str;
+	nvobj::persistent_ptr<cntr_int_int> radix_int_int;
+	nvobj::persistent_ptr<cntr_int_string> radix_int_str;
 
-	nvobj::persistent_ptr<container_inline_s_wchart> radix_inline_s_wchart;
-	nvobj::persistent_ptr<container_inline_s_wchart_wchart>
+	nvobj::persistent_ptr<cntr_inline_s_wchart> radix_inline_s_wchart;
+	nvobj::persistent_ptr<cntr_inline_s_wchart_wchart>
 		radix_inline_s_wchart_wchart;
-	nvobj::persistent_ptr<container_inline_s_u8t> radix_inline_s_u8t;
+	nvobj::persistent_ptr<cntr_inline_s_u8t> radix_inline_s_u8t;
 
-	nvobj::persistent_ptr<container_int_mt> radix_int_mt;
-	nvobj::persistent_ptr<container_string_mt> radix_str_mt;
+	nvobj::persistent_ptr<cntr_int_mt> radix_int_mt;
+	nvobj::persistent_ptr<cntr_string_mt> radix_str_mt;
 
-	nvobj::persistent_ptr<container_int_int_mt> radix_int_int_mt;
-	nvobj::persistent_ptr<container_int_string_mt> radix_int_str_mt;
+	nvobj::persistent_ptr<cntr_int_int_mt> radix_int_int_mt;
+	nvobj::persistent_ptr<cntr_int_string_mt> radix_int_str_mt;
 
-	nvobj::persistent_ptr<container_inline_s_wchart_mt>
-		radix_inline_s_wchart_mt;
-	nvobj::persistent_ptr<container_inline_s_wchart_wchart_mt>
+	nvobj::persistent_ptr<cntr_inline_s_wchart_mt> radix_inline_s_wchart_mt;
+	nvobj::persistent_ptr<cntr_inline_s_wchart_wchart_mt>
 		radix_inline_s_wchart_wchart_mt;
-	nvobj::persistent_ptr<container_inline_s_u8t_mt> radix_inline_s_u8t_mt;
+	nvobj::persistent_ptr<cntr_inline_s_u8t_mt> radix_inline_s_u8t_mt;
 };
 
+/* Helper functions to access key/value of different types */
 template <typename Container,
 	  typename Enable = typename std::enable_if<
 		  std::is_same<typename Container::mapped_type,
@@ -108,9 +108,9 @@ value(unsigned v, int repeats = 1)
 {
 	using CharT = typename Container::mapped_type::value_type;
 
+	auto str = std::to_string(v);
 	auto s = std::basic_string<CharT>{};
 	for (int i = 0; i < repeats; i++) {
-		auto str = std::to_string(v);
 		s += std::basic_string<CharT>(str.begin(), str.end());
 	}
 
@@ -178,6 +178,7 @@ operator!=(pmem::obj::experimental::basic_inline_string<CharT, Traits> &lhs,
 		       .compare(rhs) != 0;
 }
 
+/* verify all elements in container, using lower_/upper_bound and find */
 template <typename Container, typename K, typename F>
 void
 verify_elements(nvobj::persistent_ptr<Container> ptr, unsigned count, K &&key_f,
@@ -216,6 +217,8 @@ verify_elements(nvobj::persistent_ptr<Container> ptr, unsigned count, K &&key_f,
 	}
 }
 
+/* run 1 thread with modifications (erase/write/etc.) and multiple threads with
+ * reads */
 template <typename ModifyF, typename ReadF>
 static void
 parallel_modify_read(ModifyF modifier, std::vector<ReadF> &readers,
@@ -230,16 +233,30 @@ parallel_modify_read(ModifyF modifier, std::vector<ReadF> &readers,
 	});
 }
 
+/* each test suite should initialize generator at the beginning */
+void
+init_random()
+{
+	std::random_device rd;
+	auto seed = rd();
+	std::cout << "rand seed: " << seed << std::endl;
+	generator = std::mt19937_64(seed);
+}
+
 template <typename Container>
 static void
 init_container(nvobj::pool<root> &pop, nvobj::persistent_ptr<Container> &ptr,
-	       const size_t initial_elements, const size_t value_repeats = 1)
+	       const size_t initial_elements, const size_t value_repeats = 1,
+	       bool rand_keys = false)
 {
 	nvobj::transaction::run(
 		pop, [&] { ptr = nvobj::make_persistent<Container>(); });
 
 	for (size_t i = 0; i < initial_elements; ++i) {
-		ptr->emplace(key<Container>(i),
-			     value<Container>(i, value_repeats));
+		auto k = key<Container>(i);
+		if (rand_keys) {
+			k = key<Container>(static_cast<unsigned>(generator()));
+		}
+		ptr->emplace(k, value<Container>(i, value_repeats));
 	}
 }