[tarantool-patches] Re: [PATCH 1/2] swim: pool IO tasks

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

On 08/07/2019 10:25, Konstantin Osipov wrote:
> * Vladislav Shpilevoy <v.shpilevoy@tarantool.org> [19/07/07 16:28]:
>>>> +static struct stailq swim_task_pool;
>>>> +/** Number of pooled tasks. */
>>>> +static int swim_task_pool_size = 0;
>>>
>>> These should be thread-local.
>>
>> Why? SWIM works in one thread, these values are never accessed
>> from another one. They would be just copied for each thread and
>> unused in all but one.
> 
> What happens if I create multiple instances of swim library in
> multiple threads? These instances will try to concurrently access
> these members without mutexes.

How is it possible? SWIM works in TX only. You just can't
create it another thread. It is literally impossible.

Even if someday it will work in another thread, still all
the SWIMs will work in one thread. So in future we won't
allow to scatter SWIMs among multiple threads as well.

With the same reasoning we would need to make thread-local
all global on_replace triggers like on_replace_vinyl_deferred_delete
just in case if in future Vinyl will do DML/DQL in multiple
threads. But why should we do that now? I do not understand.

>>
>>>
>>> Why not use mempool?
>>>
>>>
>>
>> Because 1) it is an overkill, 2) I don't want to depend on
>> slab allocator, 3) it just does not fit this case, according
>> to mempool description from mempool.h:
>>
>>     "Good for allocating tons of small objects of the same size.".
> 
> It is also quite decent for allocating many fairly large objects.
> The key point is that the object is of the same size. You can set
> up mempool the right slab size, and in this case it will do
> exactly what you want.
> 

And again - 'many' usually won't be the case. We will have 0-2 SWIMs
in 99% of cases. One internal SWIM for box, and one external created
by a user. Barely we will have more than 10 cached tasks.

But ok, as you wish. This place is not as critical for me as thread
locality of the pool. At least we reuse existing code. Thread locality
still looks pointless waste of memory for me.