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

[Konstantin Osipov <kostja@tarantool.org>]

* Vladislav Shpilevoy <v.shpilevoy@tarantool.org> [19/02/12 20:54]:

Please use a separate serializer configuration. Both solutions are
bad, but at least I see no forward compatibility
issues with a separate configuration. If we break any existing
code by changing the behavior, we'll expose the config as a
separate one for box.tuple.new() or frommap().

> 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(+)
> > 

-- 
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32
http://tarantool.io - www.twitter.com/kostja_osipov