Re: [Tarantool-patches] [PATCH v8 3/4] box/cbox: implement cbox Lua module

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

On 31.10.2020 22:59, Cyrill Gorcunov wrote:
> On Sat, Oct 31, 2020 at 01:21:34AM +0100, Vladislav Shpilevoy wrote:
>>>
>>> I'll try to repeat. That;s interesting, actually I've been trying
>>> to force gc to trigger but without much luck. Letme investigate
>>> this.
>>
>> It is not about Lua GC. The collectgarbage('collect') works fine
>> and the function object is deleted. But the module is not unloaded.
>> Because you never call module_sym_unload().
> 
> I'll investigate.
> 
>>> Here is a paragraph above
>>> ---
>>>     Once function is loaded it is possible to execute it
>>>     in a traditional Lua way, ie to call it as a function.
>>>
>>>     ``` Lua
>>>     -- call with agruments arg1 and arg2
>>>     f(arg1, arg2)
>>>     ```
>>> ---
>>>
>>> I'm not sure what else we could put here?
>>
>> This paragraph is false, it seems. Because it does not work like Lua
>> functions 1-to-1. It returns nil,err or true,results. Lua functions
>> return whatever you return from them so just 'results'. These cbox
>> functions change the returned set a bit. And this is good, but you
>> didn't document it.
> 
> Vlad, frankly I don't understand what else we can put here in documentation,
> lets talk f2f about this.

Давай на русском.

Не понимаю. Как это нечего положить в доку? У тебя С функции
возвращают всегда не то, что юзер из них вернул. Ты добавляешь туда
true, или nil,err. Как до этого юзер догадается, что функция
при вызове через cbox модифицирует набор возвращаемых значений? Луа
так не делает, поэтому ты не можешь сказать, что оно работает как
Луа.

Если написать функцию, которая делает сумму двух чисел, то в Луа это
будет выглядеть так:

	local f = function(a, b) return a + b end
	assert(f(1, 2) == 3)

В С через cbox это будет написано так:

	test.c:

	int
	sum(box_function_ctx_t *ctx, const char *args, const char *args_end)
	{
		uint64_t a = mp_decode_uint(&args);
		uint64_t b = mp_decode_uint(&args);
		char res[16];
		char *end = mp_encode_uint(res, a + b);
		box_return_mp(ctx, res, end);
		return 0;
	}

Но в Lua вести оно себя будет нифига не так же:

	local f = cbox.load('test.sum')
	assert(f(1, 2) == 3) -- Это ебанет.

Потому что у тебя f будет возвращать true + result.

	local ok, res = f(1, 2)
	assert(ok)
	assert(res == 3) -- Вот это пройдет.

Это не точно как луа, и явно должно быть в документации.

Ошибки тоже надо задокументировать - что функции cbox никогда не
кидают исключения. Только возвращают nil, err.

По поводу бенчей - да, походу дерево выглядит лучше или по крайней
мере предсказуемо. Мне норм оставить дерево. По хешу я создал тикет,
так как оставлять это без действия нельзя. Если текущий хеш так
плох, его надо поменять. Либо его метод расчета, либо вообще на
дерево его заменить в некоторых других местах тоже. Но я все еще не
уверен, что твои данные для "худшего случая" вообще возможны в
реальности, на типичных именах функций, например. Так что это надо
еще до-проверять.

https://github.com/tarantool/tarantool/issues/5492

>>>> 9. We already discussed it a ton of times, literally exactly the
>>>> same 'problem' with struct error objects. A 64 bit integer
>>>> will not overflow for your lifetime even if it would increment
>>>> every nanosecond. How is it possible? Why the code should get
>>>> complicated by making a simple 'ref' function be able to fail?
>>>
>>> This has nothing to do for fair use. It prevents from errors where
>>> you occasionally screwed the counter.
>>
>> In the code you always either increment or decrement it. How many
>> lifes you will need to get it 'screwed'?
> 
> Counters are not always screwed by correct calls of inc/dec but sometime
> it might be an indirrect errors due to addressing bugs and such. IOW
> if we can check counters for saturation -- we should. If you really
> think it has such big penalty we could put it under #ifdef.

Это будет еще хуже с ifdef. Все равно придется проверять, что ref не
вернул -1, что выглядит супер стремно.

>>> What we really
>>> need is a general kind of kref_t and it *must* include saturation
>>> check somewhere inside and panic on overflow.
>>
>> This is not really applicable in Tarantool. At least not everywhere.
>> Struct tuple, for instance, is referenced, but it uses a very special
>> reference counter to save every single byte, because tuple count is
>> millions and billions.
> 
> And I don't propose it for bigrefs but IIRC we've a number of places
> where plain reference counters are used?

Таких мест точно не много, но да, есть структуры, которые рефаются
обычными интами. Все из-за того, что рефы очень не нравились Косте,
и их супер мало применяли. Я не найду сходу сейчас.