From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Mon, 10 Jun 2019 17:24:11 +0300 From: Vladimir Davydov Subject: Re: [PATCH v2 8/9] box: implement lua_port dump to region and to Lua Message-ID: <20190610142411.wvp4mdiqjth3pezg@esperanza> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: To: Kirill Shcherbatov Cc: tarantool-patches@freelists.org List-ID: On Thu, Jun 06, 2019 at 03:04:04PM +0300, Kirill Shcherbatov wrote: > Refactored port_lua class to reuse an existent machinery to > dump info not only for obuf, but to region, that is also > mpstream-compatible. We need this feature in scope of multikey > indexes to work with keys produced with functional index > extractor in memory. > > Also introduce a tiny method .dump_lua for port_lua. It is > necessary to export registered on-board functions call endpoints. > > Class implementation is moved to a new file port_lua.c. > > Part of #4182 > Needed for #1260 > --- > src/box/CMakeLists.txt | 1 + > src/box/execute.c | 1 + > src/box/lua/call.c | 254 +------------------------------- > src/box/lua/port_lua.c | 318 +++++++++++++++++++++++++++++++++++++++++ > src/box/port.h | 2 +- > src/lib/core/port.h | 16 +++ > 6 files changed, 338 insertions(+), 254 deletions(-) > create mode 100644 src/box/lua/port_lua.c As I've already mentioned, it isn't a good practice to move and patch code in the same patch. In this particular case you should've introduced the new port method first and only then move port_lua to a separate file. Anyway, I don't think it's worth moving port_lua out of lua/call.c - this is the only place where it is used and I don't see why we would need it anywhere else. Let's please leave is where it is for now. Regarding the new method. We will need to encapsulate function arguments in a port so that we can pass a tuple with format information to a Lua function, not just raw msgpack: struct port_msgpack { ... /** Pointer to raw msgpack data. */ void *data; void *data_end; }; We will create it from call_request. To forward it to a C function, we will need a new virtual port method that would return a pointer to the raw msgpack, because a C function takes raw msgpack (in contrast to Lua which takes Lua stack for which we already have dump_lua method). In case of port_lua and port_tuple we have no other choice but copy data to an area allocated on the region. However, for port_msgpack we can return a pointer to data stored in it without any copying. So I guess the new method should be called in such a way that it doesn't refer to a region. Not port_dump_msgpack_region. May be, port_get_msgpack or port_to_msgpack. Not sure about the name. The function should either return a pointer to msgpack data already stored in the port or allocate it on the region. We probably need to implement this new method only for port_lua and port_msgpack, because port_tuple will be used as input argument only by functional indexes, which can't use a C function for indexing.