Re: [Tarantool-patches] [PATCH 2/2] box: reduce box_process_lua Lua GC memory usage

[Sergey Ostanevich <sergos@tarantool.org>]

Hi!

Thanks for the patch!

LGTM.

Regards,
Sergos

On 21 Jun 00:24, Igor Munkin wrote:
> Vlad,
> 
> Thanks for the review! I fixed your comments and updated the remote
> branch.
> 
> On 20.06.20, Vladislav Shpilevoy wrote:
> > Hi! Thanks for the patch!
> > 
> > On 19/06/2020 22:40, Igor Munkin wrote:
> > > <box_process_lua> function created a new GCfunc object for a handler
> > > having no upvalues depending on the request context on each call.
> > > 
> > > The changes introduces the folliwing mapping:
> > 
> > Either 'changes' -> 'change', or 'introduces' -> 'introduce'.
> > 
> > 'folliwing' -> 'following'.
> 
> My bad, fixed.
> 
> > 
> > > | <handler id> -> <handler GCfunc object>
> > > Initializing this mapping on Tarantool startup is aimed to reduce Lua GC
> > > memory usage.
> > > 
> > > Signed-off-by: Igor Munkin <imun@tarantool.org>
> > > ---
> > >  src/box/lua/call.c | 32 +++++++++++++++++++++++++++-----
> > >  1 file changed, 27 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/src/box/lua/call.c b/src/box/lua/call.c
> > > index 6588ec2fa..e1b1a5e81 100644
> > > --- a/src/box/lua/call.c
> > > +++ b/src/box/lua/call.c
> > > @@ -48,6 +48,15 @@
> > >  #include "mpstream/mpstream.h"
> > >  #include "box/session.h"
> > >  
> > > +enum handlers {
> > > +	HANDLER_CALL,
> > > +	HANDLER_CALL_BY_REF,
> > > +	HANDLER_EVAL,
> > > +	HANDLER_MAX,
> > 
> > Would be nice to have a comment here explaining why so complex.
> > Why lua_pushcfunction() can't be used on each call.
> 
> OK, here is the diff:
> 
> ================================================================================
> 
> diff --git a/src/box/lua/call.c b/src/box/lua/call.c
> index e1b1a5e81..e52f16ca4 100644
> --- a/src/box/lua/call.c
> +++ b/src/box/lua/call.c
> @@ -48,6 +48,15 @@
>  #include "mpstream/mpstream.h"
>  #include "box/session.h"
>  
> +/*
> + * Handlers identifiers to obtain lua_Cfunction reference from
> + * Lua registry table. These handlers are initialized on Tarantool
> + * startup and are used until the Lua universe is destroyed.
> + * Such approach reduces Lua GC usage since there is no need to
> + * create short-lived GCfunc objects for the corresponding C
> + * function on each iproto CALL/EVAL request or stored Lua
> + * procedure call.
> + */
>  enum handlers {
>  	HANDLER_CALL,
>  	HANDLER_CALL_BY_REF,
> 
> ================================================================================
> 
> > 
> > > +};
> > > +
> > > +static int execute_lua_refs[HANDLER_MAX];
> > > +
> > >  /**
> > >   * A helper to find a Lua function by name and put it
> > >   * on top of the stack.
> 
> -- 
> Best regards,
> IM