[PATCH] mempool: Micro-optimize realloc() to avoid malloc and memcpy

[Pekka Enberg]

If object size is big enough to accommodate requested buffer size for
realloc(), skip malloc() and memcpy() as micro-optimization.

Signed-off-by: Pekka Enberg <pen...@cloudius-systems.com>
---
core/mempool.cc | 3 +++
1 file changed, 3 insertions(+)

diff --git a/core/mempool.cc b/core/mempool.cc
index 223b1ea..9cde5f1 100644
--- a/core/mempool.cc
+++ b/core/mempool.cc
@@ -1316,6 +1316,9 @@ static inline void* std_realloc(void* object, size_t size)
}

size_t old_size = object_size(object);
+ if (old_size >= size) {
+ return object;
+ }
size_t copy_size = size > old_size ? old_size : size;
void* ptr = malloc(size);
if (ptr) {
--
1.9.3

[Paweł Dziepak]

2014-08-13 13:57 GMT+02:00 Pekka Enberg <pen...@cloudius-systems.com>:

If object size is big enough to accommodate requested buffer size for
realloc(), skip malloc() and memcpy() as micro-optimization.

Signed-off-by: Pekka Enberg <pen...@cloudius-systems.com>
---
 core/mempool.cc | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/core/mempool.cc b/core/mempool.cc
index 223b1ea..9cde5f1 100644
--- a/core/mempool.cc
+++ b/core/mempool.cc
@@ -1316,6 +1316,9 @@ static inline void* std_realloc(void* object, size_t size)
     }

     size_t old_size = object_size(object);
+    if (old_size >= size) {

That makes it impossible to shrink buffers. What about something like: old_size >= size && size >= old_size / 2 ?

 

+        return object;
+    }
     size_t copy_size = size > old_size ? old_size : size;
     void* ptr = malloc(size);
     if (ptr) {

--
1.9.3

--
You received this message because you are subscribed to the Google Groups "OSv Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to osv-dev+u...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Pekka Enberg]

On Wed, Aug 13, 2014 at 3:08 PM, Paweł Dziepak <pdzi...@quarnos.org> wrote:
>> @@ -1316,6 +1316,9 @@ static inline void* std_realloc(void* object, size_t
>> size)
>> }
>>
>> size_t old_size = object_size(object);
>> + if (old_size >= size) {
>
> That makes it impossible to shrink buffers. What about something like:
> old_size >= size && size >= old_size / 2 ?

IIRC, the C standard does not guarantee that realloc() ever shrinks
the buffer so applications cannot rely on that. If glibc's realloc()
does that, we probably should do too. However, looking at the glibc's
implementation, I don't think it does. Am I reading it wrong?

- Pekka

[Nadav Har'El]

On Wed, Aug 13, 2014 at 3:18 PM, Pekka Enberg <pen...@cloudius-systems.com> wrote:

On Wed, Aug 13, 2014 at 3:08 PM, Paweł Dziepak <pdzi...@quarnos.org> wrote:
>> @@ -1316,6 +1316,9 @@ static inline void* std_realloc(void* object, size_t
>> size)
>>      }
>>
>>      size_t old_size = object_size(object);
>> +    if (old_size >= size) {
>
> That makes it impossible to shrink buffers. What about something like:
> old_size >= size && size >= old_size / 2 ?

IIRC, the C standard does not guarantee that realloc() ever shrinks
the buffer so applications cannot rely on that.

If the application can't rely on realloc() shrinking the buffer, why does it
call it at all? In which application did you find this "micro-optimization"

helpful?

 

If glibc's realloc()
does that, we probably should do too. However, looking at the glibc's
implementation, I don't think it does. Am I reading it wrong?

- Pekka

--
You received this message because you are subscribed to the Google Groups "OSv Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to osv-dev+u...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

--

Nadav Har'El

n...@cloudius-systems.com

[Pekka Enberg]

On Wed, Aug 13, 2014 at 3:26 PM, Nadav Har'El <n...@cloudius-systems.com> wrote:
> If the application can't rely on realloc() shrinking the buffer, why does it
> call it at all? In which application did you find this "micro-optimization"
> helpful?

realloc() is called to *grow* the buffer... Redis uses realloc()
rather heavily but as this is a micro-optimization I only saw minor
gains that could be just noise. ;-)

- Pekka

[Nadav Har'El]

On Wed, Aug 13, 2014 at 4:00 PM, Pekka Enberg <pen...@cloudius-systems.com> wrote:

On Wed, Aug 13, 2014 at 3:26 PM, Nadav Har'El <n...@cloudius-systems.com> wrote:
> If the application can't rely on realloc() shrinking the buffer, why does it
> call it at all? In which application did you find this "micro-optimization"
> helpful?

realloc() is called to *grow* the buffer...

I'm not disputing this patch, just being curious -

realloc() is normally used in code that determines that the buffer needs to grow, e.g.,

     if (neededlen > bufsize) {

          buf = realloc(buf, neededlen);

     }

In such code, realloc() will not be called with a *smaller* size. If they do, they were probably hoping to really free some space.

But this is just my guess. I might be completely wrong.

You can easily add a tracepoint to this realloc() case and see how many shrinking-realloc() calls you actually get.

 

Redis uses realloc()
rather heavily but as this is a micro-optimization I only saw minor
gains that could be just noise. ;-)

- Pekka

[Glauber Costa]

On Wed, Aug 13, 2014 at 5:25 PM, Nadav Har'El <n...@cloudius-systems.com> wrote:

On Wed, Aug 13, 2014 at 4:00 PM, Pekka Enberg <pen...@cloudius-systems.com> wrote:

On Wed, Aug 13, 2014 at 3:26 PM, Nadav Har'El <n...@cloudius-systems.com> wrote:
> If the application can't rely on realloc() shrinking the buffer, why does it
> call it at all? In which application did you find this "micro-optimization"
> helpful?

realloc() is called to *grow* the buffer...

I'm not disputing this patch, just being curious -

realloc() is normally used in code that determines that the buffer needs to grow, e.g.,

     if (neededlen > bufsize) {

          buf = realloc(buf, neededlen);

     }

In such code, realloc() will not be called with a *smaller* size. If they do, they were probably hoping to really free some space.

But this is just my guess. I might be completely wrong.

You can easily add a tracepoint to this realloc() case and see how many shrinking-realloc() calls you actually get.

none.
 

 

Redis uses realloc()
rather heavily but as this is a micro-optimization I only saw minor
gains that could be just noise. ;-)

