[PATCH v3 0/5] malloc: tcache improvements

[Cupertino Miranda]

Hi Wilco,

On 14-03-2025 17:47, Wilco Dijkstra wrote:
> Hi Cupertino,
> 
>> All results were taken without any tunables set, i.e. the tcache is
>> still limited to roughly 1k chunks and bigger chunks caching is not
>> benchmarked.
> 
> The v4 results look pretty good indeed!
> 
>> I will soon prepare the patches and submit has a new v4 series.
>> For v4 I will leave the new code in isolated patches to easily follow
>> the recent changes.
> 
> Could you place the simple changes in your series first please? In v4,
> patch 1 and 3 look good. The inlining change v4 patch 6 is obvious too
> (there was at least one other wrong use of inline). Plus any other generic
> improvements like adding unlikely that apply to existing code.
Ok. Will do!
> 
> However I think the pointer swizzling change (patch 2 in v3) should be
> separate - it's not clear what causes the slowdown, but it is not necessary
> for your later changes, right?
Well, it is sort of an optimization, since it now does not need to have 
a condition to disambiguate between being accessing the entry of the bin 
or the middle.
For fixed chunk size bins below ~1k size, it is Ok since you never have 
to traverse the bin, however for traversing the linked-list it would 
have some impact.

Please notice the following simplification in tcache_get_n:
-  tcache_entry *e;
-  if (ep == &(tcache->entries[tc_idx]))
-    e = *ep;
-  else
-    e = REVEAL_PTR (*ep);
+  tcache_entry *e = REVEAL_PTR (*ep);
...
-  if (ep == &(tcache->entries[tc_idx]))
-      *ep = REVEAL_PTR (e->next);
-  else
-    *ep = PROTECT_PTR (ep, REVEAL_PTR (e->next));
+  *ep = PROTECT_PTR (ep, REVEAL_PTR (e->next));

Also patch 2 allows to have a very simple implementation of 
tcache_location_for_size in patch 4.


> The best solution is to remove the horrible
> interfaces tache_get_n/tcache_next and replace with tache_get_aligned.
tcache_next can easily be removed. I presume you mean to rename 
tcache_get_n to tcache_get_aligned.

> Extra pointer swizzling could then be added if the security gain is worth
> the cost cost.
> 
>> Hope you see the value of running mimalloc-bench and accept it as a
>> valid benchmark for the purpose.
> 
> I think it is a good idea, just need to try it out...
> 
> Btw I looked into why the bench-malloc-thread test is so sensitive - it spends
> a lot of time in tcache code, so it effectively benchmarks tcache. With a few
> tweaks to move unnecessary initialization out of the critical path and using
> tailcalls, I got 33% speedup! That's just some basic tweaks to libc_free without
> changing interfaces, so if we optimize tcache a bit more, we might get 40-50%.
Nice!
> 
> Cheers,
> Wilco

Cheers,
Cupertino