Tarantool development patches archive
 help / color / mirror / Atom feed
From: Igor Munkin <imun@tarantool.org>
To: Konstantin Osipov <kostja.osipov@gmail.com>
Cc: tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH] Move txn from shema to a separate module (use C API instead of FFI)
Date: Thu, 28 Nov 2019 21:36:15 +0300	[thread overview]
Message-ID: <20191128183615.GC1214@tarantool.org> (raw)
In-Reply-To: <20191128155801.GB11584@atlas>

Kostja,

On 28.11.19, Konstantin Osipov wrote:
> * Igor Munkin <imun@tarantool.org> [19/11/28 17:08]:
> 
> > Why should we be aiming at using FFI more? The root cause is that
> > current fiber machinery (as well as some parts of triggers mechanism)
> > doesn't respect the Lua coroutine switch semantics, thereby breaking
> > trace recording. Lua-C API implicitly (or non-intentionally) prevents
> > breakage by JIT trace aborts when recording FUNCC.
> 
> It's not correct. The current FFI functions were carefully crafted
> to never lead to sandwich code: only those functions which can not
> trigger a return to Lua were implemented as FFI. 
> There was one regression between 1.10 and in 2.3 because we
> started firing rollback triggers when rolling back to a savepoint, 
> which was spotted by a failing tests.
> 
> One more time: When FFI bindings were written we were aware of NYI
> and took it into account.
> 

OK, maybe I said it the wrong way using the word "non-intentionally". I
mean that Tarantool doesn't use any special handler to asynchroniously
abort recording, since there is no such outside the LuaJIT internals
(jit.off blacklists the function regardless the host and guest stacks
layout).

> > Therefore, I guess we should be aiming either at changing fiber
> > switching to the one respecting the LuaJIT runtime or at tuning JIT
> > compiler way more regarding the Lua-C usage.
> 
> This is actually quite simple - we could easily call a LuaJIT hook
> whenever switching a fiber, to make sure that it carefully
> switches the internals as well. Mike Pall refused to cooperate on
> the matter, but now we (you) control our own destiny.

Unfortunately, I haven't seen the thread where the subj is discussed
with Mike Pall, but the approach you proposed doesn't seem to be a
convenient one, however it still solves a problem (as does the move to
use Lua-C API for the code with possible Lua VM re-entrance underneath).

The major flaw I see in this solution, is introducing the dependency on
the JIT interface into Tarantool internals. There is already one
dependency on LuaJIT-2.1.0 presented with internal headers usage for
several hacks in utils.c. As a result we are not able to simply replace
the Lua implementation to try another one (e.g. uJIT conforming
LuaJIT-2.0.5) for comparing each other.

The best proposal we had with Kirill and Sergos is to finalize a trace
exactly at CALLXS IR, however after some research I found that the
snapshot to be replayed at the corresponding trace exit will restore the
guest stack it doesn't relate to. I hope to make further research this
direction, but it requires a way more time to adjust this behaviour and
its benefits are doubtful for me now.

For now, there is a partial fix I mentioned before, however it still
violates the flow I described here[1]. I'm going to proceed with the
research a bit later and provide another patch.

> 
> > Besides, we can't fully prevent platform failures if there is an FFI
> > misusage in users code.
> 
> Tarantool has never been claiming that it prevents people from

Sorry, I simply misread the following:
|> Why not dig it up to protect from future erosion of the code base?
|>
|> This would be more valuable contribution than just falling back to
|> Lua/C for everything.

> shooting themselves in the foot. Performance is the ultimate
> design goal, at the cost of safety at times.
> 

Great, we discussed with Leonid and Sasha offline and agreed to make
several benchmarks to be provided in this thread. With no benchmarks all
our estimates can be simply wrong.

> 
> > > What should be the rule of thumb in your opinion, ffi or
> > > lua/c? 
> > 
> > If you want to know my rule of thumb: FFI is for external existing
> > libraries to be used in Lua code (and all compiler related benefits are
> > nothing more than a godsend consequence, since all guest stack
> > manipulations are implemented in LuaJIT runtime, not in an external
> > code) and Lua-C is a well-designed and well-documented API for embedding
> > Lua into a host application / extending Lua with external low-lewel
> > libs. I totally do not insist on my point of view, since everyone has
> > it's own vision on LuaJIT features.
> 
> OK, but there must be a single policy though. So far it was:
> everything that doesn't yield and doesn't call back to Lua
> uses FFI. Everything else *has* to use Lua/C API, UNTIL  there
> is a way to safely sandwich FFI calls.
> 

I agree with you for the policy existence, but we all see the one you
mentioned above can introduce bugs leading to a platform failures. So I
guess we should reconsider it or simply dump somewhere. I think we have
to make some benchmarks and provide not only stats, but also a
reproducer with the input data, otherwise JIT tests are IMHO irrelevant.

> -- 
> Konstantin Osipov, Moscow, Russia

[1]: https://github.com/tarantool/tarantool/issues/4427#issuecomment-546056302

-- 
Best regards,
IM

  reply	other threads:[~2019-11-28 18:38 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-26 13:13 Leonid
2019-11-26 21:05 ` Konstantin Osipov
2019-11-26 21:17   ` Alexander Turenko
2019-11-27  8:31     ` Konstantin Osipov
2019-11-28  8:10       ` Leonid Vasiliev
2019-11-28 12:34         ` Konstantin Osipov
2019-11-28 13:00           ` Igor Munkin
2019-11-28 13:18             ` Konstantin Osipov
2019-11-28 14:03               ` Igor Munkin
2019-11-28 15:58                 ` Konstantin Osipov
2019-11-28 18:36                   ` Igor Munkin [this message]
2019-11-29  5:30                     ` Konstantin Osipov
2019-11-29 17:43                       ` Igor Munkin
2019-11-29  5:41                     ` Konstantin Osipov
2019-11-29 17:37                       ` Igor Munkin
2019-12-04 13:05                     ` Leonid Vasiliev
2019-12-04 13:15                       ` Konstantin Osipov
2019-12-05  8:27                         ` Leonid Vasiliev
2020-03-20 18:48               ` Igor Munkin
2020-03-20 19:27                 ` Konstantin Osipov
2019-12-11 22:21 ` Alexander Turenko
2019-12-12  8:23   ` Leonid Vasiliev
2020-01-15 17:05     ` Alexander Turenko
2019-12-12  8:49   ` Konstantin Osipov

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=20191128183615.GC1214@tarantool.org \
    --to=imun@tarantool.org \
    --cc=kostja.osipov@gmail.com \
    --cc=tarantool-patches@dev.tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH] Move txn from shema to a separate module (use C API instead of FFI)' \
    /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