[Tarantool-patches] [PATCH v2 3/3] box: replace port_tuple with port_c everywhere

Vladislav Shpilevoy v.shpilevoy at tarantool.org
Thu Apr 23 03:12:41 MSK 2020


Port_tuple is exactly the same as port_c, but is not able to store
raw MessagePack. I theory it sounds like port_tuple should be a
bit simpler and therefore faster, but in fact it is not.
Microbenchmarks didn't reveal any difference. So port_tuple is no
longer needed, all its functionality is covered by port_c.

Follow up #4641
---
 src/box/box.cc         |   4 +-
 src/box/execute.c      |  10 ++--
 src/box/execute.h      |  13 +++--
 src/box/key_list.c     |   4 +-
 src/box/lua/execute.c  |   6 +--
 src/box/lua/misc.cc    |  18 -------
 src/box/lua/schema.lua |  29 ++++++-----
 src/box/port.c         | 109 ++---------------------------------------
 src/box/port.h         |  41 ----------------
 src/lib/core/port.h    |   4 +-
 10 files changed, 41 insertions(+), 197 deletions(-)

diff --git a/src/box/box.cc b/src/box/box.cc
index 9fd10d64c..c01b08613 100644
--- a/src/box/box.cc
+++ b/src/box/box.cc
@@ -1199,7 +1199,7 @@ box_select(uint32_t space_id, uint32_t index_id,
 	int rc = 0;
 	uint32_t found = 0;
 	struct tuple *tuple;
-	port_tuple_create(port);
+	port_c_create(port);
 	while (found < limit) {
 		rc = iterator_next(it, &tuple);
 		if (rc != 0 || tuple == NULL)
@@ -1208,7 +1208,7 @@ box_select(uint32_t space_id, uint32_t index_id,
 			offset--;
 			continue;
 		}
-		rc = port_tuple_add(port, tuple);
+		rc = port_c_add_tuple(port, tuple);
 		if (rc != 0)
 			break;
 		found++;
diff --git a/src/box/execute.c b/src/box/execute.c
index 66eac9814..6feb88036 100644
--- a/src/box/execute.c
+++ b/src/box/execute.c
@@ -103,7 +103,7 @@ port_sql_dump_msgpack(struct port *port, struct obuf *out);
 static void
 port_sql_destroy(struct port *base)
 {
-	port_tuple_vtab.destroy(base);
+	port_c_vtab.destroy(base);
 	struct port_sql *port_sql = (struct port_sql *) base;
 	if (port_sql->do_finalize)
 		sql_stmt_finalize(((struct port_sql *)base)->stmt);
@@ -123,7 +123,7 @@ static void
 port_sql_create(struct port *port, struct sql_stmt *stmt,
 		enum sql_serialization_format format, bool do_finalize)
 {
-	port_tuple_create(port);
+	port_c_create(port);
 	port->vtab = &port_sql_vtab;
 	struct port_sql *port_sql = (struct port_sql *) port;
 	port_sql->stmt = stmt;
@@ -271,7 +271,7 @@ sql_row_to_port(struct sql_stmt *stmt, int column_count,
 	if (tuple == NULL)
 		goto error;
 	region_truncate(region, svp);
-	return port_tuple_add(port, tuple);
+	return port_c_add_tuple(port, tuple);
 
 error:
 	region_truncate(region, svp);
@@ -492,13 +492,13 @@ port_sql_dump_msgpack(struct port *port, struct obuf *out)
 			return -1;
 		}
 		pos = mp_encode_uint(pos, IPROTO_DATA);
-		if (port_tuple_vtab.dump_msgpack(port, out) < 0)
+		if (port_c_vtab.dump_msgpack(port, out) < 0)
 			return -1;
 		break;
 	}
 	case DML_EXECUTE: {
 		int keys = 1;
-		assert(((struct port_tuple *)port)->size == 0);
+		assert(((struct port_c *)port)->size == 0);
 		struct stailq *autoinc_id_list =
 			vdbe_autoinc_id_list((struct Vdbe *)stmt);
 		uint32_t map_size = stailq_empty(autoinc_id_list) ? 1 : 2;
diff --git a/src/box/execute.h b/src/box/execute.h
index 5e2327f4a..c08cfbf25 100644
--- a/src/box/execute.h
+++ b/src/box/execute.h
@@ -91,17 +91,16 @@ sql_prepare_and_execute(const char *sql, int len, const struct sql_bind *bind,
 /**
  * Port implementation that is used to store SQL responses and
  * output them to obuf or Lua. This port implementation is
- * inherited from the port_tuple structure. This allows us to use
- * this structure in the port_tuple methods instead of port_tuple
- * itself.
+ * inherited from the port_c structure. This allows us to use
+ * this structure in the port_c methods instead of port_c itself.
  *
- * The methods of port_tuple are called via explicit access to
- * port_tuple_vtab just like C++ does with BaseClass::method, when
+ * The methods of port_c are called via explicit access to
+ * port_c_vtab just like C++ does with BaseClass::method, when
  * it is called in a child method.
  */
 struct port_sql {
-	/* port_tuple to inherit from. */
-	struct port_tuple port_tuple;
+	/** Base port struct. To be able to use port_c methods. */
+	struct port_c base;
 	/* Prepared SQL statement. */
 	struct sql_stmt *stmt;
 	/**
diff --git a/src/box/key_list.c b/src/box/key_list.c
index 7c1fa70e3..6143b84a1 100644
--- a/src/box/key_list.c
+++ b/src/box/key_list.c
@@ -56,8 +56,8 @@ key_list_iterator_create(struct key_list_iterator *it, struct tuple *tuple,
 	struct func *func = index_def->key_def->func_index_func;
 
 	struct port out_port, in_port;
-	port_tuple_create(&in_port);
-	port_tuple_add(&in_port, tuple);
+	port_c_create(&in_port);
+	port_c_add_tuple(&in_port, tuple);
 	int rc = func_call(func, &in_port, &out_port);
 	port_destroy(&in_port);
 	if (rc != 0) {
diff --git a/src/box/lua/execute.c b/src/box/lua/execute.c
index b5db3c9a2..b4c464af7 100644
--- a/src/box/lua/execute.c
+++ b/src/box/lua/execute.c
@@ -175,12 +175,12 @@ port_sql_dump_lua(struct port *port, struct lua_State *L, bool is_flat)
 		lua_createtable(L, 0, 2);
 		lua_sql_get_metadata(stmt, L, sql_column_count(stmt));
 		lua_setfield(L, -2, "metadata");
-		port_tuple_vtab.dump_lua(port, L, false);
+		port_c_vtab.dump_lua(port, L, false);
 		lua_setfield(L, -2, "rows");
 		break;
 	}
 	case DML_EXECUTE: {
-		assert(((struct port_tuple *) port)->size == 0);
+		assert(((struct port_c *) port)->size == 0);
 		struct stailq *autoinc_id_list =
 			vdbe_autoinc_id_list((struct Vdbe *) stmt);
 		lua_createtable(L, 0, stailq_empty(autoinc_id_list) ? 1 : 2);
@@ -237,7 +237,7 @@ port_sql_dump_lua(struct port *port, struct lua_State *L, bool is_flat)
 		break;
 	}
 	case DML_PREPARE : {
-		assert(((struct port_tuple *) port)->size == 0);
+		assert(((struct port_c *) port)->size == 0);
 		/* Format is following:
 		 * stmt_id,
 		 * param_count,
diff --git a/src/box/lua/misc.cc b/src/box/lua/misc.cc
index 55677ed61..ae8fbb682 100644
--- a/src/box/lua/misc.cc
+++ b/src/box/lua/misc.cc
@@ -60,24 +60,6 @@ lbox_encode_tuple_on_gc(lua_State *L, int idx, size_t *p_len)
 	return (char *) region_join_xc(gc, *p_len);
 }
 
-/**
- * Dump port_tuple content to Lua as a table. Used in box/port.c,
- * but implemented here to eliminate port.c dependency on Lua.
- */
-extern "C" void
-port_tuple_dump_lua(struct port *base, struct lua_State *L, bool is_flat)
-{
-	struct port_tuple *port = port_tuple(base);
-	if (!is_flat)
-		lua_createtable(L, port->size, 0);
-	struct port_tuple_entry *pe = port->first;
-	for (int i = 0; pe != NULL; pe = pe->next) {
-		luaT_pushtuple(L, pe->tuple);
-		if (!is_flat)
-			lua_rawseti(L, -2, ++i);
-	}
-}
-
 extern "C" void
 port_c_dump_lua(struct port *base, struct lua_State *L, bool is_flat)
 {
diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua
index 10584494b..fd1628930 100644
--- a/src/box/lua/schema.lua
+++ b/src/box/lua/schema.lua
@@ -79,17 +79,21 @@ ffi.cdef[[
     box_txn_savepoint_t *
     box_txn_savepoint();
 
-    struct port_tuple_entry {
-        struct port_tuple_entry *next;
-        struct tuple *tuple;
+    struct __attribute__((packed)) port_c_entry {
+        struct port_c_entry *next;
+        union {
+            struct tuple *tuple;
+            char *mp;
+        };
+        uint32_t mp_size;
     };
 
-    struct port_tuple {
+    struct port_c {
         const struct port_vtab *vtab;
-        int size;
-        struct port_tuple_entry *first;
-        struct port_tuple_entry *last;
-        struct port_tuple_entry first_entry;
+        struct port_c_entry *first;
+        struct port_c_entry *last;
+        struct port_c_entry first_entry;
+        int count;
     };
 
     void
@@ -1278,8 +1282,7 @@ local iterator_gen_luac = function(param, state)
 end
 
 -- global struct port instance to use by select()/get()
-local port_tuple = ffi.new('struct port_tuple')
-local port_tuple_entry_t = ffi.typeof('struct port_tuple_entry')
+local port_c = ffi.new('struct port_c')
 
 -- Helper function to check space:method() usage
 local function check_space_arg(space, method)
@@ -1533,7 +1536,7 @@ base_index_mt.select_ffi = function(index, key, opts)
     local key, key_end = tuple_encode(key)
     local iterator, offset, limit = check_select_opts(opts, key + 1 >= key_end)
 
-    local port = ffi.cast('struct port *', port_tuple)
+    local port = ffi.cast('struct port *', port_c)
 
     if builtin.box_select(index.space_id, index.id,
         iterator, offset, limit, key, key_end, port) ~= 0 then
@@ -1541,8 +1544,8 @@ base_index_mt.select_ffi = function(index, key, opts)
     end
 
     local ret = {}
-    local entry = port_tuple.first
-    for i=1,tonumber(port_tuple.size),1 do
+    local entry = port_c.first
+    for i=1,tonumber(port_c.count),1 do
         ret[i] = tuple_bless(entry.tuple)
         entry = entry.next
     end
diff --git a/src/box/port.c b/src/box/port.c
index 2c1fadb5c..9d9fc1dbc 100644
--- a/src/box/port.c
+++ b/src/box/port.c
@@ -38,106 +38,15 @@
 #include "errinj.h"
 
 /**
- * The pools is used both by port_c and port_tuple, since their
- * entires are almost of the same size. Also port_c can use
- * objects from the pool to store result data in their memory,
- * when it fits.
+ * The pools is used by port_c to allocate entries and to store
+ * result data when it fits into an object from the pool.
  */
 static struct mempool port_entry_pool;
 
 enum {
-	PORT_ENTRY_SIZE = MAX(sizeof(struct port_c_entry),
-			      sizeof(struct port_tuple_entry)),
+	PORT_ENTRY_SIZE = sizeof(struct port_c_entry),
 };
 
-int
-port_tuple_add(struct port *base, struct tuple *tuple)
-{
-	struct port_tuple *port = port_tuple(base);
-	struct port_tuple_entry *e;
-	if (port->size == 0) {
-		tuple_ref(tuple);
-		e = &port->first_entry;
-		port->first = port->last = e;
-	} else {
-		e = mempool_alloc(&port_entry_pool);
-		if (e == NULL) {
-			diag_set(OutOfMemory, sizeof(*e), "mempool_alloc", "e");
-			return -1;
-		}
-		tuple_ref(tuple);
-		port->last->next = e;
-		port->last = e;
-	}
-	e->tuple = tuple;
-	e->next = NULL;
-	++port->size;
-	return 0;
-}
-
-void
-port_tuple_create(struct port *base)
-{
-	struct port_tuple *port = (struct port_tuple *)base;
-	port->vtab = &port_tuple_vtab;
-	port->size = 0;
-	port->first = NULL;
-	port->last = NULL;
-}
-
-static void
-port_tuple_destroy(struct port *base)
-{
-	struct port_tuple *port = port_tuple(base);
-	struct port_tuple_entry *e = port->first;
-	if (e == NULL)
-		return;
-	tuple_unref(e->tuple);
-	e = e->next;
-	while (e != NULL) {
-		struct port_tuple_entry *cur = e;
-		e = e->next;
-		tuple_unref(cur->tuple);
-		mempool_free(&port_entry_pool, cur);
-	}
-}
-
-static int
-port_tuple_dump_msgpack_16(struct port *base, struct obuf *out)
-{
-	struct port_tuple *port = port_tuple(base);
-	struct port_tuple_entry *pe;
-	for (pe = port->first; pe != NULL; pe = pe->next) {
-		if (tuple_to_obuf(pe->tuple, out) != 0)
-			return -1;
-		ERROR_INJECT(ERRINJ_PORT_DUMP, {
-			diag_set(OutOfMemory, tuple_size(pe->tuple), "obuf_dup",
-				 "data");
-			return -1;
-		});
-	}
-	return port->size;
-}
-
-static int
-port_tuple_dump_msgpack(struct port *base, struct obuf *out)
-{
-	struct port_tuple *port = port_tuple(base);
-	char *size_buf = obuf_alloc(out, mp_sizeof_array(port->size));
-	if (size_buf == NULL) {
-		diag_set(OutOfMemory, mp_sizeof_array(port->size), "obuf_alloc",
-			 "size_buf");
-		return -1;
-	}
-	mp_encode_array(size_buf, port->size);
-	if (port_tuple_dump_msgpack_16(base, out) < 0)
-		return -1;
-	return 1;
-}
-
-extern void
-port_tuple_dump_lua(struct port *base, struct lua_State *L, bool is_flat);
-
 static inline void
 port_c_destroy_entry(struct port_c_entry *pe)
 {
@@ -297,7 +206,7 @@ port_c_dump_lua(struct port *port, struct lua_State *L, bool is_flat);
 extern struct sql_value *
 port_c_get_vdbemem(struct port *base, uint32_t *size);
 
-static const struct port_vtab port_c_vtab = {
+const struct port_vtab port_c_vtab = {
 	.dump_msgpack = port_c_dump_msgpack,
 	.dump_msgpack_16 = port_c_dump_msgpack_16,
 	.dump_lua = port_c_dump_lua,
@@ -328,13 +237,3 @@ port_free(void)
 {
 	mempool_destroy(&port_entry_pool);
 }
-
-const struct port_vtab port_tuple_vtab = {
-	.dump_msgpack = port_tuple_dump_msgpack,
-	.dump_msgpack_16 = port_tuple_dump_msgpack_16,
-	.dump_lua = port_tuple_dump_lua,
-	.dump_plain = NULL,
-	.get_msgpack = NULL,
-	.get_vdbemem = NULL,
-	.destroy = port_tuple_destroy,
-};
diff --git a/src/box/port.h b/src/box/port.h
index 4d6a5b399..0100bac9a 100644
--- a/src/box/port.h
+++ b/src/box/port.h
@@ -40,47 +40,6 @@ extern "C" {
 
 struct tuple;
 
-struct port_tuple_entry {
-	struct port_tuple_entry *next;
-	struct tuple *tuple;
-};
-
-/**
- * Port implementation used for storing tuples.
- */
-struct port_tuple {
-	const struct port_vtab *vtab;
-	int size;
-	struct port_tuple_entry *first;
-	struct port_tuple_entry *last;
-	struct port_tuple_entry first_entry;
-};
-static_assert(sizeof(struct port_tuple) <= sizeof(struct port),
-	      "sizeof(struct port_tuple) must be <= sizeof(struct port)");
-
-extern const struct port_vtab port_tuple_vtab;
-
-/**
- * Convert an abstract port instance to a tuple port.
- */
-static inline struct port_tuple *
-port_tuple(struct port *port)
-{
-	return (struct port_tuple *)port;
-}
-
-/**
- * Create a port for storing tuples.
- */
-void
-port_tuple_create(struct port *port);
-
-/**
- * Append a tuple to a port.
- */
-int
-port_tuple_add(struct port *port, struct tuple *tuple);
-
 /** Port implementation used for storing raw data. */
 struct port_msgpack {
 	const struct port_vtab *vtab;
diff --git a/src/lib/core/port.h b/src/lib/core/port.h
index bfdfa4656..ade12eadf 100644
--- a/src/lib/core/port.h
+++ b/src/lib/core/port.h
@@ -41,6 +41,8 @@ struct obuf;
 struct lua_State;
 struct port;
 
+extern const struct port_vtab port_c_vtab;
+
 /**
  * A single port represents a destination of any output. One such
  * destination can be a Lua stack, or the binary protocol. An
@@ -113,7 +115,7 @@ struct port_vtab {
 
 /**
  * Abstract port instance. It is supposed to be converted to
- * a concrete port realization, e.g. port_tuple.
+ * a concrete port realization, e.g. port_c.
  */
 struct port {
 	/** Virtual method table. */
-- 
2.21.1 (Apple Git-122.3)



More information about the Tarantool-patches mailing list