[PATCH 8/9] malloc: remove fastbin comments

[DJ Delorie]

Dev Jain <dev.jain@arm.com> writes:
> Now that all the fastbin code is gone, remove the remaining comments
> referencing fastbins.

Also, someone will need to update the MallocInternals wiki page:
https://sourceware.org/glibc/wiki/MallocInternals
(preferably to have both before/after information)

In the future, I think it makes more sense to delete comments along with
the thing they comment on, when they're together (like the big comments
that come before the fastbins code that was deleted in a previous
patch).  There's a lot of standalone documentation in malloc.c but some
of the below deletions could have been part of the previous patch where
the thing they related to was removed.

>      Tuning options that are also dynamically changeable via mallopt:
>  
> -    DEFAULT_MXFAST             64 (for 32bit), 128 (for 64bit)
> +    DEFAULT_MXFAST (deprecated)            64 (for 32bit), 128 (for 64bit)

Ok.

>  /* Safe-Linking:
>     Use randomness from ASLR (mmap_base) to protect single-linked lists
> -   of Fast-Bins and TCache.  That is, mask the "next" pointers of the
> +   of TCache.  That is, mask the "next" pointers of the
>     lists' chunks, and also perform allocation alignment checks on them.

Ok.

>    Symbol            param #   default    allowed param values
> -  M_MXFAST          1         64         0-80  (0 disables fastbins)
> +  M_MXFAST           1         64         0-80  (0 was used to disable fastbins, now deprecated)
>    M_TRIM_THRESHOLD -1         128*1024   any   (-1U disables trimming)

Like too long.

>  
> -/*
> -  M_MXFAST is the maximum request size used for "fastbins", special bins
> -  that hold returned chunks without consolidating their spaces. This
> -  enables future requests for chunks of the same size to be handled
> -  very quickly, but can increase fragmentation, and thus increase the
> -  overall memory footprint of a program.
> -
> -  This malloc manages fastbins very conservatively yet still
> -  efficiently, so fragmentation is rarely a problem for values less
> -  than or equal to the default.  The maximum supported value of MXFAST
> -  is 80. You wouldn't want it any higher than this anyway.  Fastbins
> -  are designed especially for use with many small structs, objects or
> -  strings -- the default handles structs/objects/arrays with sizes up
> -  to 8 4byte fields, or small strings representing words, tokens,
> -  etc. Using fastbins for larger objects normally worsens
> -  fragmentation without improving speed.
> -
> -  M_MXFAST is set in REQUEST size units. It is internally used in
> -  chunksize units, which adds padding and alignment.  You can reduce
> -  M_MXFAST to 0 to disable all use of fastbins.  This causes the malloc
> -  algorithm to be a closer approximation of fifo-best-fit in all cases,
> -  not just for larger requests, but will generally cause it to be
> -  slower.
> -*/
> -
> -

Ok.

>  /* M_MXFAST is a standard SVID/XPG tuning option, usually listed in malloc.h */
>  #ifndef M_MXFAST
>  #define M_MXFAST            1

We can get rid of this too; it's in malloc.h for backwards compatibility
purposes.

> -
> -     3. Chunks in fastbins are treated as allocated chunks from the
> -	point of view of the chunk allocator.  They are consolidated
> -	with their neighbors only in bulk, in malloc_consolidate.
>  */

Ok.

> @@ -1725,23 +1695,6 @@ unlink_chunk (mstate av, mchunkptr p)
>  
> -/*
> -   Fastbins
> -
> -    An array of lists holding recently freed small chunks.  Fastbins
> -    are not doubly linked.  It is faster to single-link them, and
> -    since chunks are never removed from the middles of these lists,
> -    double linking is not necessary. Also, unlike regular bins, they
> -    are not even processed in FIFO order (they use faster LIFO) since
> -    ordering doesn't much matter in the transient contexts in which
> -    fastbins are normally used.
> -
> -    Chunks in fastbins keep their inuse bit set, so they cannot
> -    be consolidated with other free chunks. malloc_consolidate
> -    releases all chunks in fastbins and consolidates them with
> -    other free chunks.
> - */
> -

Ok.

>  
> -/*
> -   Set value of max_fast.
> -   Use impossibly small value if 0.
> -   Precondition: there are no existing fastbin chunks in the main arena.
> -   Since do_check_malloc_state () checks this, we call malloc_consolidate ()
> -   before changing max_fast.  Note other arenas will leak their fast bin
> -   entries if max_fast is reduced.
> - */

Ok.

>  
> -/*
> -   have_fastchunks indicates that there are probably some fastbin chunks.
> -   It is set true on entering a chunk into any fastbin, and cleared early in
> -   malloc_consolidate.  The value is approximate since it may be set when there
> -   are no fastbin chunks, or it may be clear even if there are fastbin chunks
> -   available.  Given it's sole purpose is to reduce number of redundant calls to
> -   malloc_consolidate, it does not affect correctness.  As a result we can safely
> -   use relaxed atomic accesses.
> - */
> -

Ok.

>  
> -/*
> -   Properties of chunks recycled from fastbins
> - */
> -
>  static void
>  do_check_remalloced_chunk (mstate av, mchunkptr p, INTERNAL_SIZE_T s)
>  {

This function is no longer used without fastbins and can be removed,
too.  It's what the comment you're removing is referring to.

> @@ -2195,8 +2125,7 @@ do_check_malloced_chunk (mstate av, mchunkptr p, INTERNAL_SIZE_T s)
>       chunk borders either a previously allocated and still in-use
>       chunk, or the base of its memory arena. This is ensured
>       by making all allocations from the `lowest' part of any found
> -     chunk.  This does not necessarily hold however for chunks
> -     recycled via fastbins.
> +     chunk.
>     */

Ok.

> -  /*
> -     If this is a large request, consolidate fastbins before continuing.
> -     While it might look excessive to kill all fastbins before
> -     even seeing if there is space available, this avoids
> -     fragmentation problems normally associated with fastbins.
> -     Also, in practice, programs tend to have runs of either small or
> -     large requests, but less often mixtures, so consolidation is not
> -     invoked all that often in most programs. And the programs that
> -     it is called frequently in otherwise tend to fragment.
> -   */
> -

Ok.

>  static void
>  _int_free_maybe_consolidate (mstate av, INTERNAL_SIZE_T size)
>  {
> -  /* Unless max_fast is 0, we don't know if there are fastbins
> -     bordering top, so we cannot tell for sure whether threshold has
> -     been reached unless fastbins are consolidated.  But we don't want
> -     to consolidate on each free.  As a compromise, consolidation is
> -     performed if FASTBIN_CONSOLIDATION_THRESHOLD is reached.  */
> +  /* We don't want to consolidate on each free.  As a compromise, consolidation
> +     is performed if FASTBIN_CONSOLIDATION_THRESHOLD is reached.  */
>    if (size >= FASTBIN_CONSOLIDATION_THRESHOLD)
>      {
>        if (av == &main_arena)

Ok.