Tarantool development patches archive
 help / color / mirror / Atom feed
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
To: Alexander Turenko <alexander.turenko@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH 2.X 5/7] module api: luaT_temp_luastate & luaT_release_temp_luastate
Date: Wed, 30 Sep 2020 01:21:13 +0200	[thread overview]
Message-ID: <0dbdbece-3f14-ba6d-b7ca-50f39f3b46d6@tarantool.org> (raw)
In-Reply-To: <20200929051747.caabtjexvamegnno@tkn_work_nb>

On 29.09.2020 07:17, Alexander Turenko wrote:
> On Tue, Sep 29, 2020 at 12:21:18AM +0200, Vladislav Shpilevoy wrote:
>> Thanks for the patch!
>>
>> I strongly don't like this export. It seems to be too internal.
> 
> I have no point to start discussion. Can you clarify your feeling?

I don't like that we give access to fiber's Lua state. If a user will
leave anything on it, it can lead to anything as well, UB I guess. But
I didn't check.

>> But I have no better ideas than propose to implement your own
>> lua_State cache in the merger module. It does not seem to be too
>> hard, and I don't think it would occupy much memory.
>>
>> Just a simple list of lua_State objects, created by luaT_newthread
>> on demand, and put back into the list when unused. What is wrong
>> with that?
> 
> When we able to simplify modules, it worth to do so (when there are no
> certain objections). Write once, use many.

What are applications for that except the merger? Potentially.

>> Adding Igor as Lua master to help with this.
>>
>> See 2 comments below.
>>
>>> diff --git a/src/lua/utils.h b/src/lua/utils.h
>>> index 9b1fe7e57..da0140076 100644
>>> --- a/src/lua/utils.h
>>> +++ b/src/lua/utils.h
>>> @@ -554,6 +554,41 @@ luaL_iscallable(lua_State *L, int idx);
>>>  struct lua_State *
>>>  luaT_newthread(struct lua_State *L);
>>>  
>>> +/**
>>> + * Get a temporary Lua state.
>>> + *
>>> + * Use case: a function does not accept a Lua state as an argument
>>> + * to allow using from C code, but uses a Lua value, which is
>>> + * referenced in LUA_REGISTRYINDEX. A temporary Lua stack is needed
>>> + * to get and process the value.
>>> + *
>>> + * The resulting Lua state has a separate Lua stack, but the same
>>> + * globals and registry as `tarantool_L` (and all Lua states in
>>
>> 1. Users don't know what is tarantool_L.
> 
> Note: It is <luaT_state>() in the module API.

Nice, didn't know that either. But still the comment is invalid.

  reply	other threads:[~2020-09-29 23:21 UTC|newest]

Thread overview: 64+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-24 17:00 [Tarantool-patches] [PATCH 2.X 0/7] RFC: module api: extend for external merger Lua module Timur Safin
2020-09-24 17:00 ` [Tarantool-patches] [PATCH 2.X 1/7] module api: export box_tuple_validate Timur Safin
2020-09-28 22:20   ` Vladislav Shpilevoy
2020-10-02 12:24     ` Timur Safin
2020-10-09  1:11     ` Alexander Turenko
2020-10-09 20:11       ` Vladislav Shpilevoy
2020-10-10  1:19         ` Alexander Turenko
2020-09-29  5:25   ` Alexander Turenko
2020-10-05  7:35     ` Alexander Turenko
2020-09-24 17:00 ` [Tarantool-patches] [PATCH 2.X 2/7] module api: export box_key_def_dup Timur Safin
2020-09-28 22:21   ` Vladislav Shpilevoy
2020-09-29  5:03     ` Alexander Turenko
2020-09-29 23:19       ` Vladislav Shpilevoy
2020-10-01  3:05         ` Alexander Turenko
2020-10-02 12:25           ` Timur Safin
2020-10-02 12:26     ` Timur Safin
2020-09-24 17:00 ` [Tarantool-patches] [PATCH 2.X 3/7] module api: luaT_newthread Timur Safin
2020-09-28 22:21   ` Vladislav Shpilevoy
2020-10-02 12:27     ` Timur Safin
2020-10-02 21:48       ` Vladislav Shpilevoy
2020-09-29  6:25   ` Alexander Turenko
2020-10-02 12:26     ` Timur Safin
2020-10-02 12:53       ` Alexander Turenko
2020-09-29 15:19   ` Igor Munkin
2020-10-02 16:12     ` Timur Safin
2020-10-03 16:57       ` Igor Munkin
2020-09-24 17:00 ` [Tarantool-patches] [PATCH 2.X 4/7] module api: luaL_register_module & luaL_register_type Timur Safin
2020-09-28 22:21   ` Vladislav Shpilevoy
2020-09-29  5:09     ` Alexander Turenko
2020-09-29 23:20       ` Vladislav Shpilevoy
2020-09-30  6:31         ` Alexander Turenko
2020-09-30  6:33           ` Alexander Turenko
2020-10-02 16:14       ` Timur Safin
2020-09-29  8:03     ` Igor Munkin
2020-09-29 23:21       ` Vladislav Shpilevoy
2020-10-02 16:14       ` Timur Safin
2020-10-03  3:24         ` Alexander Turenko
2020-09-24 17:00 ` [Tarantool-patches] [PATCH 2.X 5/7] module api: luaT_temp_luastate & luaT_release_temp_luastate Timur Safin
2020-09-28 22:21   ` Vladislav Shpilevoy
2020-09-29  5:17     ` Alexander Turenko
2020-09-29 23:21       ` Vladislav Shpilevoy [this message]
2020-10-01  3:35         ` Alexander Turenko
2020-09-29 15:10     ` Igor Munkin
2020-09-29 21:03       ` Alexander Turenko
2020-09-29 23:23         ` Vladislav Shpilevoy
2020-09-30 10:09           ` Alexander Turenko
2020-10-01 15:06             ` Igor Munkin
2020-10-03  2:16         ` Alexander Turenko
2020-10-02 12:24       ` Timur Safin
2020-09-24 17:00 ` [Tarantool-patches] [PATCH 2.X 6/7] module api: luaL_checkibuf & luaL_checkconstchar Timur Safin
2020-09-28 22:21   ` Vladislav Shpilevoy
2020-09-29  5:53     ` Alexander Turenko
2020-09-29 23:25       ` Vladislav Shpilevoy
2020-10-01  3:00         ` Alexander Turenko
2020-10-02 16:14           ` Timur Safin
2020-10-02 21:48             ` Vladislav Shpilevoy
2020-10-08 13:50           ` Timur Safin
2020-09-24 17:00 ` [Tarantool-patches] [PATCH 2.X 7/7] module api: luaL_cdata_iscallable Timur Safin
2020-09-28 22:21   ` Vladislav Shpilevoy
2020-09-29  5:19     ` Alexander Turenko
2020-10-02 16:14     ` Timur Safin
2020-10-02 12:23 ` [Tarantool-patches] [PATCH 2.X 0/7] RFC: module api: extend for external merger Lua module Timur Safin
2020-10-02 21:49   ` Vladislav Shpilevoy
2020-10-03  2:54   ` Alexander Turenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0dbdbece-3f14-ba6d-b7ca-50f39f3b46d6@tarantool.org \
    --to=v.shpilevoy@tarantool.org \
    --cc=alexander.turenko@tarantool.org \
    --cc=tarantool-patches@dev.tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH 2.X 5/7] module api: luaT_temp_luastate & luaT_release_temp_luastate' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox