[Tarantool-patches] [PATCH v2 3/3] fiber: keep reference to userdata if fiber created once
Igor Munkin
imun at tarantool.org
Fri Sep 3 15:23:14 MSK 2021
Oleg,
Thanks for the fixes! I still have some comments regarding the new
version of this patch.
On 02.09.21, olegrok at tarantool.org wrote:
> From: Oleg Babin <babinoleg at mail.ru>
>
> This patch reworks approach to fiber management in Lua. Before
> this patch each action that should return fiber led to new
> userdata creation that was quite slow and made GC suffer. This
> patch introduces new field in struct fiber to store a reference to
> userdata that was created once for a fiber. It allows speedup
> operations as fiber.self() and fiber.id().
> Simple benchmark shows that access to fiber storage is faster in
> two times, fiber.find() - 2-3 times and fiber.new/create functions
> don't have any changes.
> ---
> src/lib/core/fiber.c | 5 ++
> src/lib/core/fiber.h | 5 ++
> src/lua/fiber.c | 113 +++++++++++++------------------------------
> 3 files changed, 44 insertions(+), 79 deletions(-)
>
> diff --git a/src/lib/core/fiber.c b/src/lib/core/fiber.c
> index 588b78504..39b67f940 100644
> --- a/src/lib/core/fiber.c
> +++ b/src/lib/core/fiber.c
> @@ -32,6 +32,7 @@
>
> #include <trivia/config.h>
> #include <trivia/util.h>
> +#include <lauxlib.h>
Totally agree with Vlad here. I guess you have two options:
* Use the approach similar to <cord_on_yield>.
* Introduce a constant in src/lib/core/fiber.h and add a static assert
in src/lua/utils.h whether this value is the same as in LuaJIT.
Maybe there are other options Vlad can suggest.
> #include <errno.h>
> #include <stdio.h>
> #include <stdlib.h>
<snipped>
> diff --git a/src/lua/fiber.c b/src/lua/fiber.c
> index 5575f2079..ab0e895eb 100644
> --- a/src/lua/fiber.c > +++ b/src/lua/fiber.c
> @@ -87,27 +87,28 @@ luaL_testcancel(struct lua_State *L)
<snipped>
> + struct fiber *f = event;
> + int storage_ref = f->storage.lua.storage_ref;
Minor: Why do you need both <storage_ref> and <ref> variables? After
your change they can be freely dropped.
> + luaL_unref(tarantool_L, LUA_REGISTRYINDEX, storage_ref);
> + f->storage.lua.storage_ref = LUA_NOREF;
> + int ref = f->storage.lua.fid_ref;
> + luaL_unref(tarantool_L, LUA_REGISTRYINDEX, ref);
> + f->storage.lua.fid_ref = LUA_NOREF;
> + trigger_clear(trigger);
> + free(trigger);
> + return 0;
> }
>
> /**
> @@ -116,42 +117,26 @@ lbox_create_weak_table(struct lua_State *L, const char *name)
<snipped>
> + int ref = f->storage.lua.fid_ref;
Minor: It would be nice also to rename <ref> to <fid_ref>.
> + if (ref == LUA_NOREF) {
<snipped>
> --
> 2.32.0
>
--
Best regards,
IM
More information about the Tarantool-patches
mailing list