Tarantool development patches archive
 help / color / mirror / Atom feed
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
To: tarantool-patches@freelists.org
Cc: alexander.turenko@tarantool.org
Subject: [tarantool-patches] [PATCH v2 3/4] tuple: use global msgpack serializer in Lua tuple
Date: Mon,  9 Sep 2019 21:00:09 +0200	[thread overview]
Message-ID: <98110cbc514cb13f3ba3b298ed5b6a493fcc36e5.1568055477.git.v.shpilevoy@tarantool.org> (raw)
In-Reply-To: <cover.1568055477.git.v.shpilevoy@tarantool.org>

Tuple is a C library exposed to Lua. In Lua to translate Lua
objects into tuples and back luaL_serializer structure is used.

In Tarantool we have several global serializers, one of which is
for msgpack. Tuples store data in msgpack, and in theory should
have used that global msgpack serializer. But in fact the tuple
module had its own private serializer because of tuples encoding
specifics such as never encode sparse arrays as maps.

This patch makes tuple Lua module use global msgpack serializer
always. But how does tuple handle sparse arrays now? In fact,
the tuple module still has its own serializer, but it is updated
each time when the msgpack serializer is changed.

Part of #4434
---
 src/box/lua/tuple.c        | 33 ++++++++++++++++++++-------
 src/lua/utils.c            | 12 ++++++++--
 src/lua/utils.h            | 26 +++++++++++++++++++++
 test/box/tuple.result      | 46 ++++++++++++++++++++++++++++++++++++++
 test/box/tuple.test.lua    | 21 +++++++++++++++++
 test/unit/luaT_tuple_new.c |  2 +-
 6 files changed, 129 insertions(+), 11 deletions(-)

diff --git a/src/box/lua/tuple.c b/src/box/lua/tuple.c
index 3902288bf..fc22f5572 100644
--- a/src/box/lua/tuple.c
+++ b/src/box/lua/tuple.c
@@ -58,6 +58,11 @@
 
 static const char *tuplelib_name = "box.tuple";
 static const char *tuple_iteratorlib_name = "box.tuple.iterator";
+/*
+ * Special serializer for box.tuple.new() to disable storage
+ * optimization for excessively sparse arrays as a tuple always
+ * must be regular MP_ARRAY.
+ */
 static struct luaL_serializer tuple_serializer;
 
 extern char tuple_lua[]; /* Lua source */
