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

Author: lukaszstolarczuk

examples/concurrent_hash_map/concurrent_hash_map.cpp

@@ -86,6 +86,10 @@ main(int argc, char *argv[])
 		auto &map = *r;
 		std::cout << map.size() << std::endl;
 
+		/* 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;
@@ -2879,6 +2860,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

@@ -530,6 +530,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
+
+	if (NOT WIN32)
+		build_test(concurrent_hash_map_reserve_mock concurrent_hash_map_reserve/concurrent_hash_map_reserve_mock.cpp common/mock_tx_alloc.cc)
+		target_link_libraries(concurrent_hash_map_reserve_mock ${CMAKE_DL_LIBS})
+		add_test_generic(NAME concurrent_hash_map_reserve_mock TRACERS none)
+	endif()
+
 	# 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/common/mock_tx_alloc.cc

@@ -0,0 +1,41 @@
+// SPDX-License-Identifier: BSD-3-Clause
+/* Copyright 2017-2020, Intel Corporation */
+
+#include <cstdlib>
+#include <dlfcn.h>
+#include <errno.h>
+#include <libpmemobj/tx_base.h>
+
+#include "mock_tx_alloc.h"
+
+size_t test_alloc_counter;
+
+extern "C" PMEMoid pmemobj_tx_alloc(size_t size, uint64_t type_num);
+
+PMEMoid
+pmemobj_tx_alloc(size_t size, uint64_t type_num)
+{
+	static auto real = (decltype(pmemobj_tx_alloc) *)dlsym(
+		RTLD_NEXT, "pmemobj_tx_alloc");
+
+	if (real == nullptr)
+		abort();
+
+	test_alloc_counter++;
+
+	return real(size, type_num);
+}
+
+PMEMoid
+pmemobj_tx_xalloc(size_t size, uint64_t type_num, uint64_t flags)
+{
+	static auto real = (decltype(pmemobj_tx_xalloc) *)dlsym(
+		RTLD_NEXT, "pmemobj_tx_xalloc");
+
+	if (real == nullptr)
+		abort();
+
+	test_alloc_counter++;
+
+	return real(size, type_num, flags);
+}

tests/common/mock_tx_alloc.h

@@ -0,0 +1,8 @@
+// SPDX-License-Identifier: BSD-3-Clause
+/* Copyright 2017-2020, Intel Corporation */
+
+#include <cstddef>
+
+#pragma once
+
+extern size_t test_alloc_counter;

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,90 @@
+// SPDX-License-Identifier: BSD-3-Clause
+/* Copyright 2020, Intel Corporation */
+
+#include "../common/mock_tx_alloc.h"
+#include "../concurrent_hash_map/concurrent_hash_map_string_test.hpp"
+#include "unittest.hpp"
+
+#define LAYOUT "concurrent_hash_map"
+
+namespace nvobj = pmem::obj;
+
+/*
+ * reserve_insert -- reserve and insert with mock pmemobj_tx_[x]alloc
+ * pmem::obj::concurrent_hash_map<pmem::obj::string, pmem::obj::string>
+ */
+void
+reserve_insert(nvobj::pool<root> &pop)
+{
+	const size_t RESERVE_COUNT = 1000000;
+
+	auto map = pop.root()->cons;
+	UT_ASSERT(map != nullptr);
+
+	ConcurrentHashMapTestPrimitives<root, persistent_map_type> test(
+		pop, pop.root()->cons, 0);
+
+	UT_ASSERT(map->bucket_count() < RESERVE_COUNT);
+	UT_ASSERTeq(map->size(), 0);
+
+	/* insert and check allocations' count */
+	test_alloc_counter = 0;
+	for (size_t i = 0; i < RESERVE_COUNT; i++) {
+		bool ret = test.insert_or_assign(std::to_string(i), i);
+		UT_ASSERT(ret);
+	}
+	auto test_alloc_counter_first = test_alloc_counter;
+	UT_ASSERTeq(map->size(), RESERVE_COUNT);
+
+	/* cleanup */
+	test.clear();
+	UT_ASSERT(map->bucket_count() < RESERVE_COUNT);
+
+	/* insert again (with reserve) and check new allocations' count */
+	test_alloc_counter = 0;
+	map->reserve(RESERVE_COUNT);
+	UT_ASSERTeq(map->size(), 0);
+	UT_ASSERT(map->bucket_count() >= RESERVE_COUNT);
+
+	for (size_t i = 0; i < RESERVE_COUNT; i++) {
+		bool ret = test.insert_or_assign(std::to_string(i), i);
+		UT_ASSERT(ret);
+	}
+	UT_ASSERT(test_alloc_counter <= test_alloc_counter_first);
+	UT_ASSERTeq(map->size(), RESERVE_COUNT);
+
+	test.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); });
+}