[PATCH] libio/vasprintf.c: Always use realloc.

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

The asprintf/vasprintf implementations calls into _IO_vasprintf
to carry out operations on a string-based FILE* object. When
the work is complete and the buffer filled with the result the
implementation null-terminates and returns the string which is
passed back to the caller of asprintf.

During underflow the string-based buffer might have grown, and
conversely the buffer might be exactly the right size, but short
by 1-byte for the NULL terminator. Therefore the implementation
chooses to realloc the buffer to return back any excess memory
or get 1-byte more for the NULL terminator.

In 2000 the implementation was changed by Ulrich Drpper to
make a local deicsion to use realloc vs. malloc/memcpy/free
based on a heuristic and in order to reduce memory fragmentation.
The idea is that if the buffer is only just bigger than what the
user needs the realloc will split the chunk and increase
fragmentation. While this is true, this decision should not be
made at the _IO_vasprintf level. The allocator should be making
this decision based on known fragmentation estimates and a tunable
fragmentation parameter which could be used to control when realloc
splits a chunk and when it decides to malloc/memcpy/free.

Therefore we remove the local heuristic. This should increase
performance slightly at the cost of fragmentation. Future malloc
work to handle fragmentation should be pursued. If someone measures
fragmentation and tracks it back down to _IO_vasprintf, we add a
comment to guide the reader to what we consider is best practice:
enhance realloc to handle fragmentation decisions.

OK to checkin?

No regressions on x86_64.

Cheers,
Carlos.

2015-11-27  Carlos O'Donell  <carlos@redhat.com>

	* libio/vasprintf.c (_IO_vasprintf): Always use realloc.

diff --git a/libio/vasprintf.c b/libio/vasprintf.c
index 61cdfdd..5b39ee7 100644
--- a/libio/vasprintf.c
+++ b/libio/vasprintf.c
@@ -41,7 +41,6 @@ _IO_vasprintf (char **result_ptr, const char *format, _IO_va_list args)
   _IO_strfile sf;
   int ret;
   _IO_size_t needed;
-  _IO_size_t allocated;
   /* No need to clear the memory here (unlike for open_memstream) since
      we know we will never seek on the stream.  */
   string = (char *) malloc (init_string_size);
@@ -62,24 +61,18 @@ _IO_vasprintf (char **result_ptr, const char *format, _IO_va_list args)
       free (sf._sbf._f._IO_buf_base);
       return ret;
     }
-  /* Only use realloc if the size we need is of the same (binary)
-     order of magnitude then the memory we allocated.  */
   needed = sf._sbf._f._IO_write_ptr - sf._sbf._f._IO_write_base + 1;
-  allocated = sf._sbf._f._IO_write_end - sf._sbf._f._IO_write_base;
-  if ((allocated >> 1) <= needed)
-    *result_ptr = (char *) realloc (sf._sbf._f._IO_buf_base, needed);
-  else
-    {
-      *result_ptr = (char *) malloc (needed);
-      if (*result_ptr != NULL)
-	{
-	  memcpy (*result_ptr, sf._sbf._f._IO_buf_base, needed - 1);
-	  free (sf._sbf._f._IO_buf_base);
-	}
-      else
-	/* We have no choice, use the buffer we already have.  */
-	*result_ptr = (char *) realloc (sf._sbf._f._IO_buf_base, needed);
-    }
+  /* We are either shrinking the buffer, doing nothing, or growing
+     the buffer by 1 byte for the NULL.  In all of these cases it's
+     fastest to call realloc.  At the cost of more memory we could
+     choose not to realloc and return a larger buffer, but tuning
+     that requires some thought.  At one point this code was expanded
+     to consider realloc vs. malloc/memcpy/free as a method for
+     reducing fragmentation caused by realloc, but was later removed
+     because such decisions are tuning parameters that should be part
+     of the realloc decisions made by malloc e.g. fragmentation vs.
+     making a copy.  */
+  *result_ptr = (char *) realloc (sf._sbf._f._IO_buf_base, needed);
   if (*result_ptr == NULL)
     *result_ptr = sf._sbf._f._IO_buf_base;
   (*result_ptr)[needed - 1] = '\0';