[tarantool-patches] Re: [PATCH 1/1] buffer: port static allocator to Lua

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

On 18/05/2019 22:12, Konstantin Osipov wrote:
> * Vladislav Shpilevoy <v.shpilevoy@tarantool.org> [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]', <cdata void *>) - 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.