diff mbox series

realloc: Return unchanged if request is within usable size

Message ID 20221125200948.2399996-1-siddhesh@sourceware.org
State New
Headers show
Series realloc: Return unchanged if request is within usable size | expand

Commit Message

Siddhesh Poyarekar Nov. 25, 2022, 8:09 p.m. UTC
If there is enough space in the chunk to satisfy the new size, return
the old pointer as is, thus avoiding any locks or reallocations.  The
only real place this has a benefit is in large chunks that tend to get
satisfied with mmap, since there is a large enough spare size (up to a
page) for it to matter.  For allocations on heap, the extra size is
typically barely a few bytes (up to 15) and it's unlikely that it would
make much difference in performance.

Also added a smoke test to ensure that the old pointer is returned
unchanged if the new size to realloc is within usable size of the old
pointer.

Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
---
 malloc/malloc.c      |  6 ++++++
 malloc/tst-realloc.c | 21 +++++++++++++++++++++
 2 files changed, 27 insertions(+)

Comments

Andreas Schwab Nov. 25, 2022, 8:40 p.m. UTC | #1
On Nov 25 2022, Siddhesh Poyarekar via Libc-alpha wrote:

> +  /* Smoke test to make sure that allocations do not move if they have enough
> +     space to expand in the chunk.  */
> +  for (size_t sz = 3; sz < 256 * 1024; sz += 2048)
> +    {
> +      p = realloc (NULL, sz);
> +      if (p == NULL)
> +	FAIL_EXIT1 ("realloc (NULL, 31) returned NULL.");

s/31/sz/

> +      size_t newsz = malloc_usable_size (p);
> +      printf ("size: %zu, usable size: %zu, extra: %zu\n",
> +	      sz, newsz, newsz - sz);
> +      void *new_p = realloc (p, newsz);
> +      if (new_p != p)

The compiler is allowed to optimize this to true under the assumption
that new_p == NULL (the only case where p is still a valid pointer).
Siddhesh Poyarekar Nov. 28, 2022, 2:23 p.m. UTC | #2
On 2022-11-25 15:40, Andreas Schwab wrote:
> On Nov 25 2022, Siddhesh Poyarekar via Libc-alpha wrote:
> 
>> +  /* Smoke test to make sure that allocations do not move if they have enough
>> +     space to expand in the chunk.  */
>> +  for (size_t sz = 3; sz < 256 * 1024; sz += 2048)
>> +    {
>> +      p = realloc (NULL, sz);
>> +      if (p == NULL)
>> +	FAIL_EXIT1 ("realloc (NULL, 31) returned NULL.");
> 
> s/31/sz/
> 
>> +      size_t newsz = malloc_usable_size (p);
>> +      printf ("size: %zu, usable size: %zu, extra: %zu\n",
>> +	      sz, newsz, newsz - sz);
>> +      void *new_p = realloc (p, newsz);
>> +      if (new_p != p)
> 
> The compiler is allowed to optimize this to true under the assumption
> that new_p == NULL (the only case where p is still a valid pointer).
> 

Hmm, I suppose something like this then?

     for (...)
       {
         ...
         volatile uintptr_t oldp = p;
         void *new_p = realloc (p, newsz)
         if (new_p != oldp)
           ...
       }

Thanks,
Sid
Andreas Schwab Nov. 28, 2022, 2:29 p.m. UTC | #3
On Nov 28 2022, Siddhesh Poyarekar via Libc-alpha wrote:

> Hmm, I suppose something like this then?
>
>     for (...)
>       {
>         ...
>         volatile uintptr_t oldp = p;
>         void *new_p = realloc (p, newsz)
>         if (new_p != oldp)
>           ...

You'll need some casts, but I don't think the volatile is needed.
diff mbox series

Patch

diff --git a/malloc/malloc.c b/malloc/malloc.c
index 2a61c8b5ee..3ef61bef34 100644
--- a/malloc/malloc.c
+++ b/malloc/malloc.c
@@ -1100,6 +1100,8 @@  static void munmap_chunk(mchunkptr p);
 static mchunkptr mremap_chunk(mchunkptr p, size_t new_size);
 #endif
 
+static size_t musable (void *mem);
+
 /* ------------------ MMAP support ------------------  */
 
 
@@ -3396,6 +3398,10 @@  __libc_realloc (void *oldmem, size_t bytes)
   if (__glibc_unlikely (mtag_enabled))
     *(volatile char*) oldmem;
 
+  /* If there's usable space in the current chunk, return as is.  */
+  if (bytes <= musable (oldmem))
+    return oldmem;
+
   /* chunk corresponding to oldmem */
   const mchunkptr oldp = mem2chunk (oldmem);
   /* its size */
diff --git a/malloc/tst-realloc.c b/malloc/tst-realloc.c
index 5eb62a770f..54dd0dd0b3 100644
--- a/malloc/tst-realloc.c
+++ b/malloc/tst-realloc.c
@@ -142,6 +142,27 @@  do_test (void)
 
   free (p);
 
+  /* Smoke test to make sure that allocations do not move if they have enough
+     space to expand in the chunk.  */
+  for (size_t sz = 3; sz < 256 * 1024; sz += 2048)
+    {
+      p = realloc (NULL, sz);
+      if (p == NULL)
+	FAIL_EXIT1 ("realloc (NULL, 31) returned NULL.");
+      size_t newsz = malloc_usable_size (p);
+      printf ("size: %zu, usable size: %zu, extra: %zu\n",
+	      sz, newsz, newsz - sz);
+      void *new_p = realloc (p, newsz);
+      if (new_p != p)
+	FAIL_EXIT1 ("Expanding (%zu bytes) to usable size (%zu) moved block",
+		    sz, newsz);
+      free (new_p);
+
+      /* We encountered a large enough extra size at least once.  */
+      if (newsz - sz > 1024)
+	break;
+    }
+
   return 0;
 }