From 1aae12acc34a338bb7f54ab6b1c7560d92300666 Mon Sep 17 00:00:00 2001
From: Vladimir Davydov <vdavydov@tarantool.org>
Date: Mon, 17 Oct 2022 17:40:24 +0300
Subject: [PATCH] index: make index_read_view_iterator fixed size

This is a straightforward patch that makes the read view iterator struct
fixed-size so that it can be allocated on stack. This is a pre-requisite
for the raw C API for read views: the function that creates an iterator
should allocate no memory.

Closes #7813

NO_DOC=refactoring
NO_TEST=refactoring
NO_CHANGELOG=refactoring
---
 src/box/index.h         | 52 +++++++++++++++++++++++++++--------------
 src/box/memtx_engine.cc | 22 ++++++++---------
 src/box/memtx_hash.cc   | 32 ++++++++++---------------
 src/box/memtx_tree.cc   | 38 +++++++++++++-----------------
 src/box/sequence.c      | 33 +++++++++++++-------------
 5 files changed, 89 insertions(+), 88 deletions(-)

diff --git a/src/box/index.h b/src/box/index.h
index 87c52c2f24..7819cf7eef 100644
--- a/src/box/index.h
+++ b/src/box/index.h
@@ -650,9 +650,10 @@ struct index_read_view_vtab {
 		       const char *key, uint32_t part_count,
 		       const char **data, uint32_t *size);
 	/** Create an index read view iterator. */
