From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng1.m.smailru.net (smtpng1.m.smailru.net [94.100.181.251]) (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 DECE746971A for ; Thu, 12 Dec 2019 11:23:15 +0300 (MSK) References: <156ce93b495648d6f3fd6c879b0d9aaf56754a1e.1574773773.git.lvasiliev@tarantool.org> <20191211222134.swzcqcbpn4tbg64m@tkn_work_nb> From: Leonid Vasiliev Message-ID: Date: Thu, 12 Dec 2019 11:23:14 +0300 MIME-Version: 1.0 In-Reply-To: <20191211222134.swzcqcbpn4tbg64m@tkn_work_nb> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Tarantool-patches] [PATCH] Move txn from shema to a separate module (use C API instead of FFI) List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Turenko Cc: tarantool-patches@dev.tarantool.org On 12/12/19 1:21 AM, Alexander Turenko wrote: > Kostya, thanks for bringing the FFI / Lua-C question into our view. > > ---- > > We decided to don't change existing convention: let's use FFI when > possible and Lua-C API for functions that access a Lua state somehow > (including touching tarantool_L, yielding and running triggers / > callbacks). > > The core reason is that FFI allows to produce longer JIT traces that > should positively impact performance of an application at whole. > > We have no good understanding how performant Lua-C API with trace > stitching, however simple benchmarks (shared by Leonid) shows positive > impact of using FFI calls. > > We have no interpretation for those benchmarks for now. Whether Lua-C > runs were JITed at all? Whether a result will be different if there will > be more JITable code around? So for now things looks just as 'FFI is > faster the Lua-C'. Hope we'll investigate this later. > > We need to find a way to reduce probability that a function that is > called via ffi (or one that is called from it) will be changed in a > future and starts to yield or touch a Lua state. > > We should look whether we can mark functions that are called from ffi > (see also [1]) and all its callee to at least verify ourself when > changing code. > > Also we can work on detection of so called ffi sandwitches and doing > more stress testing. > > It seems that both directions are worth to dig into. > > [1]: https://github.com/tarantool/tarantool/issues/4202 > > ---- > > Back to the patch. > > Since we'll just move one function, I would not perform refactoring I > proposed initially: don't move all related functions together. Let's > rewrite box.rollback_to_savepoint() using Lua-C API and place it together > with box.commit() and box.rollback() to src/box/lua/init.c > > I think we don't need any pure Lua wrapper around this function: we can > access a Lua table field and raise an error just from C, so I don't see > a reason to split a function logic around two files > (src/box/lua/schema.lua and src/box/lua/init.c). They are both about API > we expose to Lua. > > WBR, Alexander Turenko. > > On Tue, Nov 26, 2019 at 04:13:43PM +0300, Leonid wrote: >> https://github.com/tarantool/tarantool/issues/4427 >> https://github.com/tarantool/tarantool/tree/lvasiliev/gh-4427-move-some-stuff-from-ffi-to-c-api >> >> --- >> src/box/CMakeLists.txt | 2 + >> src/box/lua/init.c | 4 + >> src/box/lua/schema.lua | 71 ----------------- >> src/box/lua/txn.c | 145 +++++++++++++++++++++++++++++++++++ >> src/box/lua/txn.h | 49 ++++++++++++ >> src/box/lua/txn.lua | 53 +++++++++++++ >> src/box/txn.c | 2 + >> test/box/misc.result | 1 + >> test/engine/savepoint.result | 12 +-- >> 9 files changed, 262 insertions(+), 77 deletions(-) >> create mode 100644 src/box/lua/txn.c >> create mode 100644 src/box/lua/txn.h >> create mode 100644 src/box/lua/txn.lua > >> --- box.commit yields, so it's defined as Lua/C binding >> --- box.rollback yields as well > > Let's move box.rollback_to_savepoint() to them. > I think to have a separate module for txn is a good idea. IMHO a true way use FFI for begin and is_in_txn and use C API for create a savepoint and rollback. Really, it's more consistent in my mind. What do you think about it?