[PATCH] malloc: retire mechanism for older non used tcached chunks

[Cupertino Miranda]

On 13-11-2025 15:32, Cupertino Miranda wrote:
> Hi Florian,
> 
> Thank you for the quick feedback.
> 
> 
> On 13-11-2025 14:43, Florian Weimer wrote:
>> * Cupertino Miranda:
>>
>>> +      /* Move chunk to retire bin if chunk is beyond tcache capacity.
>>> +       * We should always do (alloc_size_state - cached_at) before 
>>> attempting
>>> +       * any comparissons.  This is a requirement to keep the 
>>> comparisson valid
>>> +       * in the cases where one of the values has wraparound.  */
>>
>> Our usual comment style does not use “*” at the beginning of lines.
> Right, my bad! Will fix it in v2.
> 
>>
>>> +      if (tcache->alloc_size_state - te->cached_at > 
>>> mp_.tcache_retire_size)
>>> +        {
>>> +          uint16_t not_used = 1;
>>> +          tcache_entry *e = tcache_get_n (tep, num_slots, *mangled);
>>> +      tcache_put_n (mem2chunk (e), &tcache->return_to_arena, &not_used,
>>> +            false);
>>> +          te = *mangled == false ? *tep : REVEAL_PTR (*tep);
>>> +          continue;
>>> +        }
>>
>> It's not entirely clear to me way return_to_arena is used.
> The thought behind having another bin to store retired chunks (i.e. the 
> return_to_arena) was to to delay the overhead of actually blocking the 
> arena to some time where it would be impossible not to lock it.
> This prioritizes performance instead of memory usage, both strategies 
> have merit, this is a good point for discussion.
> 
>>
>>> +      while (tcache_tmp->return_to_arena)
>>> +    {
>>> +      tcache_entry *e = tcache_tmp->return_to_arena;
>>> +      if (__glibc_unlikely (misaligned_mem (e)))
>>> +        malloc_printerr ("tcache_thread_shutdown(): "
>>> +                 "unaligned tcache chunk detected");
>>> +      tcache_tmp->return_to_arena = REVEAL_PTR (e->next);
>>> +      __libc_free (e);
>>
>> These needs a comment why this actually returns allocations to the
>> arena.  And if it doesn't why this isn't necessary.
> I don't know if I understand what you mean. It always does it in that 
> loop, right? The loop traverses the bin freeing all those chunks.
> One mistake I notice there is the call to __libc_free, instead of 
> _int_free_chunk, locking the arena only once instead of for each chunk.
> Will fix it for v2.
Just realize that it would only make sense if there is a single arena.

> 
> Cheers,
> Cupertino