[PATCH v4 4/7] malloc: add tcache support for large chunk caching

[Cupertino Miranda]

On 15-03-2025 00:02, DJ Delorie wrote:
> Cupertino Miranda <cupertino.miranda@oracle.com> writes:
>> Existing tcache implementation in glibc seems to focus in caching
>> smaller data size allocations, limiting the size of the allocation to
>> 1KB.
> 
> Statistics at that time showed that such packets were by far the most
> popular ones to allocate.
No doubts, I do give some more context below for why I did this work.

> 
>> This patch changes tcache implementation to allow to cache any chunk
>> size allocations.  The implementation adds extra bins (linked-lists)
>> which store chunks with different ranges of allocation sizes. Bin
>> selection is done in multiples in powers of 2 and chunks are inserted in
>> growing size ordering within the bin.  The last bin contains all other
>> sizes of allocations.
> 
> I fear this will cause fragmentation and thus higher VSS usage.  I've
> already seen proposals for changes in tcache to try to reduce the
> fragmentation it's already causing.  Do you have any benchmarks on
> memory *usage* changes with this patch?  It would have to be a fairly
> complex application.
Please notice that without setting glibc.malloc.tcache_max the semantics 
of tcache are kept to original behaviour.

Last week, I worked on collecting data with mimalloc-bench, in the 
context of a few reviews from Wilco Dijkstra which shown some 
performance degradation.
Within those I have collected the data without setting 
glibc.malloc.tcache_max tunable, which implied that only up to ~1k 
chunks would be cached.
Considering that semantically tcaches work in same way as before, I did 
not care to make any memory comparisons, as any variation would be 
unrelated to the patch series.

However, our original bench-marking has been with MySQL application, 
which historically with glibc malloc uses huge amounts of VSS and with 
time slowly but steadily consumes RSS until the process gets killed.
The presented patch allows within MySQL application to set max_arena=1 
and keep performance levels by calibrating tcache_max tunable to a big 
enough value.
Without tcache_max tunable performance would drop to half.

Doing this VSS is kept to a minimum and also RSS stops growing.
I have provided MySQL charts as a reply the initial RFC of the patch series.

> 
> 
>>   /* We want 64 entries.  This is an arbitrary limit, which tunables can reduce.  */
>> +# define TCACHE_UNBOUND_SIZE_BINS	10
>>   # define TCACHE_MAX_BINS		64
> 
> Putting the new line there means the comment no longer makes sense ;-)
Will rewrite comment here.
> 
>>   # define MAX_TCACHE_SIZE	tidx2usize (TCACHE_MAX_BINS-1)
>> +# define TCACHE_FIXED_SIZE_BINS	\
>> +	   (mp_.tcache_bins < TCACHE_MAX_BINS ? mp_.tcache_bins : TCACHE_MAX_BINS)
> 
> I can see this adding a lot of logic to tcache.  Do the benchmarks
> indicate it's faster?
In v4, yes.
> 
>>   /* Only used to pre-fill the tunables.  */
>> -# define tidx2usize(idx)	(((size_t) idx) * MALLOC_ALIGNMENT + MINSIZE - SIZE_SZ)
>> +# define tidx2usize(idx)	(((size_t) idx) * MALLOC_ALIGNMENT + MINSIZE - MALLOC_ALIGNMENT + 1)
> 
> This seems unrelated.  Has this been verified on a platform where you
> know SIZE_SZ is different than MALLOC_ALIGNMENT+1 ?
I am not sure if CI for glibc is running in such platforms. It passed 
everything, AFAIK. If not then, no.
BTW, I just reversed the equation for csize2tidx. This seemed more 
accurate. Maybe it is wrong! :(

> 
>>     ,
>>     .tcache_count = TCACHE_FILL_COUNT,
>>     .tcache_bins = TCACHE_MAX_BINS,
>> -  .tcache_max_bytes = tidx2usize (TCACHE_MAX_BINS-1),
>> +  .tcache_max_bytes = MAX_TCACHE_SIZE,
>> -  .tcache_unsorted_limit = 0 /* No limit.  */
>> +  .tcache_unsorted_limit = 0, /* No limit.  */
>>   };
> 
> Do we really need the extra comma here?
No!
> 
>> -  for (i = 0; i < TCACHE_MAX_BINS; ++i)
>> +  for (i = 0; i < mp_.tcache_bins; ++i)
> 
> This is wrong if the number of bins can be changed during program
> execution; the intent is to flush the WHOLE cache, not the CURRENT
> cache.
Right! Should change it. However to (TCACHE_MAX_BINS + 
TCACHE_UNBOUND_SIZE_BINS).

Thanks,
Cupertino