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 1A4AC2EFD5 for ; Sat, 18 May 2019 18:15:13 -0400 (EDT) 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 9n9tLHlk7QKb for ; Sat, 18 May 2019 18:15:13 -0400 (EDT) Received: from smtp60.i.mail.ru (smtp60.i.mail.ru [217.69.128.40]) (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 7C0AC2EFBD for ; Sat, 18 May 2019 18:15:12 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH 1/1] buffer: port static allocator to Lua References: <7be31fc792dac8cfee0adc221a8a22ffdf14a677.1557927101.git.v.shpilevoy@tarantool.org> <20190518191224.GF9448@atlas> From: Vladislav Shpilevoy Message-ID: <06507d82-1888-1d52-7a21-fc4661626acb@tarantool.org> Date: Sun, 19 May 2019 01:15:07 +0300 MIME-Version: 1.0 In-Reply-To: <20190518191224.GF9448@atlas> Content-Type: text/plain; charset=utf-8 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: Konstantin Osipov Cc: tarantool-patches@freelists.org On 18/05/2019 22:12, Konstantin Osipov wrote: > * Vladislav Shpilevoy [19/05/15 16:34]: > > 1) scalar looks clearly as a separate patch. while I like the idea > I don't appreciate the urgency and a huge patch. Done in a separate commit, see v2 thread. Urgency is motivated by SWIM Lua API dependency on that - I do not want to push SWIM Lua with ugly "ffi.new('int[1]')" construction. > > 2) The static alloc patch is OK to push > > 3) Since you care about performance so much, I don't understand > why you do concat (..) in Lua static alloc. Did you check it's > a compile-time, not a run-time concat? I did not check whether it is compile time, because Lua has no a compiler. But I made a benchmark - even with concat it is orders of magnitude faster. Nonetheless, probably you could be right, and it would be better to avoid concatenation - then impact would be even greater. Firstly, I tried to avoid concatenation and pass the array type always. But I faced a problem, that ffi.cast does not allow some C conversions. I needed to be able to do ffi.cast('type[size]', ) - in C I would not have problems, but in Lua I do. I started looking into FFI implementation, and found how to allow that conversion, but then I decided that it is not worth doing without a benchmark: concact vs no concat. Appeared, that noconcat version is ~5% faster. In the summary, original patch makes small allocations +10000% faster, without concat it would be +10005%. Not sure, if this difference is worth splitting static_alloc() into multiple functions, nor moving ffi.sizeof() + ffi.cast() operations onto the caller's shoulders. So I decided to keep it is as. Otherwise it becomes too complicated. >> + if ffi.C.EVP_CipherUpdate(self.ctx, wpos, scalar.ai, input, input:len()) ~= 1 then > > I don't understand, if you need a scalar and an array of scalars, > and you have scalar and scalar_array variables, why would you ever > use an array in scalar? I use array types [1] in scalar, because it is the only way to get a pointer onto a type in Lua. > Wouldn't it be better to have two unions > instead of one for the sake of clarity? I have - scalar_array is two unions. I thought about their separation in something like buffer.scalar1, buffer.scalar2, but looks ugly, unnatural. But here I do not insist. If you want, I can split them into two variables. Should I? See V2 thread.