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

[DJ Delorie]

Florian Weimer <fw@deneb.enyo.de> writes:
>> +/* This is the default, and means "check to see if a real tcache
>> +   should be allocated."  */
>> +#define TCACHE_INACTIVE() (tcache == &__tcache_dummy.inactive)
>> +/* This means "the user has disabled the tcache but we have to point
>> +   to something."  */
>> +#define TCACHE_DISABLED() (tcache == &__tcache_dummy.disabled)
>> +/* This means the tcache is active.  */
>> +#define TCACHE_ENABLED() (!TCACHE_INACTIVE() && !TCACHE_DISABLED())
>> +/* Sets the tcache to INACTIVE state.  */
>> +#define TCACHE_SET_INACTIVE() tcache = (tcache_perthread_struct *) &__tcache_dummy.inactive;
>> +/* Sets the tcache to DISABLED state.  */
>> +#define TCACHE_SET_DISABLED() tcache = (tcache_perthread_struct *) &__tcache_dummy.disabled;
>
> I suppose these could be inline functions instead of macros.

That's been suggested in the past, and IIRC my reply was "why?"
Changing these to functions just makes the file longer, it doesn't
change the generated code.  And, I like having all the related macros
together like that instead of spread across multiple pages.

OTOH maybe it's time to clean up the organization of the malloc sources?
The main file is too big and the helper files are too small, and arena.c
is an include(?) instead of a standalone file, etc.  If stuff were more
organized, the extra whitespace functions-vs-macros would add would be
less of a problem.

$ wc -l ...
   910 malloc/arena.c
  6115 malloc/malloc.c
    46 malloc/malloc-internal.h
   161 malloc/malloc.h


>> @@ -3305,14 +3332,14 @@ tcache_thread_shutdown (void)
>>  {
>>    int i;
>>    tcache_perthread_struct *tcache_tmp = tcache;
>> +  int need_free = TCACHE_ENABLED ();
>>  
>>    tcache_shutting_down = true;
>
> Isn't tcache_shutting_down now redundant?  Because we have
> TCACHE_SET_DISABLED?

Err... yes?  I'll remove it.

>> @@ -3388,6 +3411,11 @@ __libc_malloc2 (size_t bytes)
>>    mstate ar_ptr;
>>    void *victim;
>>  
>> +#if USE_TCACHE
>> +  if (__glibc_unlikely (TCACHE_INACTIVE ()))
>> +    tcache_init ();
>> +#endif
>
> Could this be moved to the
>
>>    if (SINGLE_THREAD_P)
>>      {
>>        victim = tag_new_usable (_int_malloc (&main_arena, bytes));
>> @@ -3427,8 +3455,6 @@ __libc_malloc (sizer_t bytes)
>>    if (nb < mp_.tcache_max_bytes)
>>      {
>>        size_t tc_idx = csize2tidx (nb);
>> -      if(__glibc_unlikely (tcache == NULL))
>> -	return tcache_malloc_init (bytes);
>
> tcache path here?

No, I had to move it outside the code that takes the lock, else it would
soft-lock in tcache_init when it too calls malloc.