malloc: avoid need for tcache == NULL checks

[Cupertino Miranda]

Hi DJ,

This seems possible now since the tcache->num_slots[tc_idx] == 0 if the 
entries are full. In that way you also protect tcache_put from 
attempting to write to the tcache->entries[tc_idx], just like you say in 
the commit message.

I understand the benefit but I personally do not like this type of 
hidden implicit behavior.
For large chunk caching it would rather make more sense to have some 
sort of memory management that would limit the amount of data cached 
instead of limiting to a number of slots (I am working on that now).
In any case, paths are split in current implementation and we can check 
for TCACHE_ENABLED for large chunks caching.

Anyway, apologies for the rambling.

For what is worth, it looks good to me.

Cheers,
Cupertino

On 08-08-2025 01:12, DJ Delorie wrote:
> 
> Avoid needing to check for tcache == NULL by initializing it
> to a dummy read-only tcache structure.  This dummy is all zeros,
> so logically it is both full (when you want to put) and empty (when
> you want to get).  Also, there are two dummies, one used for
> "not yet initialized" and one for "tunables say we shouldn't have
> a tcache".
> 
> The net result is twofold:
> 
> 1. Checks for tcache == NULL may be removed from the fast path.
>     Whether this makes the fast path faster when tcache is
>     disabled is TBD, but the normal case is tcache enabled.
> 
> 2. no memory for tcache is allocated if tunables disable caching.
> 
> Co-authored-by: Florian Weimer <fweimer@redhat.com>
> 
> diff --git a/malloc/malloc.c b/malloc/malloc.c
> index 9d646abae9..66ae187569 100644
> --- a/malloc/malloc.c
> +++ b/malloc/malloc.c
> @@ -3075,8 +3075,35 @@ typedef struct tcache_perthread_struct
>     tcache_entry *entries[TCACHE_MAX_BINS];
>   } tcache_perthread_struct;
>   
> +static const union
> +{
> +  struct tcache_perthread_struct inactive;
> +  struct
> +  {
> +    char pad;
> +    struct tcache_perthread_struct disabled;
> +  };
> +} __tcache_dummy;
> +
>   static __thread bool tcache_shutting_down = false;
> -static __thread tcache_perthread_struct *tcache = NULL;
> +
> +/* TCACHE is never NULL; it's either "live" or points to one of the
> +   above dummy entries.  The dummy entries are all zero so act like an
> +   empty/unusable tcache.  */
> +static __thread tcache_perthread_struct *tcache = (tcache_perthread_struct *) &__tcache_dummy.inactive;
> +
> +/* 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;
>   
>   /* Process-wide key to try and catch a double-free in the same thread.  */
>   static uintptr_t tcache_key;
> @@ -3223,7 +3250,7 @@ tcache_get_align (size_t nb, size_t alignment)
>   {
>     if (nb < mp_.tcache_max_bytes)
>       {
> -      if (__glibc_unlikely (tcache == NULL))
> +      if (__glibc_unlikely (TCACHE_INACTIVE ()))
>   	{
>   	  tcache_init ();
>   	  return NULL;
> @@ -3291,14 +3318,14 @@ tcache_thread_shutdown (void)
>   {
>     int i;
>     tcache_perthread_struct *tcache_tmp = tcache;
> +  int need_free = TCACHE_ENABLED ();
>   
>     tcache_shutting_down = true;
>   
> -  if (!tcache)
> -    return;
> -
>     /* Disable the tcache and prevent it from being reinitialized.  */
> -  tcache = NULL;
> +  TCACHE_SET_DISABLED ();
> +  if (! need_free)
> +    return;
>   
>     /* Free all of the entries and the tcache itself back to the arena
>        heap for coalescing.  */
> @@ -3331,10 +3358,20 @@ tcache_init (void)
>     if (MAX_TCACHE_SMALL_SIZE >= GLRO (dl_pagesize) / 2)
>       malloc_printerr ("max tcache size too large");
>   
> +  if (mp_.tcache_count == 0)
> +    {
> +      TCACHE_SET_DISABLED ();
> +      return;
> +    }
> +
>     size_t bytes = sizeof (tcache_perthread_struct);
>     tcache = (tcache_perthread_struct *) __libc_malloc2 (bytes);
>   
> -  if (tcache != NULL)
> +  if (tcache == NULL)
> +    {
> +      TCACHE_SET_INACTIVE ();
> +    }
> +  else
>       {
>         memset (tcache, 0, bytes);
>         for (int i = 0; i < TCACHE_MAX_BINS; i++)
> @@ -3413,7 +3450,7 @@ __libc_malloc (size_t bytes)
>     if (nb < mp_.tcache_max_bytes)
>       {
>         size_t tc_idx = csize2tidx (nb);
> -      if(__glibc_unlikely (tcache == NULL))
> +      if(__glibc_unlikely (TCACHE_INACTIVE ()))
>   	return tcache_malloc_init (bytes);
>   
>         if (__glibc_likely (tc_idx < TCACHE_SMALL_BINS))
> @@ -3461,7 +3498,7 @@ __libc_free (void *mem)
>     check_inuse_chunk (arena_for_chunk (p), p);
>   
>   #if USE_TCACHE
> -  if (__glibc_likely (size < mp_.tcache_max_bytes && tcache != NULL))
> +  if (__glibc_likely (size < mp_.tcache_max_bytes))
>       {
>         /* Check to see if it's already in the tcache.  */
>         tcache_entry *e = (tcache_entry *) chunk2mem (p);
> @@ -3848,7 +3885,7 @@ __libc_calloc (size_t n, size_t elem_size)
>   
>     if (nb < mp_.tcache_max_bytes)
>       {
> -      if (__glibc_unlikely (tcache == NULL))
> +      if (__glibc_unlikely (TCACHE_INACTIVE ()))
>   	return tcache_calloc_init (bytes);
>   
>         size_t tc_idx = csize2tidx (nb);
> @@ -3982,7 +4019,7 @@ _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;
>   
> @@ -4042,7 +4079,7 @@ _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;
>   
> @@ -4104,7 +4141,7 @@ _int_malloc (mstate av, size_t bytes)
>   #if USE_TCACHE
>     INTERNAL_SIZE_T tcache_nb = 0;
>     size_t tc_idx = csize2tidx (nb);
> -  if (tcache != NULL && tc_idx < mp_.tcache_small_bins)
> +  if (tc_idx < mp_.tcache_small_bins)
>       tcache_nb = nb;
>     int return_cached = 0;
>   
>