@@ -548,6 +553,21 @@ static const struct luaL_Reg lbox_tuple_iterator_meta[] = {
 
 /* }}} */
 
+static inline void
+tuple_serializer_fill(void)
+{
+	luaL_serializer_copy_options(&tuple_serializer, luaL_msgpack_default);
+	tuple_serializer.encode_sparse_ratio = 0;
+}
+
+static void
+on_msgpack_serializer_update(struct trigger *trigger, void *event)
+{
+	(void) trigger;
+	(void) event;
+	tuple_serializer_fill();
+}
+
 void
 box_lua_tuple_init(struct lua_State *L)
 {
@@ -564,14 +584,11 @@ box_lua_tuple_init(struct lua_State *L)
 
 	luamp_set_encode_extension(luamp_encode_extension_box);
 
-	/*
-	 * Create special serializer for box.tuple.new().
-	 * Disable storage optimization for excessively
-	 * sparse arrays as a tuple always must be regular
-	 * MP_ARRAY.
-	 */
-	luaL_serializer_create(&tuple_serializer);
-	tuple_serializer.encode_sparse_ratio = 0;
+	tuple_serializer_fill();
+	trigger_create(&tuple_serializer.update_trigger,
+		       on_msgpack_serializer_update, NULL, NULL);
+	trigger_add(&luaL_msgpack_default->on_update,
+		    &tuple_serializer.update_trigger);
 
 	/* Get CTypeID for `struct tuple' */
 	int rc = luaL_cdef(L, "struct tuple;");
diff --git a/src/lua/utils.c b/src/lua/utils.c
index a082a2e5b..b5cefd07f 100644
--- a/src/lua/utils.c
+++ b/src/lua/utils.c
@@ -251,12 +251,20 @@ static struct {
 void
 luaL_serializer_create(struct luaL_serializer *cfg)
 {
+	rlist_create(&cfg->on_update);
 	for (int i = 0; OPTIONS[i].name != NULL; i++) {
 		int *pval = (int *) ((char *) cfg + OPTIONS[i].offset);
 		*pval = OPTIONS[i].defvalue;
 	}
 }
 
+void
+luaL_serializer_copy_options(struct luaL_serializer *dst,
+			     const struct luaL_serializer *src)
+{
+	memcpy(dst, src, offsetof(struct luaL_serializer, end_of_options));
+}
+
 /**
  * Configure one field in @a cfg.
  * @param L Lua stack.
@@ -319,6 +327,7 @@ luaL_serializer_cfg(struct lua_State *L)
 		else
 			lua_setfield(L, 1, OPTIONS[i].name);
 	}
+	trigger_run(&cfg->on_update, cfg);
 	return 0;
 }
 
@@ -342,7 +351,7 @@ luaL_newserializer(struct lua_State *L, const char *modname, const luaL_Reg *reg
 			lua_newuserdata(L, sizeof(*serializer));
 	luaL_getmetatable(L, LUAL_SERIALIZER);
 	lua_setmetatable(L, -2);
-	memset(serializer, 0, sizeof(*serializer));
+	luaL_serializer_create(serializer);
 
 	for (; reg->name != NULL; reg++) {
 		/* push luaL_serializer as upvalue */
@@ -362,7 +371,6 @@ luaL_newserializer(struct lua_State *L, const char *modname, const luaL_Reg *reg
 	/* Save configuration values to serializer.cfg */
 	for (int i = 0; OPTIONS[i].name != NULL; i++) {
 		int *pval = (int *) ((char *) serializer + OPTIONS[i].offset);
-		*pval = OPTIONS[i].defvalue;
 		switch (OPTIONS[i].type) {
 		case LUA_TBOOLEAN:
 			lua_pushboolean(L, *pval);
diff --git a/src/lua/utils.h b/src/lua/utils.h
index d42cc3992..e9c316d22 100644
--- a/src/lua/utils.h
+++ b/src/lua/utils.h
@@ -36,6 +36,7 @@
 #include <math.h> /* modf, isfinite */
 
 #include <msgpuck.h> /* enum mp_type */
+#include "trigger.h"
 
 #if defined(__cplusplus)
 extern "C" {
@@ -242,6 +243,23 @@ struct luaL_serializer {
 
 	/** Enable support for compact represenation (internal, YAML-only). */
 	int has_compact;
+	/**
+	 * Border where copyable fields end. Is used to copy
+	 * serializer options into an existing serializer without
+	 * erasure of its non-option fields like triggers.
+	 */
+	char end_of_options[0];
+	/**
+	 * Trigger object to subscribe on updates of a more
+	 * general serializer. For example, tuple serializer
+	 * subscribes on msgpack.
+	 */
+	struct trigger update_trigger;
+	/**
+	 * List of triggers on update of this serializer. To push
+	 * updates down to dependent serializers.
+	 */
+	struct rlist on_update;
 };
 
 extern int luaL_nil_ref;
@@ -254,6 +272,14 @@ extern int luaL_array_metatable_ref;
 struct luaL_serializer *
 luaL_newserializer(struct lua_State *L, const char *modname, const luaL_Reg *reg);
 
+/**
+ * Copy all option fields of @a src into @a dst. Other fields,
+ * such as triggers, are not touched.
+ */
+void
+luaL_serializer_copy_options(struct luaL_serializer *dst,
+			     const struct luaL_serializer *src);
+
 static inline struct luaL_serializer *
 luaL_checkserializer(struct lua_State *L) {
 	return (struct luaL_serializer *)
diff --git a/test/box/tuple.result b/test/box/tuple.result
index 895462518..83f74d111 100644
--- a/test/box/tuple.result
+++ b/test/box/tuple.result
@@ -1395,3 +1395,49 @@ d:update{{'-', 1, dec.new('1e37')}}
 ---
 - [80000000000000000000000000000000000000]
 ...
+--
+-- gh-4434: tuple should use global msgpack serializer.
+--
+max_depth = msgpack.cfg.encode_max_depth
+---
+...
+t = nil
+---
+...
+for i = 1, max_depth + 5 do t = {t} end
+---
+...
+tuple = box.tuple.new(t):totable()
+---
+...
+level = 0
+---
+...
+while tuple ~= nil do level = level + 1 tuple = tuple[1] end
+---
+...
+level == max_depth or {level, max_depth}
+---
+- true
+...
+msgpack.cfg({encode_max_depth = max_depth + 5})
+---
+...
+tuple = box.tuple.new(t):totable()
+---
+...
+level = 0
+---
+...
+while tuple ~= nil do level = level + 1 tuple = tuple[1] end
+---
+...
+-- Level should be bigger now, because the default msgpack
+-- serializer allows deeper tables.
+level == max_depth + 5 or {level, max_depth}
+---
+- true
+...
+msgpack.cfg({encode_max_depth = max_depth})
+---
+...
diff --git a/test/box/tuple.test.lua b/test/box/tuple.test.lua
index 9762fc8b3..c8a0c03c5 100644
--- a/test/box/tuple.test.lua
+++ b/test/box/tuple.test.lua
@@ -473,3 +473,24 @@ d = box.tuple.new(dec.new('9e37'))
 d
 d:update{{'+', 1, dec.new('1e37')}}
 d:update{{'-', 1, dec.new('1e37')}}
+
+--
+-- gh-4434: tuple should use global msgpack serializer.
+--
+max_depth = msgpack.cfg.encode_max_depth
+t = nil
+for i = 1, max_depth + 5 do t = {t} end
+tuple = box.tuple.new(t):totable()
+level = 0
+while tuple ~= nil do level = level + 1 tuple = tuple[1] end
+level == max_depth or {level, max_depth}
+
+msgpack.cfg({encode_max_depth = max_depth + 5})
+tuple = box.tuple.new(t):totable()
+level = 0
+while tuple ~= nil do level = level + 1 tuple = tuple[1] end
+-- Level should be bigger now, because the default msgpack
+-- serializer allows deeper tables.
+level == max_depth + 5 or {level, max_depth}
+
+msgpack.cfg({encode_max_depth = max_depth})
diff --git a/test/unit/luaT_tuple_new.c b/test/unit/luaT_tuple_new.c
index 609d64e45..8f25c8e07 100644
--- a/test/unit/luaT_tuple_new.c
+++ b/test/unit/luaT_tuple_new.c
@@ -170,8 +170,8 @@ main()
 	luaL_openlibs(L);
 
 	box_init();
-	box_lua_tuple_init(L);
 	luaopen_msgpack(L);
+	box_lua_tuple_init(L);
 	lua_pop(L, 1);
 
 	return test_basic(L);
-- 
2.20.1 (Apple Git-117)

  parent reply	other threads:[~2019-09-09 18:56 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-10 20:24 [tarantool-patches] [PATCH v2 0/4] Serializer bugs Vladislav Shpilevoy
2019-09-09 19:00 ` [tarantool-patches] [PATCH v2 1/4] app: serializers update now is reflected in Lua Vladislav Shpilevoy
2019-09-12 23:22   ` [tarantool-patches] " Alexander Turenko
2019-09-13 22:32     ` Vladislav Shpilevoy
2019-09-09 19:00 ` [tarantool-patches] [PATCH v2 2/4] msgpack: make msgpackffi use encode_max_depth option Vladislav Shpilevoy
2019-09-12 23:24   ` [tarantool-patches] " Alexander Turenko
2019-09-13 22:32     ` Vladislav Shpilevoy
2019-09-09 19:00 ` Vladislav Shpilevoy [this message]
2019-09-12 23:27   ` [tarantool-patches] Re: [PATCH v2 3/4] tuple: use global msgpack serializer in Lua tuple Alexander Turenko
2019-09-13 22:32     ` Vladislav Shpilevoy
2019-09-09 19:00 ` [tarantool-patches] [PATCH v2 4/4] app: allow to raise an error on too nested tables Vladislav Shpilevoy
2019-09-12 23:32   ` [tarantool-patches] " Alexander Turenko
2019-09-13 22:32     ` Vladislav Shpilevoy
2019-09-10 20:25 ` [tarantool-patches] Re: [PATCH v2 0/4] Serializer bugs Vladislav Shpilevoy
2019-09-12 23:44 ` Alexander Turenko
2019-09-13 22:32   ` Vladislav Shpilevoy

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=98110cbc514cb13f3ba3b298ed5b6a493fcc36e5.1568055477.git.v.shpilevoy@tarantool.org \
    --to=v.shpilevoy@tarantool.org \
    --cc=alexander.turenko@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --subject='Re: [tarantool-patches] [PATCH v2 3/4] tuple: use global msgpack serializer in Lua tuple' \
    /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