From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 4BD8B27922 for ; Wed, 13 Feb 2019 07:19:30 -0500 (EST) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id VPpZi79aP3t6 for ; Wed, 13 Feb 2019 07:19:30 -0500 (EST) Received: from smtp61.i.mail.ru (smtp61.i.mail.ru [217.69.128.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 7FFB427855 for ; Wed, 13 Feb 2019 07:19:29 -0500 (EST) Date: Wed, 13 Feb 2019 15:19:26 +0300 From: Konstantin Osipov Subject: [tarantool-patches] Re: box: disable sparse optimization in box.tuple.new() Message-ID: <20190213121926.GA27871@chai> References: <38d2414b18464bc28be295281de1779fd7cfb1cc.1549881301.git.kshcherbatov@tarantool.org> <708eb7bb-88a5-7663-3682-bd4d4a61d2c5@tarantool.org> <57b38a8d-14f4-a272-7fdb-4f6cf08cf925@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <57b38a8d-14f4-a272-7fdb-4f6cf08cf925@tarantool.org> Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: tarantool-patches@freelists.org Cc: Kirill Shcherbatov * Vladislav Shpilevoy [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