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

[Cupertino Miranda]

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.

Cheers,
Cupertino