[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