- Pekka

--

Nadav Har'El

n...@cloudius-systems.com

--

[Raphael S. Carvalho]

On Wed, Aug 13, 2014 at 10:26 AM, Glauber Costa <glo...@cloudius-systems.com> wrote:

On Wed, Aug 13, 2014 at 5:25 PM, Nadav Har'El <n...@cloudius-systems.com> wrote:

On Wed, Aug 13, 2014 at 4:00 PM, Pekka Enberg <pen...@cloudius-systems.com> wrote:

On Wed, Aug 13, 2014 at 3:26 PM, Nadav Har'El <n...@cloudius-systems.com> wrote:
> If the application can't rely on realloc() shrinking the buffer, why does it
> call it at all? In which application did you find this "micro-optimization"
> helpful?

realloc() is called to *grow* the buffer...

I'm not disputing this patch, just being curious -

realloc() is normally used in code that determines that the buffer needs to grow, e.g.,

     if (neededlen > bufsize) {

          buf = realloc(buf, neededlen);

     }

In such code, realloc() will not be called with a *smaller* size. If they do, they were probably hoping to really free some space.

But this is just my guess. I might be completely wrong.

You can easily add a tracepoint to this realloc() case and see how many shrinking-realloc() calls you actually get.

none.

I will blindly say something here, but anyway: Isn't it that the old buffer size could be greater than the size previously requested (if the memory allocation isn't always perfect fit), then could serve future expansion requests, e.g. realloc? Or am I mistaken?

 

 

Redis uses realloc()
rather heavily but as this is a micro-optimization I only saw minor
gains that could be just noise. ;-)

- Pekka

--

Nadav Har'El

n...@cloudius-systems.com

--
You received this message because you are subscribed to the Google Groups "OSv Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to osv-dev+u...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

--
You received this message because you are subscribed to the Google Groups "OSv Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to osv-dev+u...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

--

Raphael S. Carvalho

[Glauber Costa]

On Wed, Aug 13, 2014 at 5:34 PM, Raphael S. Carvalho <raph...@cloudius-systems.com> wrote:

On Wed, Aug 13, 2014 at 10:26 AM, Glauber Costa <glo...@cloudius-systems.com> wrote:

On Wed, Aug 13, 2014 at 5:25 PM, Nadav Har'El <n...@cloudius-systems.com> wrote:

On Wed, Aug 13, 2014 at 4:00 PM, Pekka Enberg <pen...@cloudius-systems.com> wrote:

On Wed, Aug 13, 2014 at 3:26 PM, Nadav Har'El <n...@cloudius-systems.com> wrote:
> If the application can't rely on realloc() shrinking the buffer, why does it
> call it at all? In which application did you find this "micro-optimization"
> helpful?

realloc() is called to *grow* the buffer...

I'm not disputing this patch, just being curious -

realloc() is normally used in code that determines that the buffer needs to grow, e.g.,

     if (neededlen > bufsize) {

          buf = realloc(buf, neededlen);

     }

In such code, realloc() will not be called with a *smaller* size. If they do, they were probably hoping to really free some space.

But this is just my guess. I might be completely wrong.

You can easily add a tracepoint to this realloc() case and see how many shrinking-realloc() calls you actually get.

none.

I will blindly say something here, but anyway: Isn't it that the old buffer size could be greater than the size previously requested (if the memory allocation isn't always perfect fit), then could serve future expansion requests, e.g. realloc? Or am I mistaken?

That is indeed what the patch does, but redis has a very clear pattern here.
It calls realloc a lot when new data comes to the buffer.

It never shrinks.
And the size almost doubles every time. That is why Pekka can't see an improvement here.

[Nadav Har'El]

On Wed, Aug 13, 2014 at 4:38 PM, Glauber Costa <glo...@cloudius-systems.com> wrote:

That is indeed what the patch does, but redis has a very clear pattern here.
It calls realloc a lot when new data comes to the buffer.

It never shrinks.
And the size almost doubles every time. That is why Pekka can't see an improvement here.

I mis-spoke - Pekka's patch isn't just for "shrinking", it's also for slight size increases which still fit the allocated space.

But it's trivial to add a tracepoint (or other counter or whatever) to the new if() he added, and see exactly how many
times this if() succeeded.

Instead of guessing like all of us have been doing.

[Glauber Costa]

We are not guessing. This came from looking at backtrace traces from redis, to see where the mallocs comes from.
A lot of them are from realloc, and they follow this very clear pattern of reallocating to close to double the size every once
in a while.