hashmap: move reserve() from base class to public method

Author: lukaszstolarczuk

examples/concurrent_hash_map/concurrent_hash_map.cpp

@@ -81,6 +81,10 @@ main(int argc, char *argv[])
 
 		auto &map = *r;
 
+		/* We expect around 10 * THREADS_NUM items, so we reserve
+		 * hashmap's capacity to speed up insert operations. */
+		map.reserve(10 * THREADS_NUM);
+
 		std::vector<std::thread> threads;
 		threads.reserve(static_cast<size_t>(THREADS_NUM));
 

include/libpmemobj++/container/concurrent_hash_map.hpp

@@ -1298,25 +1298,6 @@ class hash_map_base {
 		return false;
 	}
 
-	/**
-	 * Prepare enough segments for number of buckets
-	 */
-	void
-	reserve(size_type buckets)
-	{
-		if (buckets == 0)
-			return;
-
-		--buckets;
-
-		bool is_initial = this->size() == 0;
-
-		for (size_type m = mask(); buckets > m; m = mask())
-			enable_segment(
-				segment_traits_t::segment_index_of(m + 1),
-				is_initial);
-	}
-
 	/**
 	 * Swap hash_map_base
 	 * @throw std::transaction_error in case of PMDK transaction failed
@@ -1661,6 +1642,7 @@ class concurrent_hash_map
 	using hash_map_base::check_growth;
 	using hash_map_base::check_mask_race;
 	using hash_map_base::embedded_buckets;
+	using hash_map_base::enable_segment;
 	using hash_map_base::FEATURE_CONSISTENT_SIZE;
 	using hash_map_base::get_bucket;
 	using hash_map_base::get_pool_base;
@@ -1669,7 +1651,6 @@ class concurrent_hash_map
 	using hash_map_base::internal_swap;
 	using hash_map_base::layout_features;
 	using hash_map_base::mask;
-	using hash_map_base::reserve;
 	using tls_t = typename hash_map_base::tls_t;
 	using node = typename hash_map_base::node;
 	using node_mutex_t = typename node::mutex_t;
@@ -2874,6 +2855,27 @@ class concurrent_hash_map
 		return my_defrag.run();
 	}
 
+	/**
+	 * Prepare enough segments for number of buckets.
+	 *
+	 * XXX: fixme
+	 */
+	void
+	reserve(size_type buckets)
+	{
+		if (buckets == 0)
+			return;
+
+		--buckets;
+
+		bool is_initial = this->size() == 0;
+
+		for (size_type m = mask(); buckets > m; m = mask())
+			enable_segment(
+				segment_traits_t::segment_index_of(m + 1),
+				is_initial);
+	}
+
 	/**
 	 * Remove element with corresponding key
 	 *

tests/CMakeLists.txt

@@ -497,6 +497,15 @@ if(TEST_CONCURRENT_HASHMAP)
 	build_test(concurrent_hash_map_defrag concurrent_hash_map_defrag/concurrent_hash_map_defrag.cpp)
 	add_test_generic(NAME concurrent_hash_map_defrag TRACERS none)
 
+	build_test(concurrent_hash_map_reserve concurrent_hash_map_reserve/concurrent_hash_map_reserve.cpp)
+	add_test_generic(NAME concurrent_hash_map_reserve TRACERS none) # memcheck pmemcheck
+
+	build_test(concurrent_hash_map_reserve_mock concurrent_hash_map_reserve/concurrent_hash_map_reserve_mock.cpp)
+	if (NOT WIN32)
+		target_link_libraries(concurrent_hash_map_reserve_mock "-Wl,--wrap=reserve")
+	endif()
+	add_test_generic(NAME concurrent_hash_map_reserve_mock TRACERS none)
+
 	# This test can NOT be run under helgrind as it will report wrong lock ordering. Helgrind is right about
 	# possible deadlock situation but that could only happen in case of wrong API usage.
 	build_test(concurrent_hash_map_deadlock concurrent_hash_map_deadlock/concurrent_hash_map_deadlock.cpp)

tests/concurrent_hash_map_reserve/concurrent_hash_map_reserve.cpp

@@ -0,0 +1,65 @@
+// SPDX-License-Identifier: BSD-3-Clause
+/* Copyright 2020, Intel Corporation */
+
+#include "../concurrent_hash_map/concurrent_hash_map_string_test.hpp"
+#include "unittest.hpp"
+
+#define LAYOUT "concurrent_hash_map"
+
+namespace nvobj = pmem::obj;
+
+/*
+ * reserve -- basic reserve test of
+ * pmem::obj::concurrent_hash_map<pmem::obj::string, pmem::obj::string>
+ */
+void
+reserve_test(nvobj::pool<root> &pop)
+{
+	const size_t RESERVE_COUNT = 5000;
+	auto map = pop.root()->cons;
+
+	UT_ASSERT(map != nullptr);
+
+	map->runtime_initialize();
+
+	UT_ASSERT(map->bucket_count() < RESERVE_COUNT);
+	UT_ASSERTeq(map->size(), 0);
+	map->reserve(RESERVE_COUNT);
+	UT_ASSERTeq(map->size(), 0);
+	UT_ASSERT(map->bucket_count() >= RESERVE_COUNT);
+
+	map->clear();
+}
+
+static void
+test(int argc, char *argv[])
+{
+	if (argc < 1) {
+		UT_FATAL("usage: %s file-name", argv[0]);
+	}
+
+	const char *path = argv[1];
+	nvobj::pool<root> pop;
+
+	try {
+		pop = nvobj::pool<root>::create(path, LAYOUT,
+						200 * PMEMOBJ_MIN_POOL,
+						S_IWUSR | S_IRUSR);
+		pmem::obj::transaction::run(pop, [&] {
+			pop.root()->cons =
+				nvobj::make_persistent<persistent_map_type>();
+		});
+	} catch (pmem::pool_error &pe) {
+		UT_FATAL("!pool::create: %s %s", pe.what(), path);
+	}
+
+	reserve_test(pop);
+
+	pop.close();
+}
+
+int
+main(int argc, char *argv[])
+{
+	return run_test([&] { test(argc, argv); });
+}

