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]

[RFC] Calling realloc vs. mallo/memcpy/free and _IO_vasprintf.


While answering a question on libc-help about asprintf I happened
to look at _IO_vasprintf and noticed some code which tries to
decide if it should do a realloc vs. malloc/memcpy/free.

In this particular case we should always be shrinking the buffer.
For example _IO_vfprintf may have expanded the stream buffer but
now no longer needs the extra space. If we are growing the buffer
then something is very very wrong since that means _IO_vpfrintf
will have failed due to lack of memory, and we handled that case
earlier in _IO_vasprintf.

So again, if we are only shrinking the buffer, then realloc has
to be the most optimal operation in all cases. It would seem to
me that an allocation shrink like this would almost never result
in an allocation move, but rather malloc would just split the
allocation and reuse the remainder for other malloc calls.

So why the micro-optimization?

Is my assumption of always shrinking the buffer wrong?

If my assumption is right, and my logic is right, wouldn't the
following patch be a natural cleanup?

diff --git a/libio/vasprintf.c b/libio/vasprintf.c
index 61cdfdd..fff8c54 100644
--- a/libio/vasprintf.c
+++ b/libio/vasprintf.c
@@ -62,24 +62,13 @@ _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 or doing nothing, otherwise
+     _IO_vfprintf would have failed itself being unable to grow the
+     buffer to the needed size.  Therefore it's always fastest here to
+     call realloc.  */
+  *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';
---

Cheers,
Carlos.


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