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 CE44526ADE for ; Tue, 12 Feb 2019 12:50:12 -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 GmTebZEY5VqD for ; Tue, 12 Feb 2019 12:50:12 -0500 (EST) Received: from smtp3.mail.ru (smtp3.mail.ru [94.100.179.58]) (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 7650C26A9D for ; Tue, 12 Feb 2019 12:50:12 -0500 (EST) Subject: [tarantool-patches] Re: box: disable sparse optimization in box.tuple.new() References: <38d2414b18464bc28be295281de1779fd7cfb1cc.1549881301.git.kshcherbatov@tarantool.org> <708eb7bb-88a5-7663-3682-bd4d4a61d2c5@tarantool.org> From: Vladislav Shpilevoy Message-ID: <57b38a8d-14f4-a272-7fdb-4f6cf08cf925@tarantool.org> Date: Tue, 12 Feb 2019 18:50:06 +0100 MIME-Version: 1.0 In-Reply-To: <708eb7bb-88a5-7663-3682-bd4d4a61d2c5@tarantool.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit 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, Kirill Shcherbatov 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(+) >