From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 13DFC4696C8 for ; Thu, 23 Apr 2020 03:12:46 +0300 (MSK) From: Vladislav Shpilevoy Date: Thu, 23 Apr 2020 02:12:41 +0200 Message-Id: <4177aec25c7ff283575a0ccb3a3a62d2ee51fde8.1587600640.git.v.shpilevoy@tarantool.org> In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [Tarantool-patches] [PATCH v2 3/3] box: replace port_tuple with port_c everywhere List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: tarantool-patches@dev.tarantool.org, korablev@tarantool.org, imun@tarantool.org 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)