[Tarantool-patches] [PATCH 1/1] fiber: extend max fiber name length to 255
Vladislav Shpilevoy
v.shpilevoy at tarantool.org
Sat Mar 14 20:25:48 MSK 2020
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;
More information about the Tarantool-patches
mailing list