[tarantool-patches] Re: box: disable sparse optimization in box.tuple.new()
Vladislav Shpilevoy
v.shpilevoy at tarantool.org
Sat Feb 16 00:31:52 MSK 2019
LGTM.
On 15/02/2019 16:17, Kirill Shcherbatov wrote:
> Hi! I've accounted Kostya's proposal to implement separate serializer.
>
> ===================================================
>
> http://github.com/tarantool/tarantool/tree/kshch/gh-3882-disable-sparse-optimization
> https://github.com/tarantool/tarantool/issues/3882
>
> v1:
> https://www.freelists.org/post/tarantool-patches/box-disable-sparse-optimization-in-boxtuplenew
>
> The box.tuple.new() used to call luamp_encode_tuple with
> default LUA serializer config 'luaL_msgpack_default'. This
> routine may consider an array to be excessively sparse when
> + encode_sparse_ratio > 0
> + max(table) > encode_sparse_safe
> + max(table) > count(table) * encode_sparse_ratio.
> Sparse optimization save memory via representing excessively
> sparse tuple as MP_MAP. But Tarantool tuple always must be
> MP_ARRAY so it is not relevant for box.tuple.new semantics.
> So it is disabled with encode_sparse_ratio = 0 in a new local
> serializer config.
>
> Closes #3882
> ---
> src/box/lua/tuple.c | 12 +++++++++-
> src/lua/utils.c | 9 ++++++++
> src/lua/utils.h | 7 ++++++
> test/box/tuple.result | 50 +++++++++++++++++++++++++++++++++++++++++
> test/box/tuple.test.lua | 26 +++++++++++++++++++++
> 5 files changed, 103 insertions(+), 1 deletion(-)
>
> diff --git a/src/box/lua/tuple.c b/src/box/lua/tuple.c
> index 756856f4e..5f2bfc807 100644
> --- a/src/box/lua/tuple.c
> +++ b/src/box/lua/tuple.c
> @@ -58,6 +58,7 @@
>
> static const char *tuplelib_name = "box.tuple";
> static const char *tuple_iteratorlib_name = "box.tuple.iterator";
> +static struct luaL_serializer tuple_serializer;
>
> extern char tuple_lua[]; /* Lua source */
>
> @@ -109,7 +110,7 @@ lbox_tuple_new(lua_State *L)
>
> if (argc == 1 && (lua_istable(L, 1) || luaT_istuple(L, 1))) {
> /* New format: box.tuple.new({1, 2, 3}) */
> - luamp_encode_tuple(L, luaL_msgpack_default, &stream, 1);
> + luamp_encode_tuple(L, &tuple_serializer, &stream, 1);
> } else {
> /* Backward-compatible format: box.tuple.new(1, 2, 3). */
> mpstream_encode_array(&stream, argc);
> @@ -539,6 +540,15 @@ 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;
> +
> /* Get CTypeID for `struct tuple' */
> int rc = luaL_cdef(L, "struct tuple;");
> assert(rc == 0);
> diff --git a/src/lua/utils.c b/src/lua/utils.c
> index 978fe61f1..a418b9586 100644
> --- a/src/lua/utils.c
> +++ b/src/lua/utils.c
> @@ -213,6 +213,15 @@ static struct {
> { NULL, 0, 0, 0},
> };
>
> +void
> +luaL_serializer_create(struct luaL_serializer *cfg)
> +{
> + for (int i = 0; OPTIONS[i].name != NULL; i++) {
> + int *pval = (int *) ((char *) cfg + OPTIONS[i].offset);
> + *pval = OPTIONS[i].defvalue;
> + }
> +}
> +
> /**
> * Configure one field in @a cfg.
> * @param L Lua stack.
> diff --git a/src/lua/utils.h b/src/lua/utils.h
> index a47e3d2b4..96311b75c 100644
> --- a/src/lua/utils.h
> +++ b/src/lua/utils.h
> @@ -241,6 +241,13 @@ luaL_checkserializer(struct lua_State *L) {
> luaL_checkudata(L, lua_upvalueindex(1), LUAL_SERIALIZER);
> }
>
> +/**
> + * Initialize serializer with default parameters.
> + * @param cfg Serializer to inherit configuration.
> + */
> +void
> +luaL_serializer_create(struct luaL_serializer *cfg);
> +
> /**
> * Parse configuration table into @a cfg.
> * @param L Lua stack.
> diff --git a/test/box/tuple.result b/test/box/tuple.result
> index b42012485..16aa66b1a 100644
> --- a/test/box/tuple.result
> +++ b/test/box/tuple.result
> @@ -1164,3 +1164,53 @@ test_run:cmd("clear filter")
> ---
> - true
> ...
> +--
> +-- gh-3882: Inappropriate storage optimization for sparse arrays
> +-- in box.tuple.new.
> +--
> +t = {}
> +---
> +...
> +t[1] = 1
> +---
> +...
> +t[2] = 2
> +---
> +...
> +t[11] = 11
> +---
> +...
> +box.tuple.new(t)
> +---
> +- [1, 2, null, null, null, null, null, null, null, null, 11]
> +...
> +s2 = box.schema.space.create('test')
> +---
> +...
> +test_run:cmd("setopt delimiter ';'")
> +---
> +- true
> +...
> +s2:format({{name="a", type="str"}, {name="b", type="str", is_nullable=true},
> + {name="c", type="str", is_nullable=true},
> + {name="d", type="str", is_nullable=true},
> + {name="e", type="str", is_nullable=true},
> + {name="f", type="str", is_nullable=true},
> + {name="g", type="str", is_nullable=true},
> + {name="h", type="str", is_nullable=true},
> + {name="i", type="str", is_nullable=true},
> + {name="j", type="str", is_nullable=true},
> + {name="k", type="str", is_nullable=true}});
> +---
> +...
> +test_run:cmd("setopt delimiter ''");
> +---
> +- true
> +...
> +s2:frommap({a="1", k="11"})
> +---
> +- ['1', null, null, null, null, null, null, null, null, null, '11']
> +...
> +s2:drop()
> +---
> +...
> diff --git a/test/box/tuple.test.lua b/test/box/tuple.test.lua
> index 276bb0f67..0c89feace 100644
> --- a/test/box/tuple.test.lua
> +++ b/test/box/tuple.test.lua
> @@ -384,3 +384,29 @@ t2 = box.tuple.new(2)
> t1 = t1:update{{'+', 1, 1}}
>
> test_run:cmd("clear filter")
> +
> +--
> +-- gh-3882: Inappropriate storage optimization for sparse arrays
> +-- in box.tuple.new.
> +--
> +t = {}
> +t[1] = 1
> +t[2] = 2
> +t[11] = 11
> +box.tuple.new(t)
> +
> +s2 = box.schema.space.create('test')
> +test_run:cmd("setopt delimiter ';'")
> +s2:format({{name="a", type="str"}, {name="b", type="str", is_nullable=true},
> + {name="c", type="str", is_nullable=true},
> + {name="d", type="str", is_nullable=true},
> + {name="e", type="str", is_nullable=true},
> + {name="f", type="str", is_nullable=true},
> + {name="g", type="str", is_nullable=true},
> + {name="h", type="str", is_nullable=true},
> + {name="i", type="str", is_nullable=true},
> + {name="j", type="str", is_nullable=true},
> + {name="k", type="str", is_nullable=true}});
> +test_run:cmd("setopt delimiter ''");
> +s2:frommap({a="1", k="11"})
> +s2:drop()
> --
> 2.20.1
>
More information about the Tarantool-patches
mailing list