[Tarantool-patches] [PATCH] lua: refactor port_lua_do_dump and encode_lua_call

[Vladislav Shpilevoy]

Hi! Thanks for the patch!

> Branch: https://github.com/tarantool/tarantool/tree/skaplun/gh-noticket-refactor-lua-call
> See the benchmarks sources here [1].
> 
> Before patch:
> | Encode map: 189851357 mcs, 15.8 K ps
> | Encode seq: 187926351 mcs, 16.0 K ps
> | Encode str: 185451675 mcs, 16.2 K ps
> | Encode dig: 184833396 mcs, 16.2 K ps
> 
> After patch:
> | Encode map: 187814261 mcs, 16.0 K ps
> | Encode seq: 183755028 mcs, 16.3 K ps
> | Encode str: 181571626 mcs, 16.5 K ps
> | Encode dig: 181572998 mcs, 16.5 K ps
> 
> Looks like the perf doesn't degrade at least.
> 
> [1]: https://gist.github.com/Buristan/3e6d6bf2c722874bec55a8c5a44b98f3
> 
>  src/box/lua/call.c | 71 ++++++++++++++++++++++++++++++++++++----------
>  1 file changed, 56 insertions(+), 15 deletions(-)
> 
> diff --git a/src/box/lua/call.c b/src/box/lua/call.c
> index 0315e720c..3b2572096 100644
> --- a/src/box/lua/call.c
> +++ b/src/box/lua/call.c
> @@ -450,13 +482,20 @@ port_lua_do_dump(struct port *base, struct mpstream *stream,
>  	struct encode_lua_ctx ctx;
>  	ctx.port = port;
>  	ctx.stream = stream;
> -	struct lua_State *L = tarantool_L;
> -	int top = lua_gettop(L);
> -	if (lua_cpcall(L, handler, &ctx) != 0) {
> -		luaT_toerror(port->L);
> +	lua_State *L = port->L;
> +	/*
> +	 * At the moment Lua stack holds only values to encode.
> +	 * Insert corresponding encoder to the bottom and push
> +	 * encode context as lightuserdata to the top.
> +	 */
> +	const int size = lua_gettop(L);
> +	lua_rawgeti(L, LUA_REGISTRYINDEX, execute_lua_refs[handler]);
> +	assert(lua_isfunction(L, -1) && lua_iscfunction(L, -1));
> +	lua_insert(L, 1);

If I remember correctly, this leads to moving all the existing
stack elements forward. Which might be expensive. I know from
Vlad Grubov's words that they have code with hundreds of values in
multireturn from stored functions. Did you bench what happens when
the Lua coroutine contains a lot of values? In the bench by the
link above I see only 1-element array and map. Size of the array
and map does not matter though. Only multireturn is interesting
here. Like 'return 1, 2, 3, ...'.