From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org> To: tarantool-patches@freelists.org, Konstantin Osipov <kostja@tarantool.org> Subject: [tarantool-patches] Re: [PATCH v3 3/4] session: introduce binary box.session.push Date: Thu, 7 Jun 2018 20:02:35 +0300 [thread overview] Message-ID: <3c142fdd-fb7b-bd60-4615-f9152fd8b8c9@tarantool.org> (raw) In-Reply-To: <20180607125352.GA30262@chai> On 07/06/2018 15:53, Konstantin Osipov wrote: > Hi, > > My code review for this patch is on branch > box-session-push-kostja. > > What still needs to be done: > > - convert push.test to box-tap I have removed assertions and left the test be output checking. Push test does many table members checking, and it is simpler to just output the tables. Assertions were in a single place were a table contains 400 members. But ok, now I did it like in vinyl/select_consistency test. > - think about alternatives which would preserve sync in a Lua > routine. Looks like using an upvalue would do > https://www.lua.org/pil/27.3.3.html I had investigated it before you have recommended and before I started the patch. But ok, I have investigated it again, and wrote the simple patch, that shows upvalues to be unusable as sync storage. See the diff below and the explanation under the diff. +++ b/src/box/lua/call.c @@ -434,8 +434,9 @@ box_process_lua(struct call_request *request, struct port *base, - lua_pushcfunction(L, handler); + uint64_t sync = request->header->sync; + lua_pushinteger(L, sync); + lua_pushcclosure(L, handler, 1); +++ b/src/box/lua/session.c @@ -370,6 +370,9 @@ lbox_session_push(struct lua_State *L) + uint64_t sync = lua_tointeger(L, lua_upvalueindex(1)); + uint64_t f_sync = fiber_sync(fiber()); + say_info("%llu, %llu", (unsigned long long) sync, f_sync); if (session_push(current_session(), &port) != 0) { After running push.test.lua I found say_info() printing always 0 + real sync. So the upvalue is actually nil here. C upvalues are not the same as Lua upvalues. C upvalue can be associated with a single function only, while the Lua ones can be referenced in multiple and enclosed functions. C upvalue can be accessed only from the function owning it. In the diff above I have pushed sync as an upvalue and tried to get it in lbox_session_push function. But the upvalue was not available. Lets look at the implementation of lua_tointeger(L, lua_upvalueindex(1)). lua_upvalueindex(idx): #define LUA_GLOBALSINDEX (-10002) #define lua_upvalueindex(i) (LUA_GLOBALSINDEX-(i)) Nothing is interesting. Upvalue index is just an index. lua_tointeger(L, idx): LUA_API lua_Integer lua_tointeger(lua_State *L, int idx) { cTValue *o = index2adr(L, idx); ... index2addr(L, idx): .... } else { GCfunc *fn = curr_func(L); api_check(L, fn->c.gct == ~LJ_TFUNC && !isluafunc(fn)); if (idx == LUA_ENVIRONINDEX) { TValue *o = &G(L)->tmptv; settabV(L, o, tabref(fn->c.env)); return o; } else { idx = LUA_GLOBALSINDEX - idx; return idx <= fn->c.nupvalues ? &fn->c.upvalue[idx-1] : niltv(L); } } As you see, upvalue array is got from curr_func(L). lbox_session_push != C closure I had pushed earlier. And it has no upvalues. Summary: C upvalues can not be used as a sync storage.
next prev parent reply other threads:[~2018-06-07 17:02 UTC|newest] Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-06-01 20:55 [tarantool-patches] [PATCH v3 0/4] box.session.push Vladislav Shpilevoy 2018-06-01 20:55 ` [tarantool-patches] [PATCH v3 1/4] session: introduce text box.session.push Vladislav Shpilevoy 2018-06-01 20:55 ` [tarantool-patches] [PATCH v3 2/4] session: enable box.session.push in local console Vladislav Shpilevoy 2018-06-01 20:55 ` [tarantool-patches] [PATCH v3 3/4] session: introduce binary box.session.push Vladislav Shpilevoy 2018-06-07 12:53 ` [tarantool-patches] " Konstantin Osipov 2018-06-07 17:02 ` Vladislav Shpilevoy [this message] 2018-06-08 3:51 ` Konstantin Osipov 2018-06-01 20:55 ` [tarantool-patches] [PATCH v3 4/4] netbox: introduce iterable future objects Vladislav Shpilevoy 2018-06-07 12:56 ` [tarantool-patches] " Konstantin Osipov 2018-06-07 17:02 ` Vladislav Shpilevoy 2018-06-08 3:52 ` Konstantin Osipov 2018-06-08 14:20 ` Vladislav Shpilevoy
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=3c142fdd-fb7b-bd60-4615-f9152fd8b8c9@tarantool.org \ --to=v.shpilevoy@tarantool.org \ --cc=kostja@tarantool.org \ --cc=tarantool-patches@freelists.org \ --subject='[tarantool-patches] Re: [PATCH v3 3/4] session: introduce binary box.session.push' \ /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