[Tarantool-patches] [PATCH v2 3/3] fiber: keep reference to userdata if fiber created once
Oleg Babin
olegrok at tarantool.org
Fri Sep 3 23:48:58 MSK 2021
Thanks for your review. See my answers below.
On 03.09.2021 15:23, Igor Munkin wrote:
> 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.
I've found that there is lua_nil_ref value in our code. For my purpose
I've added lua_no_ref.
Incremental diff will be below. Also I will send new series since this
change was done in separate patch.
>
>> #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.
Removed
>
>> + 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>.
Fixed.
>> + if (ref == LUA_NOREF) {
> <snipped>
>
>> --
>> 2.32.0
>>
diff --git a/src/lib/core/fiber.c b/src/lib/core/fiber.c
index 39b67f940..1e5b2fb80 100644
--- a/src/lib/core/fiber.c
+++ b/src/lib/core/fiber.c
@@ -32,7 +32,6 @@
#include <trivia/config.h>
#include <trivia/util.h>
-#include <lauxlib.h>
#include <errno.h>
#include <stdio.h>
#include <stdlib.h>
@@ -45,6 +44,7 @@
#include "errinj.h"
extern void cord_on_yield(void);
+extern int luaL_no_ref;
#if ENABLE_FIBER_TOP
#include <x86intrin.h> /* __rdtscp() */
@@ -896,8 +896,8 @@ fiber_recycle(struct fiber *fiber)
fiber->f = NULL;
fiber->wait_pad = NULL;
memset(&fiber->storage, 0, sizeof(fiber->storage));
- fiber->storage.lua.storage_ref = LUA_NOREF;
- fiber->storage.lua.fid_ref = LUA_NOREF;
+ fiber->storage.lua.storage_ref = luaL_no_ref;
+ fiber->storage.lua.fid_ref = luaL_no_ref;
unregister_fid(fiber);
fiber->fid = 0;
region_free(&fiber->gc);
@@ -1239,8 +1239,8 @@ fiber_new_ex(const char *name, const struct
fiber_attr *fiber_attr,
return NULL;
}
memset(fiber, 0, sizeof(struct fiber));
- fiber->storage.lua.storage_ref = LUA_NOREF;
- fiber->storage.lua.fid_ref = LUA_NOREF;
+ fiber->storage.lua.storage_ref = luaL_no_ref;
+ fiber->storage.lua.fid_ref = luaL_no_ref;
if (fiber_stack_create(fiber, &cord()->slabc,
fiber_attr->stack_size)) {
diff --git a/src/lua/fiber.c b/src/lua/fiber.c
index f82f4032f..70addd8f6 100644
--- a/src/lua/fiber.c
+++ b/src/lua/fiber.c
@@ -100,11 +100,9 @@ static int
lbox_fiber_on_stop(struct trigger *trigger, void *event)
{
struct fiber *f = event;
- int storage_ref = f->storage.lua.storage_ref;
- luaL_unref(tarantool_L, LUA_REGISTRYINDEX, storage_ref);
+ luaL_unref(tarantool_L, LUA_REGISTRYINDEX, f->storage.lua.storage_ref);
f->storage.lua.storage_ref = LUA_NOREF;
- int ref = f->storage.lua.fid_ref;
- luaL_unref(tarantool_L, LUA_REGISTRYINDEX, ref);
+ luaL_unref(tarantool_L, LUA_REGISTRYINDEX, f->storage.lua.fid_ref);
f->storage.lua.fid_ref = LUA_NOREF;
trigger_clear(trigger);
free(trigger);
@@ -117,8 +115,8 @@ lbox_fiber_on_stop(struct trigger *trigger, void *event)
static void
lbox_pushfiber(struct lua_State *L, struct fiber *f)
{
- int ref = f->storage.lua.fid_ref;
- if (ref == LUA_NOREF) {
+ int fid_ref = f->storage.lua.fid_ref;
+ if (fid_ref == LUA_NOREF) {
struct trigger *t = malloc(sizeof(*t));
if (t == NULL) {
diag_set(OutOfMemory, sizeof(*t), "malloc", "t");
@@ -133,10 +131,10 @@ lbox_pushfiber(struct lua_State *L, struct fiber *f)
*ptr = fid;
luaL_getmetatable(L, fiberlib_name);
lua_setmetatable(L, -2);
- ref = luaL_ref(L, LUA_REGISTRYINDEX);
- f->storage.lua.fid_ref = ref;
+ fid_ref = luaL_ref(L, LUA_REGISTRYINDEX);
+ f->storage.lua.fid_ref = fid_ref;
}
- lua_rawgeti(L, LUA_REGISTRYINDEX, ref);
+ lua_rawgeti(L, LUA_REGISTRYINDEX, fid_ref);
}
static struct fiber *
diff --git a/src/lua/utils.c b/src/lua/utils.c
index c71cd4857..c252d7cdf 100644
--- a/src/lua/utils.c
+++ b/src/lua/utils.c
@@ -40,6 +40,7 @@
#include "uuid/tt_uuid.h"
int luaL_nil_ref = LUA_REFNIL;
+int luaL_no_ref = LUA_NOREF;
static int luaT_newthread_ref = LUA_NOREF;
diff --git a/test/unit/core_test_utils.c b/test/unit/core_test_utils.c
index 23452bbfd..9b1789e75 100644
--- a/test/unit/core_test_utils.c
+++ b/test/unit/core_test_utils.c
@@ -35,3 +35,5 @@
void cord_on_yield(void)
{
}
+
+int luaL_no_ref = -2;
More information about the Tarantool-patches
mailing list