From patchwork Fri Nov 27 02:34:29 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Carlos O'Donell X-Patchwork-Id: 9837 Received: (qmail 69694 invoked by alias); 27 Nov 2015 02:34:34 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 69179 invoked by uid 89); 27 Nov 2015 02:34:33 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.8 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com To: GNU C Library From: "Carlos O'Donell" Subject: [RFC] Calling realloc vs. mallo/memcpy/free and _IO_vasprintf. X-Enigmail-Draft-Status: N1110 Message-ID: <5657C135.1090807@redhat.com> Date: Thu, 26 Nov 2015 21:34:29 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0 MIME-Version: 1.0 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? --- Cheers, Carlos. 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';