Tarantool development patches archive
 help / color / mirror / Atom feed
From: Igor Munkin <imun@tarantool.org>
To: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>,
	Sergey Ostanevich <sergos@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH] lua: prohibit fiber yield when GC hook is active
Date: Fri, 2 Oct 2020 17:49:29 +0300	[thread overview]
Message-ID: <20201002144929.GI18920@tarantool.org> (raw)
In-Reply-To: <61072ba98da2e4098b0c35cad83ffcdcbc537c8e.1601644669.git.imun@tarantool.org>

Sorry, forgot branch, issues and ChangeLog entry. Too fast, too furious.

On 02.10.20, Igor Munkin wrote:
> While running GC hook (i.e. __gc  metamethod) garbage collector engine
> is "stopped": the memory penalty threshold is set to LJ_MAX_MEM and
> incremental GC step is not triggered as a result. Ergo, yielding the
> execution at the finalizer body leads to further running platform with
> disabled LuaJIT GC. It is not re-enabled until the yielded fiber doesn't
> get the execution back.
> 
> This changeset extends <cord_on_yield> routine with the check whether GC
> hook is active. If the switch-over occurs in scope of __gc metamethod
> the platform is forced to stop its execution with EXIT_FAILURE and calls
> panic routine before the exit.
> 
> Relates to #4518
> Follows up #4727
> 
> Signed-off-by: Igor Munkin <imun@tarantool.org>
> ---

@ChangeLog:
* Fixed fibers switch-over to prevent implicit GC disabling. The yield
  occurring while user-defined __gc metatmethod is running leads to the
  platform panic.

Branch: https://github.com/tarantool/tarantool/tree/imun/gh-4518-forbid-yield-in-GC-finalizer
Issues:
* https://github.com/tarantool/tarantool/issues/4518
* https://github.com/tarantool/tarantool/issues/4727

> 
> Vlad introduced the internal interface and local internal background
> fiber in scope of 8443bd9 ("fiber: introduce schedule_task() internal
> function") to postpone any yielding finalization (e.g. 3d5b4da ("fio:
> close unused descriptors automatically") and f073834 ("swim: use
> fiber._internal.schedule_task() for GC")). After this patch is merged we
> need to update docs and provide users a correct scenario to detect and
> fix yielding finalizers.
> 

Benchmark is the same[1].

> Here are also the benchmark results for the Release build:
> * Vanilla (2711797) -> Patched (61072ba) (min, median, mean, max):
> | fibers: 10; iters: 100	0%	0%	0%	2%
> | fibers: 10; iters: 1000	-3%	0%	0%	1%
> | fibers: 10; iters: 10000	-3%	0%	-1%	-2%
> | fibers: 10; iters: 100000	0%	0%	0%	1%
> | fibers: 100; iters: 100	-1%	-2%	-2%	-9%
> | fibers: 100; iters: 1000	0%	0%	0%	5%
> | fibers: 100; iters: 10000	0%	0%	0%	0%
> | fibers: 100; iters: 100000	0%	0%	0%	-3%
> | fibers: 1000; iters: 100	0%	0%	0%	0%
> | fibers: 1000; iters: 1000	0%	0%	0%	0%
> | fibers: 1000; iters: 10000	0%	0%	0%	1%
> | fibers: 1000; iters: 100000	0%	0%	0%	2%
> | fibers: 10000; iters: 100	0%	-3%	-1%	-2%
> | fibers: 10000; iters: 1000	0%	-2%	-3%	-2%
> | fibers: 10000; iters: 10000	0%	0%	0%	-2%
> | fibers: 10000; iters: 100000	0%	-3%	-3%	-6%
> 
>  src/lua/utils.c                             | 26 ++++++++++++++-
>  test/app-tap/yield-in-gc-finalizer.test.lua | 36 +++++++++++++++++++++
>  2 files changed, 61 insertions(+), 1 deletion(-)
>  create mode 100755 test/app-tap/yield-in-gc-finalizer.test.lua
> 

<snipped>

> -- 
> 2.25.0
> 

[1]: https://gist.github.com/igormunkin/7e0cf48005bd003ffbdf30181eedb40e

-- 
Best regards,
IM

  reply	other threads:[~2020-10-02 15:00 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-02 14:39 Igor Munkin
2020-10-02 14:49 ` Igor Munkin [this message]
2020-10-04 14:02 ` Vladislav Shpilevoy
2020-10-04 18:51   ` Igor Munkin
2020-10-05 18:37 ` Sergey Ostanevich
2020-10-05 18:39   ` Igor Munkin
2020-10-05 18:43 ` Igor Munkin
2020-10-06 10:56 ` Kirill Yukhin

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=20201002144929.GI18920@tarantool.org \
    --to=imun@tarantool.org \
    --cc=sergos@tarantool.org \
    --cc=tarantool-patches@dev.tarantool.org \
    --cc=v.shpilevoy@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH] lua: prohibit fiber yield when GC hook is active' \
    /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