[patch v4] malloc: avoid need for tcache == NULL checks

[Florian Weimer]

* Wilco Dijkstra:

> Hi Florian/DJ,
>
> ___> @@ -4030,10 +4073,14 @@ _int_malloc (mstate av, size_t bytes)
>>              /* While we're here, if we see other chunks of the same size,
>>                 stash them in the tcache.  */
>>              size_t tc_idx = csize2tidx (nb);
>> -           if (tcache != NULL && tc_idx < mp_.tcache_small_bins)
>> +           if (tc_idx < mp_.tcache_small_bins)
>>                {
>>                  mchunkptr tc_victim;
>> 
>> +#if USE_TCACHE
>
> Note we are already in a USE_TCACHE block here...

^^^ DJ, I think you should remove that #if.

>> +               if (__glibc_unlikely (tcache_inactive ()))
>> +                 tcache_init (av);
>> +#endif
>>                  /* While bin not empty and tcache not full, copy chunks.  */
>>                  while (tcache->num_slots[tc_idx] != 0 && (tc_victim = *fb) != NULL)
>>                    {
>
>> I'm still concerned that tcache_init invalidates *fb because it triggers
>> consolidation.
>>
>> Wilco, what do you thinK?
>
> The victim has already been unlinked, and 'fb' is a constant offset
> from arena, so there isn't anything that could be out of date as far
> as I can see.

Right, this one seems okay.

There's a different loop further below:

 	      /* While bin not empty and tcache not full, copy chunks over.  */
 	      while (tcache->num_slots[tc_idx] != 0
 		     && (tc_victim = last (bin)) != bin)

But t hat's okay as well because bin is again an address that points
into struct malloc_state.

It looks like this applies to all cases.

> But yes, recursive calls in the middle of allocation
> functions are inherently risky, hence I don't like all the on-demand
> initializations. And there is a performance impact of placing calls in
> the middle of fast paths...

We could complete the call and then initialize tcache for the next call.
But I think we really should IFUNC malloc in total or just the front end
routines, compile in tcache for __libc_initial only, and put tcache
directly into the TCB.  For !__libc_initial, we don't use tcache at all.
That avoids a bunch of indirections and data loads.

Thanks,
Florian