[PATCH v8 1/2] malloc: add tcache support for large chunk caching

[Cupertino Miranda]

Hi Wilco,

> if (size <= TCACHE_SMALL_SIZE)
>    {
>       size_t tc_idx = csize2tidx (bytes);
>       if (tcache != NULL && tcache->entries[tc_idx] != NULL)
>          return tag_new_usable (tcache_get (tc_idx));
>     }
> else if (size <= mp_.tcache_max_bytes && tcache != NULL)
>    {
>      size_t tc_idx = large_csize2tidx (bytes);
>      void *victim = tcache_get_large (tc_idx, nb);
>      if (victim != NULL)
>         return tag_new_usable (victim);
>    }

Any reason why in your suggested code, you do not check for tcache != 
NULL as the first condition for both cases ?

> 
> @@ -3946,7 +4043,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_bins)
> +             if (tcache != NULL && nb < mp_.tcache_max_bytes)
> 
> This is not correct - this is only for small tcache (and will remain so).
>   
> @@ -4007,7 +4104,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_bins)
> +         if (tcache != NULL && tc_idx < mp_.tcache_max_bytes)
This was wrong, it was a typo, should have changed tc_idx to nb.
> 
> Likewise.
>   
> @@ -4069,7 +4166,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_bins)
> +  if (tcache != NULL && nb < mp_.tcache_max_bytes)
> 
> Likewise (while we could possibly add large tcache support here in the future, it's likely
> a bad idea to preload tcache with large blocks).
I am confused why this is actually wrong?

This change is not about adding large blocks support, it was rather 
about removing the need for tcache_bins struct field and make all 
tcache_put calls being checked against tcache_max_bytes, making 
tcache_bins not needed.

Now I wonder is any of this locations could be reached with a larger 
chunk then the supported by the original fixed sized bins (up to 1kb 
chunks) ? Otherwise even the original check against mp_.tcache_bins is 
not enough.

Cheers,
Cupertino