This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Inline useless nested function mi_arena.


On Wed, Feb 26, 2014 at 10:49:24PM +0100, OndÅej BÃlka wrote:
> Hi,
> 
> I noticed a minor simplification in malloc_info, that nested function
> mi_arena looks useless as it gets called at only one site and I do not
> have to rename its argument. The resulting patch is big due of
> reindenting but is simple in principle.
> 
> Is there any deep reason to keep it?

I agree that it makes more sense to just inline it since there's just
one call site and especially since breaking it out into a function
will result in a lot of state being passed on to the function, which
is a waste.  If Carlos agrees, I think this patch is good to go in.
If not, then we'll have to wait to hear why he thinks the function is
necessary.

Siddhesh

> 
> 	* malloc/malloc.c (malloc_info): Inline mi_arena.
> 
> diff --git a/malloc/malloc.c b/malloc/malloc.c
> index 516120f..fe51725 100644
> --- a/malloc/malloc.c
> +++ b/malloc/malloc.c
> @@ -4894,147 +4894,143 @@ malloc_info (int options, FILE *fp)
>    size_t total_aspace = 0;
>    size_t total_aspace_mprotect = 0;
>  
> -  void
> -  mi_arena (mstate ar_ptr)
> -  {
> -    fprintf (fp, "<heap nr=\"%d\">\n<sizes>\n", n++);
> -
> -    size_t nblocks = 0;
> -    size_t nfastblocks = 0;
> -    size_t avail = 0;
> -    size_t fastavail = 0;
> -    struct
> -    {
> -      size_t from;
> -      size_t to;
> -      size_t total;
> -      size_t count;
> -    } sizes[NFASTBINS + NBINS - 1];
> -#define nsizes (sizeof (sizes) / sizeof (sizes[0]))
> -
> -    mutex_lock (&ar_ptr->mutex);
>  
> -    for (size_t i = 0; i < NFASTBINS; ++i)
> -      {
> -        mchunkptr p = fastbin (ar_ptr, i);
> -        if (p != NULL)
> -          {
> -            size_t nthissize = 0;
> -            size_t thissize = chunksize (p);
>  
> -            while (p != NULL)
> -              {
> -                ++nthissize;
> -                p = p->fd;
> -              }
> +  if (__malloc_initialized < 0)
> +    ptmalloc_init ();
>  
> -            fastavail += nthissize * thissize;
> -            nfastblocks += nthissize;
> -            sizes[i].from = thissize - (MALLOC_ALIGNMENT - 1);
> -            sizes[i].to = thissize;
> -            sizes[i].count = nthissize;
> -          }
> -        else
> -          sizes[i].from = sizes[i].to = sizes[i].count = 0;
> +  fputs ("<malloc version=\"1\">\n", fp);
>  
> -        sizes[i].total = sizes[i].count * sizes[i].to;
> -      }
> +  /* Iterate over all arenas currently in use.  */
> +  mstate ar_ptr = &main_arena;
> +  do
> +    {
> +      fprintf (fp, "<heap nr=\"%d\">\n<sizes>\n", n++);
>  
> +      size_t nblocks = 0;
> +      size_t nfastblocks = 0;
> +      size_t avail = 0;
> +      size_t fastavail = 0;
> +      struct
> +      {
> +	size_t from;
> +	size_t to;
> +	size_t total;
> +	size_t count;
> +      } sizes[NFASTBINS + NBINS - 1];
> +#define nsizes (sizeof (sizes) / sizeof (sizes[0]))
>  
> -    mbinptr bin;
> -    struct malloc_chunk *r;
> +      mutex_lock (&ar_ptr->mutex);
>  
> -    for (size_t i = 1; i < NBINS; ++i)
> -      {
> -        bin = bin_at (ar_ptr, i);
> -        r = bin->fd;
> -        sizes[NFASTBINS - 1 + i].from = ~((size_t) 0);
> -        sizes[NFASTBINS - 1 + i].to = sizes[NFASTBINS - 1 + i].total
> -                                        = sizes[NFASTBINS - 1 + i].count = 0;
> -
> -        if (r != NULL)
> -          while (r != bin)
> -            {
> -              ++sizes[NFASTBINS - 1 + i].count;
> -              sizes[NFASTBINS - 1 + i].total += r->size;
> -              sizes[NFASTBINS - 1 + i].from
> -                = MIN (sizes[NFASTBINS - 1 + i].from, r->size);
> -              sizes[NFASTBINS - 1 + i].to = MAX (sizes[NFASTBINS - 1 + i].to,
> -                                                 r->size);
> -
> -              r = r->fd;
> -            }
> +      for (size_t i = 0; i < NFASTBINS; ++i)
> +	{
> +	  mchunkptr p = fastbin (ar_ptr, i);
> +	  if (p != NULL)
> +	    {
> +	      size_t nthissize = 0;
> +	      size_t thissize = chunksize (p);
> +
> +	      while (p != NULL)
> +		{
> +		  ++nthissize;
> +		  p = p->fd;
> +		}
> +
> +	      fastavail += nthissize * thissize;
> +	      nfastblocks += nthissize;
> +	      sizes[i].from = thissize - (MALLOC_ALIGNMENT - 1);
> +	      sizes[i].to = thissize;
> +	      sizes[i].count = nthissize;
> +	    }
> +	  else
> +	    sizes[i].from = sizes[i].to = sizes[i].count = 0;
>  
> -        if (sizes[NFASTBINS - 1 + i].count == 0)
> -          sizes[NFASTBINS - 1 + i].from = 0;
> -        nblocks += sizes[NFASTBINS - 1 + i].count;
> -        avail += sizes[NFASTBINS - 1 + i].total;
> -      }
> +	  sizes[i].total = sizes[i].count * sizes[i].to;
> +	}
>  
> -    mutex_unlock (&ar_ptr->mutex);
>  
> -    total_nfastblocks += nfastblocks;
> -    total_fastavail += fastavail;
> +      mbinptr bin;
> +      struct malloc_chunk *r;
>  
> -    total_nblocks += nblocks;
> -    total_avail += avail;
> +      for (size_t i = 1; i < NBINS; ++i)
> +	{
> +	  bin = bin_at (ar_ptr, i);
> +	  r = bin->fd;
> +	  sizes[NFASTBINS - 1 + i].from = ~((size_t) 0);
> +	  sizes[NFASTBINS - 1 + i].to = sizes[NFASTBINS - 1 + i].total
> +					  = sizes[NFASTBINS - 1 + i].count = 0;
> +
> +	  if (r != NULL)
> +	    while (r != bin)
> +	      {
> +		++sizes[NFASTBINS - 1 + i].count;
> +		sizes[NFASTBINS - 1 + i].total += r->size;
> +		sizes[NFASTBINS - 1 + i].from
> +		  = MIN (sizes[NFASTBINS - 1 + i].from, r->size);
> +		sizes[NFASTBINS - 1 + i].to = MAX (sizes[NFASTBINS - 1 + i].to,
> +						   r->size);
> +
> +		r = r->fd;
> +	      }
> +
> +	  if (sizes[NFASTBINS - 1 + i].count == 0)
> +	    sizes[NFASTBINS - 1 + i].from = 0;
> +	  nblocks += sizes[NFASTBINS - 1 + i].count;
> +	  avail += sizes[NFASTBINS - 1 + i].total;
> +	}
>  
> -    for (size_t i = 0; i < nsizes; ++i)
> -      if (sizes[i].count != 0 && i != NFASTBINS)
> -        fprintf (fp, "							      \
> -<size from=\"%zu\" to=\"%zu\" total=\"%zu\" count=\"%zu\"/>\n",
> -                 sizes[i].from, sizes[i].to, sizes[i].total, sizes[i].count);
> +      mutex_unlock (&ar_ptr->mutex);
>  
> -    if (sizes[NFASTBINS].count != 0)
> -      fprintf (fp, "\
> -<unsorted from=\"%zu\" to=\"%zu\" total=\"%zu\" count=\"%zu\"/>\n",
> -               sizes[NFASTBINS].from, sizes[NFASTBINS].to,
> -               sizes[NFASTBINS].total, sizes[NFASTBINS].count);
> +      total_nfastblocks += nfastblocks;
> +      total_fastavail += fastavail;
>  
> -    total_system += ar_ptr->system_mem;
> -    total_max_system += ar_ptr->max_system_mem;
> +      total_nblocks += nblocks;
> +      total_avail += avail;
>  
> -    fprintf (fp,
> -             "</sizes>\n<total type=\"fast\" count=\"%zu\" size=\"%zu\"/>\n"
> -             "<total type=\"rest\" count=\"%zu\" size=\"%zu\"/>\n"
> -             "<system type=\"current\" size=\"%zu\"/>\n"
> -             "<system type=\"max\" size=\"%zu\"/>\n",
> -             nfastblocks, fastavail, nblocks, avail,
> -             ar_ptr->system_mem, ar_ptr->max_system_mem);
> +      for (size_t i = 0; i < nsizes; ++i)
> +	if (sizes[i].count != 0 && i != NFASTBINS)
> +	  fprintf (fp, "							      \
> +  <size from=\"%zu\" to=\"%zu\" total=\"%zu\" count=\"%zu\"/>\n",
> +		   sizes[i].from, sizes[i].to, sizes[i].total, sizes[i].count);
>  
> -    if (ar_ptr != &main_arena)
> -      {
> -        heap_info *heap = heap_for_ptr (top (ar_ptr));
> -        fprintf (fp,
> -                 "<aspace type=\"total\" size=\"%zu\"/>\n"
> -                 "<aspace type=\"mprotect\" size=\"%zu\"/>\n",
> -                 heap->size, heap->mprotect_size);
> -        total_aspace += heap->size;
> -        total_aspace_mprotect += heap->mprotect_size;
> -      }
> -    else
> -      {
> -        fprintf (fp,
> -                 "<aspace type=\"total\" size=\"%zu\"/>\n"
> -                 "<aspace type=\"mprotect\" size=\"%zu\"/>\n",
> -                 ar_ptr->system_mem, ar_ptr->system_mem);
> -        total_aspace += ar_ptr->system_mem;
> -        total_aspace_mprotect += ar_ptr->system_mem;
> -      }
> +      if (sizes[NFASTBINS].count != 0)
> +	fprintf (fp, "\
> +  <unsorted from=\"%zu\" to=\"%zu\" total=\"%zu\" count=\"%zu\"/>\n",
> +		 sizes[NFASTBINS].from, sizes[NFASTBINS].to,
> +		 sizes[NFASTBINS].total, sizes[NFASTBINS].count);
>  
> -    fputs ("</heap>\n", fp);
> -  }
> +      total_system += ar_ptr->system_mem;
> +      total_max_system += ar_ptr->max_system_mem;
>  
> -  if (__malloc_initialized < 0)
> -    ptmalloc_init ();
> +      fprintf (fp,
> +	       "</sizes>\n<total type=\"fast\" count=\"%zu\" size=\"%zu\"/>\n"
> +	       "<total type=\"rest\" count=\"%zu\" size=\"%zu\"/>\n"
> +	       "<system type=\"current\" size=\"%zu\"/>\n"
> +	       "<system type=\"max\" size=\"%zu\"/>\n",
> +	       nfastblocks, fastavail, nblocks, avail,
> +	       ar_ptr->system_mem, ar_ptr->max_system_mem);
>  
> -  fputs ("<malloc version=\"1\">\n", fp);
> +      if (ar_ptr != &main_arena)
> +	{
> +	  heap_info *heap = heap_for_ptr (top (ar_ptr));
> +	  fprintf (fp,
> +		   "<aspace type=\"total\" size=\"%zu\"/>\n"
> +		   "<aspace type=\"mprotect\" size=\"%zu\"/>\n",
> +		   heap->size, heap->mprotect_size);
> +	  total_aspace += heap->size;
> +	  total_aspace_mprotect += heap->mprotect_size;
> +	}
> +      else
> +	{
> +	  fprintf (fp,
> +		   "<aspace type=\"total\" size=\"%zu\"/>\n"
> +		   "<aspace type=\"mprotect\" size=\"%zu\"/>\n",
> +		   ar_ptr->system_mem, ar_ptr->system_mem);
> +	  total_aspace += ar_ptr->system_mem;
> +	  total_aspace_mprotect += ar_ptr->system_mem;
> +	}
>  
> -  /* Iterate over all arenas currently in use.  */
> -  mstate ar_ptr = &main_arena;
> -  do
> -    {
> -      mi_arena (ar_ptr);
> +      fputs ("</heap>\n", fp);
>        ar_ptr = ar_ptr->next;
>      }
>    while (ar_ptr != &main_arena);

Attachment: pgpeOTXDUSZbV.pgp
Description: PGP signature


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]