-	struct index_read_view_iterator *
+	int
 	(*create_iterator)(struct index_read_view *rv, enum iterator_type type,
-			   const char *key, uint32_t part_count);
+			   const char *key, uint32_t part_count,
+			   struct index_read_view_iterator *it);
 };
 
 /**
@@ -674,13 +675,10 @@ struct index_read_view {
 	struct space_read_view *space;
 };
 
-/** Iterator over an index read view. */
-struct index_read_view_iterator {
+/** Base class for iterator over an index read view. */
+struct index_read_view_iterator_base {
 	/** Pointer to the index read view. */
 	struct index_read_view *index;
-	/** Free an index read view iterator instance. */
-	void
-	(*free)(struct index_read_view_iterator *iterator);
 	/**
 	 * Iterate to the next tuple in the read view.
 	 *
@@ -696,6 +694,29 @@ struct index_read_view_iterator {
 		    const char **data, uint32_t *size);
 };
 
+/** Size of the index_read_view_iterator struct. */
+#define INDEX_READ_VIEW_ITERATOR_SIZE 48
+
+static_assert(sizeof(struct index_read_view_iterator_base) <=
+	      INDEX_READ_VIEW_ITERATOR_SIZE,
+	      "sizeof(struct index_read_view_iterator_base) must be less than "
+	      "or equal to INDEX_READ_VIEW_ITERATOR_SIZE");
+
+/**
+ * Iterator over an index read view.
+ *
+ * Implemented as an opaque fixed-size structure so that it can be declared on
+ * stack for any kind of read view iterator.
+ */
+struct index_read_view_iterator {
+	union {
+		/* Base class. */
+		struct index_read_view_iterator_base base;
+		/* Implementation dependent content. */
+		char data[INDEX_READ_VIEW_ITERATOR_SIZE];
+	};
+};
+
 /**
  * Check if replacement of an old tuple with a new one is
  * allowed.
@@ -954,29 +975,26 @@ index_read_view_get_raw(struct index_read_view *rv,
 	return rv->vtab->get_raw(rv, key, part_count, data, size);
 }
 
-static inline struct index_read_view_iterator *
+static inline int
 index_read_view_create_iterator(struct index_read_view *rv,
 				enum iterator_type type,
-				const char *key, uint32_t part_count)
+				const char *key, uint32_t part_count,
+				struct index_read_view_iterator *it)
 {
-	return rv->vtab->create_iterator(rv, type, key, part_count);
+	return rv->vtab->create_iterator(rv, type, key, part_count, it);
 }
 
 static inline void
-index_read_view_iterator_delete(struct index_read_view_iterator *iterator)
+index_read_view_iterator_destroy(struct index_read_view_iterator *iterator)
 {
-	/*
-	 * A read view iterator may be deleted after the read view is closed so
-	 * we don't check the read view ownership here.
-	 */
-	iterator->free(iterator);
+	TRASH(iterator);
 }
 
 static inline int
 index_read_view_iterator_next_raw(struct index_read_view_iterator *iterator,
 				  const char **data, uint32_t *size)
 {
-	return iterator->next_raw(iterator, data, size);
+	return iterator->base.next_raw(iterator, data, size);
 }
 
 /*
diff --git a/src/box/memtx_engine.cc b/src/box/memtx_engine.cc
index 26c4fcb6ae..d5889551d8 100644
--- a/src/box/memtx_engine.cc
+++ b/src/box/memtx_engine.cc
@@ -848,15 +848,14 @@ checkpoint_f(va_list ap)
 		struct index_read_view *index_rv =
 			space_read_view_index(space_rv, 0);
 		assert(index_rv != NULL);
-		struct index_read_view_iterator *it =
-			index_read_view_create_iterator(index_rv, ITER_ALL,
-							NULL, 0);
-		if (it == NULL) {
+		struct index_read_view_iterator it;
+		if (index_read_view_create_iterator(index_rv, ITER_ALL,
+						    NULL, 0, &it) != 0) {
 			rc = -1;
 			break;
 		}
 		while (true) {
-			rc = index_read_view_iterator_next_raw(it, &data,
+			rc = index_read_view_iterator_next_raw(&it, &data,
 							       &size);
 			if (rc != 0 || data == NULL)
 				break;
@@ -867,7 +866,7 @@ checkpoint_f(va_list ap)
 				break;
 			fiber_gc();
 		}
-		index_read_view_iterator_delete(it);
+		index_read_view_iterator_destroy(&it);
 		if (rc != 0)
 			break;
 	}
@@ -1085,17 +1084,16 @@ memtx_join_f(va_list ap)
 		struct index_read_view *index_rv =
 			space_read_view_index(space_rv, 0);
 		assert(index_rv != NULL);
-		struct index_read_view_iterator *it =
-			index_read_view_create_iterator(index_rv, ITER_ALL,
-							NULL, 0);
-		if (it == NULL) {
+		struct index_read_view_iterator it;
+		if (index_read_view_create_iterator(index_rv, ITER_ALL,
+						    NULL, 0, &it) != 0) {
 			rc = -1;
 			break;
 		}
 		uint32_t size;
 		const char *data;
 		while (true) {
-			rc = index_read_view_iterator_next_raw(it, &data,
+			rc = index_read_view_iterator_next_raw(&it, &data,
 							       &size);
 			if (rc != 0 || data == NULL)
 				break;
@@ -1105,7 +1103,7 @@ memtx_join_f(va_list ap)
 				break;
 			fiber_gc();
 		}
-		index_read_view_iterator_delete(it);
+		index_read_view_iterator_destroy(&it);
 		if (rc != 0)
 			break;
 	}
diff --git a/src/box/memtx_hash.cc b/src/box/memtx_hash.cc
index 8b9593be8d..04ec67cfdf 100644
--- a/src/box/memtx_hash.cc
+++ b/src/box/memtx_hash.cc
@@ -516,11 +516,16 @@ struct hash_read_view {
 /** Read view iterator implementation. */
 struct hash_read_view_iterator {
 	/** Base class. */
-	struct index_read_view_iterator base;
+	struct index_read_view_iterator_base base;
 	/** Light iterator. */
 	struct light_index_iterator iterator;
 };
 
+static_assert(sizeof(struct hash_read_view_iterator) <=
+	      INDEX_READ_VIEW_ITERATOR_SIZE,
+	      "sizeof(struct hash_read_view_iterator) must be less than or "
+	      "equal to INDEX_READ_VIEW_ITERATOR_SIZE");
+
 static void
 hash_read_view_free(struct index_read_view *base)
 {
@@ -532,14 +537,6 @@ hash_read_view_free(struct index_read_view *base)
 	free(rv);
 }
 
-static void
-hash_read_view_iterator_free(struct index_read_view_iterator *iterator)
-{
-	assert(iterator->free == hash_read_view_iterator_free);
-	TRASH(iterator);
-	free(iterator);
-}
-
 #if defined(ENABLE_READ_VIEW)
 # include "memtx_hash_read_view.cc"
 #else /* !defined(ENABLE_READ_VIEW) */
@@ -563,10 +560,9 @@ static int
 hash_read_view_iterator_next_raw(struct index_read_view_iterator *iterator,
 				 const char **data, uint32_t *size)
 {
-	assert(iterator->free == hash_read_view_iterator_free);
 	struct hash_read_view_iterator *it =
 		(struct hash_read_view_iterator *)iterator;
-	struct hash_read_view *rv = (struct hash_read_view *)iterator->index;
+	struct hash_read_view *rv = (struct hash_read_view *)it->base.index;
 
 	while (true) {
 		struct tuple **res = light_index_view_iterator_get_and_next(
@@ -611,23 +607,19 @@ hash_read_view_reset_key_def(struct hash_read_view *rv)
 #endif /* !defined(ENABLE_READ_VIEW) */
 
 /** Implementation of create_iterator index_read_view callback. */
-static struct index_read_view_iterator *
+static int
 hash_read_view_create_iterator(struct index_read_view *base,
 			       enum iterator_type type,
-			       const char *key, uint32_t part_count)
+			       const char *key, uint32_t part_count,
+			       struct index_read_view_iterator *iterator)
 {
 	struct hash_read_view *rv = (struct hash_read_view *)base;
 	struct hash_read_view_iterator *it =
-		(struct hash_read_view_iterator *)xmalloc(sizeof(*it));
+		(struct hash_read_view_iterator *)iterator;
 	it->base.index = base;
-	it->base.free = hash_read_view_iterator_free;
 	it->base.next_raw = exhausted_index_read_view_iterator_next_raw;
 	light_index_view_iterator_begin(&rv->view, &it->iterator);
-	if (hash_read_view_iterator_start(it, type, key, part_count) != 0) {
-		index_read_view_iterator_delete(&it->base);
-		return NULL;
-	}
-	return (struct index_read_view_iterator *)it;
+	return hash_read_view_iterator_start(it, type, key, part_count);
 }
 
 /** Implementation of create_read_view index callback. */
diff --git a/src/box/memtx_tree.cc b/src/box/memtx_tree.cc
index cbc5d94c1e..a92103f1a4 100644
--- a/src/box/memtx_tree.cc
+++ b/src/box/memtx_tree.cc
@@ -1856,13 +1856,22 @@ struct tree_read_view {
 template <bool USE_HINT>
 struct tree_read_view_iterator {
 	/** Base class. */
-	struct index_read_view_iterator base;
+	struct index_read_view_iterator_base base;
 	/** Iterator key. */
 	struct memtx_tree_key_data<USE_HINT> key_data;
 	/** BPS tree iterator. */
 	memtx_tree_iterator_t<USE_HINT> tree_iterator;
 };
 
+static_assert(sizeof(struct tree_read_view_iterator<false>) <=
+	      INDEX_READ_VIEW_ITERATOR_SIZE,
+	      "sizeof(struct tree_read_view_iterator<false>) must be less than "
+	      "or equal to INDEX_READ_VIEW_ITERATOR_SIZE");
+static_assert(sizeof(struct tree_read_view_iterator<true>) <=
+	      INDEX_READ_VIEW_ITERATOR_SIZE,
+	      "sizeof(struct tree_read_view_iterator<true>) must be less than "
+	      "or equal to INDEX_READ_VIEW_ITERATOR_SIZE");
+
 template <bool USE_HINT>
 static void
 tree_read_view_free(struct index_read_view *base)
@@ -1876,15 +1885,6 @@ tree_read_view_free(struct index_read_view *base)
 	free(rv);
 }
 
-template <bool USE_HINT>
-static void
-tree_read_view_iterator_free(struct index_read_view_iterator *iterator)
-{
-	assert(iterator->free == &tree_read_view_iterator_free<USE_HINT>);
-	TRASH(iterator);
-	free(iterator);
-}
-
 #if defined(ENABLE_READ_VIEW)
 # include "memtx_tree_read_view.cc"
 #else /* !defined(ENABLE_READ_VIEW) */
@@ -1910,11 +1910,10 @@ static int
 tree_read_view_iterator_next_raw(struct index_read_view_iterator *iterator,
 				 const char **data, uint32_t *size)
 {
-	assert(iterator->free == &tree_read_view_iterator_free<USE_HINT>);
 	struct tree_read_view_iterator<USE_HINT> *it =
 		(struct tree_read_view_iterator<USE_HINT> *)iterator;
 	struct tree_read_view<USE_HINT> *rv =
-		(struct tree_read_view<USE_HINT> *)iterator->index;
+		(struct tree_read_view<USE_HINT> *)it->base.index;
 
 	while (true) {
 		struct memtx_tree_data<USE_HINT> *res =
@@ -1967,27 +1966,22 @@ tree_read_view_reset_key_def(struct tree_read_view<USE_HINT> *rv)
 
 /** Implementation of create_iterator index_read_view callback. */
 template <bool USE_HINT>
-static struct index_read_view_iterator *
+static int
 tree_read_view_create_iterator(struct index_read_view *base,
 			       enum iterator_type type,
-			       const char *key, uint32_t part_count)
+			       const char *key, uint32_t part_count,
+			       struct index_read_view_iterator *iterator)
 {
 	struct tree_read_view_iterator<USE_HINT> *it =
-		(struct tree_read_view_iterator<USE_HINT> *)
-		xmalloc(sizeof(*it));
+		(struct tree_read_view_iterator<USE_HINT> *)iterator;
 	it->base.index = base;
-	it->base.free = tree_read_view_iterator_free<USE_HINT>;
 	it->base.next_raw = exhausted_index_read_view_iterator_next_raw;
 	it->key_data.key = NULL;
 	it->key_data.part_count = 0;
 	if (USE_HINT)
 		it->key_data.set_hint(HINT_NONE);
 	invalidate_tree_iterator(&it->tree_iterator);
-	if (tree_read_view_iterator_start(it, type, key, part_count) != 0) {
-		index_read_view_iterator_delete(&it->base);
-		return NULL;
-	}
-	return (struct index_read_view_iterator *)it;
+	return tree_read_view_iterator_start(it, type, key, part_count);
 }
 
 /** Implementation of create_read_view index callback. */
diff --git a/src/box/sequence.c b/src/box/sequence.c
index d56780be9a..7f479274cd 100644
--- a/src/box/sequence.c
+++ b/src/box/sequence.c
@@ -311,20 +311,25 @@ struct sequence_data_read_view {
 /** Read view iterator implementation. */
 struct sequence_data_iterator {
 	/** Base class. */
-	struct index_read_view_iterator base;
+	struct index_read_view_iterator_base base;
 	/** Iterator over the data index. */
 	struct light_sequence_iterator iter;
 };
 
+static_assert(sizeof(struct sequence_data_iterator) <=
+	      INDEX_READ_VIEW_ITERATOR_SIZE,
+	      "sizeof(struct sequence_data_iterator) must be less than or "
+	      "equal to INDEX_READ_VIEW_ITERATOR_SIZE");
+
 /** Implementation of next_raw index_read_view_iterator callback. */
 static int
-sequence_data_iterator_next_raw(struct index_read_view_iterator *base,
+sequence_data_iterator_next_raw(struct index_read_view_iterator *iterator,
 				const char **data, uint32_t *size)
 {
 	struct sequence_data_iterator *iter =
-		(struct sequence_data_iterator *)base;
+		(struct sequence_data_iterator *)iterator;
 	struct sequence_data_read_view *rv =
-		(struct sequence_data_read_view *)base->index;
+		(struct sequence_data_read_view *)iter->base.index;
 
 	struct sequence_data *sd = light_sequence_view_iterator_get_and_next(
 		&rv->view, &iter->iter);
@@ -348,13 +353,6 @@ sequence_data_iterator_next_raw(struct index_read_view_iterator *base,
 	return 0;
 }
 
-static void
-sequence_data_iterator_free(struct index_read_view_iterator *iter)
-{
-	TRASH(iter);
-	free(iter);
-}
-
 static inline int
 sequence_data_read_view_get_raw(struct index_read_view *rv,
 				const char *key, uint32_t part_count,
@@ -371,26 +369,27 @@ sequence_data_read_view_get_raw(struct index_read_view *rv,
 }
 
 /** Implementation of create_iterator index_read_view callback. */
-static struct index_read_view_iterator *
+static int
 sequence_data_iterator_create(struct index_read_view *base,
 			      enum iterator_type type,
-			      const char *key, uint32_t part_count)
+			      const char *key, uint32_t part_count,
+			      struct index_read_view_iterator *iterator)
 {
 	if (type != ITER_ALL) {
 		diag_set(ClientError, ER_UNSUPPORTED,
 			 "_sequence_data read view", "requested iterator type");
-		return NULL;
+		return -1;
 	}
 	(void)key;
 	(void)part_count;
 	struct sequence_data_read_view *rv =
 		(struct sequence_data_read_view *)base;
-	struct sequence_data_iterator *iter = xmalloc(sizeof(*iter));
+	struct sequence_data_iterator *iter =
+		(struct sequence_data_iterator *)iterator;
 	iter->base.index = base;
 	iter->base.next_raw = sequence_data_iterator_next_raw;
-	iter->base.free = sequence_data_iterator_free;
 	light_sequence_view_iterator_begin(&rv->view, &iter->iter);
-	return (struct index_read_view_iterator *)iter;
+	return 0;
 }
 
 static void
-- 
GitLab