tests/concurrent_hash_map_reserve/concurrent_hash_map_reserve_mock.cpp

@@ -0,0 +1,69 @@
+// SPDX-License-Identifier: BSD-3-Clause
+/* Copyright 2020, Intel Corporation */
+
+#include "../concurrent_hash_map/concurrent_hash_map_string_test.hpp"
+#include "tests/wrap_reserve.h"
+#include "unittest.hpp"
+
+#define LAYOUT "concurrent_hash_map"
+
+namespace nvobj = pmem::obj;
+
+/*
+ * reserve_insert -- mock reserve and insert
+ * pmem::obj::concurrent_hash_map<pmem::obj::string, pmem::obj::string>
+ */
+void
+reserve_insert(nvobj::pool<root> &pop)
+{
+	const size_t RESERVE_COUNT = 5000;
+	auto map = pop.root()->cons;
+
+	UT_ASSERT(map != nullptr);
+
+	map->runtime_initialize();
+
+	UT_ASSERT(map->bucket_count() < RESERVE_COUNT);
+	UT_ASSERTeq(map->size(), 0);
+	map->reserve(RESERVE_COUNT);
+	// UT_ASSERT(0);
+	UT_ASSERTeq(map->size(), 0);
+	UT_ASSERT(map->bucket_count() >= RESERVE_COUNT);
+
+	/* insert and check allocations */
+
+	map->clear();
+}
+
+static void
+test(int argc, char *argv[])
+{
+	if (argc < 1) {
+		UT_FATAL("usage: %s file-name", argv[0]);
+	}
+
+	const char *path = argv[1];
+	nvobj::pool<root> pop;
+
+	try {
+		pop = nvobj::pool<root>::create(path, LAYOUT,
+						200 * PMEMOBJ_MIN_POOL,
+						S_IWUSR | S_IRUSR);
+		pmem::obj::transaction::run(pop, [&] {
+			pop.root()->cons =
+				nvobj::make_persistent<persistent_map_type>();
+		});
+	} catch (pmem::pool_error &pe) {
+		UT_FATAL("!pool::create: %s %s", pe.what(), path);
+	}
+
+	reserve_insert(pop);
+
+	pop.close();
+}
+
+int
+main(int argc, char *argv[])
+{
+	return run_test([&] { test(argc, argv); });
+}

tests/wrap_reserve.h

@@ -0,0 +1,25 @@
+// SPDX-License-Identifier: BSD-3-Clause
+/* Copyright 2020, Intel Corporation */
+
+/*
+ * wrap_reserve.h -- mock function for hashmap reserve()
+ * used in the concurrent_hash_map tests which are checking...
+ * XXX: fixme
+ */
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/*
+ * __wrap_reserve -- mock function for hashmap reserve()
+ */
+void
+__wrap_reserve(size_t buckets)
+{
+	std::cerr << "debug!n";
+}
+
+#ifdef __cplusplus
+}
+#endif