Re: [Tarantool-patches] [PATCH] Move txn from shema to a separate module (use C API instead of FFI)

[Alexander Turenko <alexander.turenko@tarantool.org>]

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.