[tarantool-patches] Re: box: disable sparse optimization in box.tuple.new()

[Vladislav Shpilevoy]

Hi! Thanks for the patch!

On 12/02/2019 14:18, Kirill Shcherbatov wrote:
> 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
> ---

Please, next time put here links to the branch and issue.

The patch is ok for me. But I see, that Kostja complaints, that
you could create a special serializer for it. I agree, but I see
a drawback about it. Separate serializer would be unconfigurable.
Now a user can change serializer by making require('msgpack').cfg(...).
So the issue can be fixed manually like this even on 2.1:

[002] Test failed! Result content mismatch:
[002] --- box/tuple.result	Tue Feb 12 18:26:58 2019
[002] +++ box/tuple.reject	Tue Feb 12 18:42:10 2019
[002] @@ -1168,6 +1168,12 @@
[002]  -- gh-3882: Inappropriate storage optimization for sparse arrays
[002]  --          in box.tuple.new.
[002]  --
[002] +m = require('msgpack')
[002] +---
[002] +...
[002] +m.cfg{encode_sparse_ratio = 0}
[002] +---
[002] +...

I added these two lines without a patch, and the test passed.

With Kostja's way of implementation, tuple encoding would be
impossible to configure.

If Kostja thinks, that your way does not work, then do whatever
he wants, please. Ask him for clarification.

>   src/box/lua/tuple.c     |  9 ++++++++
>   test/box/tuple.result   | 50 +++++++++++++++++++++++++++++++++++++++++
>   test/box/tuple.test.lua | 26 +++++++++++++++++++++
>   3 files changed, 85 insertions(+)
>