From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lj1-f194.google.com (mail-lj1-f194.google.com [209.85.208.194]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 7A50E469719 for ; Fri, 9 Oct 2020 09:57:58 +0300 (MSK) Received: by mail-lj1-f194.google.com with SMTP id a5so8522663ljj.11 for ; Thu, 08 Oct 2020 23:57:58 -0700 (PDT) Date: Fri, 9 Oct 2020 09:57:55 +0300 From: Cyrill Gorcunov Message-ID: <20201009065755.GJ2069@grain> References: <20201008213608.1022476-1-gorcunov@gmail.com> <20201008213608.1022476-6-gorcunov@gmail.com> <3cc78ca9-5bf1-0128-1a09-313256be9253@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <3cc78ca9-5bf1-0128-1a09-313256be9253@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH v5 5/6] box/cbox: implement cbox Lua module List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vladislav Shpilevoy Cc: tml On Fri, Oct 09, 2020 at 12:35:09AM +0200, Vladislav Shpilevoy wrote: > > + > > +#include "box/error.h" > > +#include "box/func.h" > > 1. Why do you still depend on box/func.h? Func.h is > about stored functions with credentials and stuff. In the > previous review I proposed to move needed parts to a new > system: module_sym of whatever it is called. So it would > be module_sym.h and module_sym.c. And it would not depend > on schema at all. Currently func.h brings schema dependency > into cbox. Well, I remember your proposal. The reason is indeed to include module_sym structure which sits in func.h. I thought we could make this separation later together with fix of lazy address loading. But on the other hands let me try to implement it today. > Maybe a good name for the new system would be module_cache.h > and module_cache.c. Like we have with coll_id_cache.h/.c. > (The structs can remain module_sym.) Sounds reasonable. > > + > > + int coro_ref = luaL_ref(tarantool_L, LUA_REGISTRYINDEX); > > + lua_xmove(L, args_L, lua_gettop(L) - 1); > > 2. Why didn't you use lua_remove() to get rid of the unnecessary > parts of the stack? Which one you think is redundant? The object itself? If so then it sits as a bottom element of the stack and removing it would simply make code slower. Vlad this "prepare environment for calling" part of code is very sensitive to the arguments, I tried a various combinations in attempts to get rid of lua thread and none of them work. So I left it as is and put a FIXME, because it requires more serious rework. Or I misundertood you, and you meant something different? > > + > > + lua_pushstring(L, "__call"); > > + lua_pushcfunction(L, lcbox_func_call); > > + lua_settable(L, -3); > > + > > + lua_setmetatable(L, -2); > > 3. What happens when the function object is deleted by Lua GC? > It seems the user can't get the function again without dropping > it. And that is a good question. I must confess I missed Lua GC aspect here. Thanks a huge! I'll rework!