From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 21C622F9E0 for ; Wed, 28 Nov 2018 08:33:52 -0500 (EST) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id XRA1EMSAX8qi for ; Wed, 28 Nov 2018 08:33:52 -0500 (EST) Received: from smtp47.i.mail.ru (smtp47.i.mail.ru [94.100.177.107]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id B5B442F9D6 for ; Wed, 28 Nov 2018 08:33:51 -0500 (EST) Received: from [185.6.245.156] (port=10069 helo=mimeev-ThinkPad-T460p.mail.msk) by smtp47.i.mail.ru with esmtpa (envelope-from ) id 1gRzyP-0004Sn-UO for tarantool-patches@freelists.org; Wed, 28 Nov 2018 16:33:50 +0300 From: imeevma@tarantool.org Subject: [tarantool-patches] [PATCH v3 2/7] box: add method dump_lua to port Date: Wed, 28 Nov 2018 16:33:49 +0300 Message-Id: In-Reply-To: References: Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: tarantool-patches@freelists.org Hi! Thank you for review and fixes! I squashed your fixes and removed libs from port.c that were added in previous version. New version below. On 11/23/18 12:49 AM, Vladislav Shpilevoy wrote: > Hi! Thanks for the fixes! See my 5 comments below, fix > at the end of the email and on the branch. > > Also, please, do not forget next time to answer to my comments > inlined. Do not just send v2 without any answers. > > On 22/11/2018 22:10, imeevma@tarantool.org wrote: >> New method dump_lua dumps saved in port tuples to Lua stack. It >> will allow us to call this method without any other interaction >> with port. >> >> Needed for #3505 >> --- >> src/box/lua/call.c | 1 + >> src/box/port.c | 22 ++++++++++++++++++++++ >> src/box/port.h | 12 ++++++++++++ >> 3 files changed, 35 insertions(+) >> >> diff --git a/src/box/port.c b/src/box/port.c >> index 266cf3d..a65a32d 100644 >> --- a/src/box/port.c >> +++ b/src/box/port.c >> @@ -36,6 +36,8 @@ >> #include >> #include >> #include "errinj.h" >> +#include "lua/utils.h" >> +#include "lua/tuple.h" > > 1. src/box/ files should not depend in Lua. I moved > implementation to misc.cc and put here extern declaration. Squashed. > >> static struct mempool port_tuple_entry_pool; >> @@ -121,6 +123,19 @@ port_tuple_dump_msgpack(struct port *base, struct obuf *out) >> return 1; >> } >> +static int > > 2. Why do you need to return number of dumped tuples? Dump_msgpack > returns number of tuples since in iproto.cc it firstly reserves > msgpack array header, then dumps port and then fills the reserved > header. For Lua we do not need it. Squashed. > >> +port_tuple_dump_lua(struct port *base, struct lua_State *L) >> +{ >> + struct port_tuple *port = port_tuple(base); >> + struct port_tuple_entry *pe; >> + int i = 0; > > 3. Why did not you add here lua_createtable(L, port->size, 0); > as it is done in misc.cc in the original implementation? Squashed. > > 4. Why did not you replaced lbox_port_to_table with this new > method? > > I did this points and it works. Squashed. > >> + for (pe = port->first; pe != NULL; pe = pe->next) { >> + luaT_pushtuple(L, pe->tuple); >> + lua_rawseti(L, -2, ++i); >> + } >> + return port->size; >> +} >> + >> void >> port_destroy(struct port *port) >> { >> diff --git a/src/box/port.h b/src/box/port.h >> index 882bb37..3bd83b0 100644 >> --- a/src/box/port.h >> +++ b/src/box/port.h >> @@ -185,6 +190,13 @@ int >> port_dump_msgpack_16(struct port *port, struct obuf *out); >> /** >> + * Same as port_dump(), but use the legacy Tarantool 1.6 >> + * format. > > 5. This comment does not make sense to this function. Squashed. > >> + */ >> +int >> +port_dump_lua(struct port *port, struct lua_State *L); >> + >> +/** >> * Dump a port content as a plain text into a buffer, >> * allocated inside. >> * @param port Port with data to dump. >> > New version: commit ff2fc3fd58dd99d3a98ad790c8e82363949cb3db Author: Mergen Imeev Date: Sat Nov 17 15:37:17 2018 +0300 box: add method dump_lua to port New method dump_lua dumps saved in port tuples to Lua stack. It will allow us to call this method without any other interaction with port. Needed for #3505 diff --git a/src/box/lua/call.c b/src/box/lua/call.c index 1f20426..52939ae 100644 --- a/src/box/lua/call.c +++ b/src/box/lua/call.c @@ -424,6 +424,7 @@ port_lua_dump_plain(struct port *port, uint32_t *size); static const struct port_vtab port_lua_vtab = { .dump_msgpack = port_lua_dump, .dump_msgpack_16 = port_lua_dump_16, + .dump_lua = NULL, .dump_plain = port_lua_dump_plain, .destroy = port_lua_destroy, }; diff --git a/src/box/lua/misc.cc b/src/box/lua/misc.cc index 8bd33ae..8de7401 100644 --- a/src/box/lua/misc.cc +++ b/src/box/lua/misc.cc @@ -56,23 +56,26 @@ lbox_encode_tuple_on_gc(lua_State *L, int idx, size_t *p_len) return (char *) region_join_xc(gc, *p_len); } -/* }}} */ - -/** {{{ Lua/C implementation of index:select(): used only by Vinyl **/ - -static inline void -lbox_port_to_table(lua_State *L, struct port *port_base) +/** + * 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) { - struct port_tuple *port = port_tuple(port_base); + struct port_tuple *port = port_tuple(base); lua_createtable(L, port->size, 0); - struct port_tuple_entry *entry = port->first; - for (int i = 0 ; i < port->size; i++) { - luaT_pushtuple(L, entry->tuple); - lua_rawseti(L, -2, i + 1); - entry = entry->next; + struct port_tuple_entry *pe = port->first; + for (int i = 0; pe != NULL; pe = pe->next) { + luaT_pushtuple(L, pe->tuple); + lua_rawseti(L, -2, ++i); } } +/* }}} */ + +/** {{{ Lua/C implementation of index:select(): used only by Vinyl **/ + static int lbox_select(lua_State *L) { @@ -106,7 +109,7 @@ lbox_select(lua_State *L) * table always crashed the first (can't be fixed with pcall). * https://github.com/tarantool/tarantool/issues/1182 */ - lbox_port_to_table(L, &port); + port_dump_lua(&port, L); port_destroy(&port); return 1; /* lua table with tuples */ } diff --git a/src/box/port.c b/src/box/port.c index 266cf3d..853d24c 100644 --- a/src/box/port.c +++ b/src/box/port.c @@ -121,6 +121,9 @@ port_tuple_dump_msgpack(struct port *base, struct obuf *out) return 1; } +extern void +port_tuple_dump_lua(struct port *base, struct lua_State *L); + void port_destroy(struct port *port) { @@ -139,6 +142,12 @@ port_dump_msgpack_16(struct port *port, struct obuf *out) return port->vtab->dump_msgpack_16(port, out); } +void +port_dump_lua(struct port *port, struct lua_State *L) +{ + port->vtab->dump_lua(port, L); +} + const char * port_dump_plain(struct port *port, uint32_t *size) { @@ -161,6 +170,7 @@ port_free(void) 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, .destroy = port_tuple_destroy, }; diff --git a/src/box/port.h b/src/box/port.h index 882bb37..751e44e 100644 --- a/src/box/port.h +++ b/src/box/port.h @@ -77,6 +77,8 @@ struct port_vtab { * 1.6 format. */ int (*dump_msgpack_16)(struct port *port, struct obuf *out); + /** Dump the content of a port to Lua stack. */ + void (*dump_lua)(struct port *port, struct lua_State *L); /** * Dump a port content as a plain text into a buffer, * allocated inside. @@ -184,6 +186,10 @@ port_dump_msgpack(struct port *port, struct obuf *out); int port_dump_msgpack_16(struct port *port, struct obuf *out); +/** Dump port content to Lua stack. */ +void +port_dump_lua(struct port *port, struct lua_State *L); + /** * Dump a port content as a plain text into a buffer, * allocated inside.