Re: [Tarantool-patches] [PATCH 1/1] fiber: extend max fiber name length to 255

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

On 14/03/2020 17:34, Cyrill Gorcunov wrote:
> On Sat, Mar 14, 2020 at 04:16:35PM +0100, Vladislav Shpilevoy wrote:
>> Users keep complaining about too short fiber name. New limit is
>> 255, should be enough for any sane name.
>>
>> Closes #4394
>>
>>  fiber_set_name(struct fiber *fiber, const char *name)
>>  {
>> -	assert(name != NULL);
>> -	snprintf(fiber->name, sizeof(fiber->name), "%s", name);
>> +	size_t len = MIN(strlen(name), FIBER_NAME_MAX);
>> +	char *new_name = realloc(fiber->name, len + 1);
> 
> I don't like it completely. The fiber cache has been made
> not just to eliminate new memory allocation but also to
> reduce memory fragmentation and now we give a user a hand
> to shuffle memory in easy path :(

Heap fragmentation does not affect anything. We allocate data
not on the heap, but on slab arena. Heap is used for metadata
such as space_defs, index_defs, key_defs. Moreover, heap is
used for everything used by Lua. I don't see any problem with
allocating non-data objects on the heap.

Talking of fiber cache - I think *this* may be a bad thing.
Because fibers are never deleted. So from a certain point of
time the mempool does not do speed up any alloc/free calls. And
just occupies slabs that could be used for data, and increases
the arena fragmentation.

> If 32 bytes is really not enough lest make it 64 (or 128)

If we make it 64 are more, we just aggravate the problem I
described above.

> and allocate together with fiber cache.
> 
>> +	if (new_name == NULL)
>> +		panic("fiber_set_name() can't fail");
>> +	fiber->name = new_name;
>> +	memcpy(new_name, name, len);
>> +	new_name[len] = 0;