From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <skaplun@tarantool.org>
Received: from smtp41.i.mail.ru (smtp41.i.mail.ru [94.100.177.101])
 (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
 (No client certificate requested)
 by dev.tarantool.org (Postfix) with ESMTPS id D66C3469710
 for <tarantool-patches@dev.tarantool.org>;
 Tue,  2 Jun 2020 17:16:49 +0300 (MSK)
Date: Tue, 2 Jun 2020 17:16:36 +0300
From: Sergey Kaplun <skaplun@tarantool.org>
Message-ID: <20200602141636.GA8942@root>
References: <20200602121949.20254-1-skaplun@tarantool.org>
 <20200602135140.GC5745@tarantool.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20200602135140.GC5745@tarantool.org>
Subject: Re: [Tarantool-patches] [PATCH v2] lua: remove excess Lua call from
	table encoding
List-Id: Tarantool development patches <tarantool-patches.dev.tarantool.org>
List-Unsubscribe: <https://lists.tarantool.org/mailman/options/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=unsubscribe>
List-Archive: <https://lists.tarantool.org/pipermail/tarantool-patches/>
List-Post: <mailto:tarantool-patches@dev.tarantool.org>
List-Help: <mailto:tarantool-patches-request@dev.tarantool.org?subject=help>
List-Subscribe: <https://lists.tarantool.org/mailman/listinfo/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=subscribe>
To: Igor Munkin <imun@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org, Vladislav Shpilevoy <v.shpilevoy@tarantool.org>

Hi! Thanks for the review!

On 02.06.20, Igor Munkin wrote:
> Sergey,
> 
> Thanks, the patch LGTM except the one typo below.
> 
> On 02.06.20, Sergey Kaplun wrote:
> > For safe table encoding <lua_field_try_serialize> function is pushed
> > to Lua stack along with auxiliary lightuserdata and table object to be
> > encoded. Its further protected call catches Lua error if one is raised
> > while encoding. It is only necessary when the object to be serialized
> > has __serialize field in metatable and this field is a Lua function.
> > 
> > This change reduces GC usage since a Lua function object is not
> > created. Moreover the function serializing the given object is called
> > without excess protected frame and auxiliary status struct.
> > ---
> > 
> > branch: https://github.com/tarantool/tarantool/tree/skaplun/no-ticket-lua-inspect-table-refactoring
> > 
> >  src/lua/utils.c | 136 +++++++++++++++++-------------------------------
> >  1 file changed, 48 insertions(+), 88 deletions(-)
> > 
> > diff --git a/src/lua/utils.c b/src/lua/utils.c
> > index d410a3d03..67cab802c 100644
> > --- a/src/lua/utils.c
> > +++ b/src/lua/utils.c
> 
> <snipped>
> 
> > +		/* Fallthrouth with res == 1 */
> 
> Typo again: s/Fallthrouth/Fallthrough/.
> 
> <snipped>
> 

I added corresponding fix to the branch

diff --git a/src/lua/utils.c b/src/lua/utils.c
index 67cab802c..0b05d7257 100644
--- a/src/lua/utils.c
+++ b/src/lua/utils.c
@@ -548,7 +548,7 @@ lua_field_inspect_table(struct lua_State *L, struct luaL_serializer *cfg,
        (void)top;
        if (res == 0)
            return 0;
-       /* Fallthrouth with res == 1 */
+       /* Fallthrough with res == 1 */
    }
 
    field->type = MP_ARRAY;


> > -- 
> > 2.24.1
> > 
> 
> -- 
> Best regards,
> IM

-- 
Best regards,
Sergey Kaplun