Re: [tarantool-patches] Re: [PATCH v3 1/7] memtx: introduce universal iterator_pool

[Kirill Shcherbatov <kshcherbatov@tarantool.org>]

Memtx uses separate mempools for iterators of different types.
Due to the fact that there will be more iterators of different
sizes in a series of upcoming changes, let's always allocate the
iterator of the largest size.
No changes have been made to the rtree iterators pool because the
size of these structures is significantly larger.

Needed for #3961
---
 src/box/memtx_bitset.c | 15 +++++++--------
 src/box/memtx_engine.c |  9 +++------
 src/box/memtx_engine.h | 20 ++++++++++++++------
 src/box/memtx_hash.c   | 15 +++++++--------
 src/box/memtx_tree.c   | 13 ++++++-------
 5 files changed, 37 insertions(+), 35 deletions(-)

diff --git a/src/box/memtx_bitset.c b/src/box/memtx_bitset.c
index 12124c3f3..b6f8f7ac6 100644
--- a/src/box/memtx_bitset.c
+++ b/src/box/memtx_bitset.c
@@ -175,6 +175,10 @@ struct bitset_index_iterator {
 	struct mempool *pool;
 };
 
+static_assert(sizeof(struct bitset_index_iterator) <= MEMTX_ITERATOR_SIZE,
+	      "sizeof(struct bitset_index_iterator) must be less than or equal "
+	      "to MEMTX_ITERATOR_SIZE");
+
 static struct bitset_index_iterator *
 bitset_index_iterator(struct iterator *it)
 {
@@ -331,7 +335,7 @@ memtx_bitset_index_create_iterator(struct index *base, enum iterator_type type,
 	(void) part_count;
 
 	struct bitset_index_iterator *it;
-	it = mempool_alloc(&memtx->bitset_iterator_pool);
+	it = mempool_alloc(&memtx->iterator_pool);
 	if (!it) {
 		diag_set(OutOfMemory, sizeof(*it),
 			 "memtx_bitset_index", "iterator");
@@ -339,7 +343,7 @@ memtx_bitset_index_create_iterator(struct index *base, enum iterator_type type,
 	}
 
 	iterator_create(&it->base, base);
-	it->pool = &memtx->bitset_iterator_pool;
+	it->pool = &memtx->iterator_pool;
 	it->base.next = bitset_index_iterator_next;
 	it->base.free = bitset_index_iterator_free;
 
@@ -402,7 +406,7 @@ memtx_bitset_index_create_iterator(struct index *base, enum iterator_type type,
 	return (struct iterator *)it;
 fail:
 	tt_bitset_expr_destroy(&expr);
-	mempool_free(&memtx->bitset_iterator_pool, it);
+	mempool_free(&memtx->iterator_pool, it);
 	return NULL;
 }
 
@@ -506,11 +510,6 @@ memtx_bitset_index_new(struct memtx_engine *memtx, struct index_def *def)
 	assert(def->iid > 0);
 	assert(!def->opts.is_unique);
 
-	if (!mempool_is_initialized(&memtx->bitset_iterator_pool)) {
-		mempool_create(&memtx->bitset_iterator_pool, cord_slab_cache(),
-			       sizeof(struct bitset_index_iterator));
-	}
-
 	struct memtx_bitset_index *index =
 		(struct memtx_bitset_index *)calloc(1, sizeof(*index));
 	if (index == NULL) {
diff --git a/src/box/memtx_engine.c b/src/box/memtx_engine.c
index 64f43456e..b8638680b 100644
--- a/src/box/memtx_engine.c
+++ b/src/box/memtx_engine.c
@@ -176,14 +176,9 @@ static void
 memtx_engine_shutdown(struct engine *engine)
 {
 	struct memtx_engine *memtx = (struct memtx_engine *)engine;
-	if (mempool_is_initialized(&memtx->tree_iterator_pool))
-		mempool_destroy(&memtx->tree_iterator_pool);
+	mempool_destroy(&memtx->iterator_pool);
 	if (mempool_is_initialized(&memtx->rtree_iterator_pool))
 		mempool_destroy(&memtx->rtree_iterator_pool);
-	if (mempool_is_initialized(&memtx->hash_iterator_pool))
-		mempool_destroy(&memtx->hash_iterator_pool);
-	if (mempool_is_initialized(&memtx->bitset_iterator_pool))
-		mempool_destroy(&memtx->bitset_iterator_pool);
 	mempool_destroy(&memtx->index_extent_pool);
 	slab_cache_destroy(&memtx->index_slab_cache);
 	small_alloc_destroy(&memtx->alloc);
@@ -1069,6 +1064,8 @@ memtx_engine_new(const char *snap_dirname, bool force_recovery,
 	slab_cache_create(&memtx->index_slab_cache, &memtx->arena);
 	mempool_create(&memtx->index_extent_pool, &memtx->index_slab_cache,
 		       MEMTX_EXTENT_SIZE);
+	mempool_create(&memtx->iterator_pool, cord_slab_cache(),
+		       MEMTX_ITERATOR_SIZE);
 	memtx->num_reserved_extents = 0;
 	memtx->reserved_extents = NULL;
 
diff --git a/src/box/memtx_engine.h b/src/box/memtx_engine.h
index 0f8e92ee4..8f4ce7cdd 100644
--- a/src/box/memtx_engine.h
+++ b/src/box/memtx_engine.h
@@ -87,6 +87,14 @@ enum memtx_recovery_state {
 /** Memtx extents pool, available to statistics. */
 extern struct mempool memtx_index_extent_pool;
 
+/**
+ * The size of the biggest memtx iterator. Used with
+ * mempool_create. This is the size of the block that will be
+ * allocated for each iterator (except rtree index iterator that
+ * is significantly bigger so has own pool).
+ */
+#define MEMTX_ITERATOR_SIZE (152)
+
 struct memtx_engine {
 	struct engine base;
 	/** Engine recovery state. */
@@ -129,14 +137,14 @@ struct memtx_engine {
 	size_t max_tuple_size;
 	/** Incremented with each next snapshot. */
 	uint32_t snapshot_version;
-	/** Memory pool for tree index iterator. */
-	struct mempool tree_iterator_pool;
 	/** Memory pool for rtree index iterator. */
 	struct mempool rtree_iterator_pool;
-	/** Memory pool for hash index iterator. */
-	struct mempool hash_iterator_pool;
-	/** Memory pool for bitset index iterator. */
-	struct mempool bitset_iterator_pool;
+	/**
+	 * Memory pool for all index iterators except rtree.
+	 * The latter is significantly larger so it has its
+	 * own memory pool.
+	 */
+	struct mempool iterator_pool;
 	/**
 	 * Garbage collection fiber. Used for asynchronous
 	 * destruction of dropped indexes.
diff --git a/src/box/memtx_hash.c b/src/box/memtx_hash.c
index 63bdff208..ff478bdca 100644
--- a/src/box/memtx_hash.c
+++ b/src/box/memtx_hash.c
@@ -88,6 +88,10 @@ struct hash_iterator {
 	struct mempool *pool;
 };
 
+static_assert(sizeof(struct hash_iterator) <= MEMTX_ITERATOR_SIZE,
+	      "sizeof(struct hash_iterator) must be less than or equal "
+	      "to MEMTX_ITERATOR_SIZE");
+
 static void
 hash_iterator_free(struct iterator *iterator)
 {
@@ -350,14 +354,14 @@ memtx_hash_index_create_iterator(struct index *base, enum iterator_type type,
 
 	assert(part_count == 0 || key != NULL);
 
-	struct hash_iterator *it = mempool_alloc(&memtx->hash_iterator_pool);
+	struct hash_iterator *it = mempool_alloc(&memtx->iterator_pool);
 	if (it == NULL) {
 		diag_set(OutOfMemory, sizeof(struct hash_iterator),
 			 "memtx_hash_index", "iterator");
 		return NULL;
 	}
 	iterator_create(&it->base, base);
-	it->pool = &memtx->hash_iterator_pool;
+	it->pool = &memtx->iterator_pool;
 	it->base.free = hash_iterator_free;
 	it->hash_table = &index->hash_table;
 	light_index_iterator_begin(it->hash_table, &it->iterator);
@@ -386,7 +390,7 @@ memtx_hash_index_create_iterator(struct index *base, enum iterator_type type,
 	default:
 		diag_set(UnsupportedIndexFeature, base->def,
 			 "requested iterator type");
-		mempool_free(&memtx->hash_iterator_pool, it);
+		mempool_free(&memtx->iterator_pool, it);
 		return NULL;
 	}
 	return (struct iterator *)it;
@@ -489,11 +493,6 @@ static const struct index_vtab memtx_hash_index_vtab = {
 struct index *
 memtx_hash_index_new(struct memtx_engine *memtx, struct index_def *def)
 {
-	if (!mempool_is_initialized(&memtx->hash_iterator_pool)) {
-		mempool_create(&memtx->hash_iterator_pool, cord_slab_cache(),
-			       sizeof(struct hash_iterator));
-	}
-
 	struct memtx_hash_index *index =
 		(struct memtx_hash_index *)calloc(1, sizeof(*index));
 	if (index == NULL) {
diff --git a/src/box/memtx_tree.c b/src/box/memtx_tree.c
index 2e8adb682..250df7f2d 100644
--- a/src/box/memtx_tree.c
+++ b/src/box/memtx_tree.c
@@ -119,6 +119,10 @@ struct tree_iterator {
 	struct mempool *pool;
 };
 
+static_assert(sizeof(struct tree_iterator) <= MEMTX_ITERATOR_SIZE,
+	      "sizeof(struct tree_iterator) must be less than or equal "
+	      "to MEMTX_ITERATOR_SIZE");
+
 static void
 tree_iterator_free(struct iterator *iterator);
 
@@ -560,14 +564,14 @@ memtx_tree_index_create_iterator(struct index *base, enum iterator_type type,
 		key = NULL;
 	}
 
-	struct tree_iterator *it = mempool_alloc(&memtx->tree_iterator_pool);
+	struct tree_iterator *it = mempool_alloc(&memtx->iterator_pool);
 	if (it == NULL) {
 		diag_set(OutOfMemory, sizeof(struct tree_iterator),
 			 "memtx_tree_index", "iterator");
 		return NULL;
 	}
 	iterator_create(&it->base, base);
-	it->pool = &memtx->tree_iterator_pool;
+	it->pool = &memtx->iterator_pool;
 	it->base.next = tree_iterator_start;
 	it->base.free = tree_iterator_free;
 	it->type = type;
@@ -744,11 +748,6 @@ static const struct index_vtab memtx_tree_index_vtab = {
 struct index *
 memtx_tree_index_new(struct memtx_engine *memtx, struct index_def *def)
 {
-	if (!mempool_is_initialized(&memtx->tree_iterator_pool)) {
-		mempool_create(&memtx->tree_iterator_pool, cord_slab_cache(),
-			       sizeof(struct tree_iterator));
-	}
-
 	struct memtx_tree_index *index =
 		(struct memtx_tree_index *)calloc(1, sizeof(*index));
 	if (index == NULL) {
-- 
2.20.1