[COMMITTED] malloc/malloc.c : Remove nested function mi_arena.

["Carlos O'Donell" <carlos at redhat dot com>]

I've committed the following patch to mechanically remove
the nested function mi_arena. This is a very simple
refactoring that's easy to review and prove. Which is what
we want from any refactoring.

We do this because it's harder to debug nested functions,
and it allows non-GNU compilers to compile this code, and
this code is not performance critical.

The resulting code is ~100 bytes larger because the compiler
has to handle the function arguments and pointers to size_t
objects. This code, and in particular calling malloc_info, is
not considered in the hot path of average applications.

I've added `inline' to the function to indicate to the compiler
that we'd like it inlined if beneficial.

Tested on x86_64 with no regressions.

Tested before and after with simple malloc_info application
that prints stats to verify statistics look sane. Test passes.

Red Hat in particular is looking to extend and change this
interface to support JSON and provide better statistics,
therefore we do what is minimally required here to remove the
nested function, as opposed to an inlined version as suggested
by Ondrej Bilka.

Cheers,
Carlos.

2014-05-26  Carlos O'Donell  <carlos@redhat.com>

	* malloc/malloc.c (mi_arena): New function.
	(malloc_info): Remove nested function mi_arena. Call non-nested
	function mi_arena.

diff --git a/malloc/malloc.c b/malloc/malloc.c
index 1120d4d..92ad9f9 100644
--- a/malloc/malloc.c
+++ b/malloc/malloc.c
@@ -4986,154 +4986,162 @@ __posix_memalign (void **memptr, size_t alignment, size_t size)
 }
 weak_alias (__posix_memalign, posix_memalign)
 
-
-int
-malloc_info (int options, FILE *fp)
+static inline void
+mi_arena (mstate ar_ptr,
+	  FILE *fp,
+	  size_t *total_nblocks,
+	  size_t *total_nfastblocks,
+	  size_t *total_avail,
+	  size_t *total_fastavail,
+	  size_t *total_system,
+	  size_t *total_max_system,
+	  size_t *total_aspace,
+	  size_t *total_aspace_mprotect)
 {
-  /* For now, at least.  */
-  if (options != 0)
-    return EINVAL;
-
   int n = 0;
-  size_t total_nblocks = 0;
-  size_t total_nfastblocks = 0;
-  size_t total_avail = 0;
-  size_t total_fastavail = 0;
-  size_t total_system = 0;
-  size_t total_max_system = 0;
-  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++);
+  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];
+  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);
+  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;
-              }
+          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;
+          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;
 
-        sizes[i].total = sizes[i].count * sizes[i].to;
-      }
+      sizes[i].total = sizes[i].count * sizes[i].to;
+    }
 
 
-    mbinptr bin;
-    struct malloc_chunk *r;
+  mbinptr bin;
+  struct malloc_chunk *r;
 
-    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 = 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;
-      }
+      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;
+    }
 
-    mutex_unlock (&ar_ptr->mutex);
+  mutex_unlock (&ar_ptr->mutex);
 
-    total_nfastblocks += nfastblocks;
-    total_fastavail += fastavail;
+  *total_nfastblocks += nfastblocks;
+  *total_fastavail += fastavail;
 
-    total_nblocks += nblocks;
-    total_avail += avail;
+  *total_nblocks += nblocks;
+  *total_avail += avail;
 
-    for (size_t i = 0; i < nsizes; ++i)
-      if (sizes[i].count != 0 && i != NFASTBINS)
-        fprintf (fp, "							      \
+  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);
+               sizes[i].from, sizes[i].to, sizes[i].total, sizes[i].count);
 
-    if (sizes[NFASTBINS].count != 0)
-      fprintf (fp, "\
+  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);
+             sizes[NFASTBINS].from, sizes[NFASTBINS].to,
+             sizes[NFASTBINS].total, sizes[NFASTBINS].count);
 
-    total_system += ar_ptr->system_mem;
-    total_max_system += ar_ptr->max_system_mem;
+  *total_system += ar_ptr->system_mem;
+  *total_max_system += ar_ptr->max_system_mem;
 
-    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);
+  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);
 
-    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 (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;
+    }
 
-    fputs ("</heap>\n", fp);
-  }
+  fputs ("</heap>\n", fp);
+}
+
+int
+malloc_info (int options, FILE *fp)
+{
+  /* For now, at least.  */
+  if (options != 0)
+    return EINVAL;
+
+  size_t total_nblocks = 0;
+  size_t total_nfastblocks = 0;
+  size_t total_avail = 0;
+  size_t total_fastavail = 0;
+  size_t total_system = 0;
+  size_t total_max_system = 0;
+  size_t total_aspace = 0;
+  size_t total_aspace_mprotect = 0;
 
   if (__malloc_initialized < 0)
     ptmalloc_init ();
@@ -5144,7 +5152,9 @@ malloc_info (int options, FILE *fp)
   mstate ar_ptr = &main_arena;
   do
     {
-      mi_arena (ar_ptr);
+      mi_arena (ar_ptr, fp, &total_nblocks, &total_nfastblocks, &total_avail,
+		&total_fastavail, &total_system, &total_max_system,
+		&total_aspace, &total_aspace_mprotect);
       ar_ptr = ar_ptr->next;
     }
   while (ar_ptr != &main_arena);
---

Cheers,
Carlos.