Re: [PATCH v2 8/9] box: implement lua_port dump to region and to Lua

[Vladimir Davydov <vdavydov.dev@gmail.com>]

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.