You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Sander Striker <st...@apache.org> on 2002/03/19 20:45:38 UTC

[PATCH] Maximum free memory in an allocator OR: hifree, revisited

Hi,

Keep getting the question if the hifree patch is
going in.  So, decided to revisit that patch and
implement it now we have the allocators.

For those who didn't follow that thread:
This patch allows the programmer to set a maximum
amount of free bytes allowed on the allocators
freelist.  Anything over it will be free()d back
to the system.

Thoughts?

Sander

Index: include/apr_allocator.h
===================================================================
RCS file: /home/cvs/apr/include/apr_allocator.h,v
retrieving revision 1.2
diff -u -r1.2 apr_allocator.h
--- include/apr_allocator.h     18 Mar 2002 16:24:54 -0000      1.2
+++ include/apr_allocator.h     19 Mar 2002 19:32:41 -0000
@@ -143,6 +143,18 @@
  */
 APR_DECLARE(apr_pool_t *) apr_allocator_get_owner(apr_allocator_t *allocator);

+/**
+ * Set the maximum amount of free memory to be held by
+ * the allocator.
+ * @param allocator The allocator to set the max amount for
+ * @param size The maximum amount the allocator is allowed
+ *        to keep.
+ * @remark Passing 0 for size will set the max amount to
+ *         unlimited.
+ */
+APR_DECLARE(void) apr_allocator_set_max_free(apr_allocator_t *allocator,
+                                             apr_size_t size);
+
 #if APR_HAS_THREADS
 /**
  * Set a mutex for the allocator to use
Index: memory/unix/apr_pools.c
===================================================================
RCS file: /home/cvs/apr/memory/unix/apr_pools.c,v
retrieving revision 1.163
diff -u -r1.163 apr_pools.c
--- memory/unix/apr_pools.c     19 Mar 2002 15:30:07 -0000      1.163
+++ memory/unix/apr_pools.c     19 Mar 2002 19:32:49 -0000
@@ -106,6 +106,8 @@

 struct apr_allocator_t {
     apr_uint32_t        max_index;
+    apr_uint32_t        max_free_index;
+    apr_uint32_t        current_free_index;
 #if APR_HAS_THREADS
     apr_thread_mutex_t *mutex;
 #endif /* APR_HAS_THREADS */
@@ -179,6 +181,32 @@
     return allocator->owner;
 }

+APR_DECLARE(void) apr_allocator_set_max_free(apr_allocator_t *allocator,
+                                             apr_size_t size)
+{
+    apr_uint32_t max_free_index;
+
+#if APR_HAS_THREADS
+    apr_thread_mutex_t *mutex;
+
+    mutex = apr_allocator_get_mutex(allocator);
+    if (mutex != NULL)
+        apr_thread_mutex_lock(mutex);
+#endif /* APR_HAS_THREADS */
+
+    max_free_index = APR_ALIGN(size, BOUNDARY_SIZE) >> BOUNDARY_INDEX;
+    allocator->current_free_index += max_free_index;
+    allocator->current_free_index -= allocator->max_free_index;
+    allocator->max_free_index = max_free_index;
+    if (allocator->current_free_index > max_free_index)
+        allocator->current_free_index = max_free_index;
+
+#if APR_HAS_THREADS
+    if (mutex != NULL)
+        apr_thread_mutex_unlock(mutex);
+#endif
+}
+
 APR_INLINE
 APR_DECLARE(apr_memnode_t *) apr_allocator_alloc(apr_allocator_t *allocator,
                                                  apr_size_t size)
@@ -241,6 +269,10 @@
                 allocator->max_index = max_index;
             }

+            allocator->current_free_index += node->index;
+            if (allocator->current_free_index > allocator->max_free_index)
+                allocator->current_free_index = allocator->max_free_index;
+
 #if APR_HAS_THREADS
             if (allocator->mutex)
                 apr_thread_mutex_unlock(allocator->mutex);
@@ -277,6 +309,10 @@
         if (node) {
             *ref = node->next;

+            allocator->current_free_index += node->index;
+            if (allocator->current_free_index > allocator->max_free_index)
+                allocator->current_free_index = allocator->max_free_index;
+
 #if APR_HAS_THREADS
             if (allocator->mutex)
                 apr_thread_mutex_unlock(allocator->mutex);
@@ -312,8 +348,9 @@
 APR_DECLARE(void) apr_allocator_free(apr_allocator_t *allocator,
                                      apr_memnode_t *node)
 {
-    apr_memnode_t *next;
+    apr_memnode_t *next, *freelist = NULL;
     apr_uint32_t index, max_index;
+    apr_uint32_t max_free_index, current_free_index;

 #if APR_HAS_THREADS
     if (allocator->mutex)
@@ -321,6 +358,8 @@
 #endif /* APR_HAS_THREADS */

     max_index = allocator->max_index;
+    max_free_index = allocator->max_free_index;
+    current_free_index = allocator->current_free_index;

     /* Walk the list of submitted nodes and free them one by one,
      * shoving them in the right 'size' buckets as we go.
@@ -329,7 +368,12 @@
         next = node->next;
         index = node->index;

-        if (index < MAX_INDEX) {
+        if (max_free_index != 0 && index > current_free_index) {
+            node->next = freelist;
+            freelist = node;
+            current_free_index -= index;
+        }
+        else if (index < MAX_INDEX) {
             /* Add the node to the appropiate 'size' bucket.  Adjust
              * the max_index when appropiate.
              */
@@ -349,11 +393,18 @@
     } while ((node = next) != NULL);

     allocator->max_index = max_index;
+    allocator->current_free_index = current_free_index;

 #if APR_HAS_THREADS
     if (allocator->mutex)
         apr_thread_mutex_unlock(allocator->mutex);
 #endif /* APR_HAS_THREADS */
+
+    while (freelist != NULL) {
+        node = freelist;
+        freelist = node->next;
+        free(node);
+    }
 }


Re: [PATCH] Maximum free memory in an allocator OR: hifree, revisited

Posted by Justin Erenkrantz <je...@apache.org>.
On Wed, Apr 10, 2002 at 12:17:58PM +0200, Sander Striker wrote:
> Hi,
> 
> Just raising this again.  This time I'll ask differently:
> Is anyone opposed to adding this? [feedback please]

Can you please repost the patch?  -- justin

Re: [PATCH] Maximum free memory in an allocator OR: hifree, revisited

Posted by Aaron Bannert <aa...@clove.org>.
On Wed, Apr 10, 2002 at 02:47:26PM +0200, Branko ?ibej wrote:
> >I know Ian wants this badly.  I can go either way.
> >I would really like to see some perf comparisons with
> >and without it to see if it doesn't do too much damage
> >to performance (or if it does at all in a measurable fashion).
> >
> >Sander
> >
> 
> +1
> 
> I think a small performance penalty is better than keeping unused 
> allocated VM around.

Actually pools trade processing time for memory space. Pools allow us to
malloc larger chunks of memory (to reduce freelist management overhead)
and then to delay the costs of freeing until we want to "checkpoint"
the memory.

However, the hi-free idea may be useful to counteract extreme cases where
a single thread may have consumed an abnormally high amount of memory. I
can see us using the hi-free setting somewhere significantly higher than
a "normal" requests's memory usage, just to catch those stray requests
that take 10x normal.

Like Sander said, we need to test this for performance implications.

-aaron

Re: [PATCH] Maximum free memory in an allocator OR: hifree, revisited

Posted by Branko Čibej <br...@xbc.nu>.
Sander Striker wrote:

>Hi,
>
>Just raising this again.  This time I'll ask differently:
>Is anyone opposed to adding this? [feedback please]
>
>I know Ian wants this badly.  I can go either way.
>I would really like to see some perf comparisons with
>and without it to see if it doesn't do too much damage
>to performance (or if it does at all in a measurable fashion).
>
>Sander
>

+1

I think a small performance penalty is better than keeping unused 
allocated VM around.

>
>
>
>>From: Sander Striker [mailto:striker@apache.org]
>>Sent: 21 March 2002 09:13
>>
>
>>>From: Bill Stoddard [mailto:bill@wstoddard.com]
>>>Sent: 19 March 2002 21:36
>>>
>>>This could be quite useful for mod_*_cache.  What triggers the free?
>>>
>>The free is triggered on either apr_pool_clear or apr_pool_destroy,
>>when the blocks in the pool are returned to the allocator (using
>>apr_allocator_free()).  When adding a block to the freelist causes
>>the freelist to be larger than then threshold (set with 
>>apr_allocator_set_max_free()), the block will be free()d instead
>>of added.
>>
>>The default is to have max_free be 0, which translates to unlimited,
>>which is the current behaviour.
>>
>>Ofcourse this patch will come at a small performance penalty, but
>>I'm not sure if it is measurable.
>> 
>>
>>>Bill
>>>
>>Sander
>>
>>>----- Original Message ----- 
>>>From: "Sander Striker" <st...@apache.org>
>>>To: <de...@apr.apache.org>
>>>Sent: Tuesday, March 19, 2002 2:45 PM
>>>Subject: [PATCH] Maximum free memory in an allocator OR: hifree, revisited
>>>
>>>
>>>>Hi,
>>>>
>>>>Keep getting the question if the hifree patch is
>>>>going in.  So, decided to revisit that patch and
>>>>implement it now we have the allocators.
>>>>
>>>>For those who didn't follow that thread:
>>>>This patch allows the programmer to set a maximum
>>>>amount of free bytes allowed on the allocators
>>>>freelist.  Anything over it will be free()d back
>>>>to the system.
>>>>
>>>>Thoughts?
>>>>
>>>>Sander
>>>>
>


-- 
Brane Čibej   <br...@xbc.nu>   http://www.xbc.nu/brane/




RE: [PATCH] Maximum free memory in an allocator OR: hifree, revisited

Posted by Sander Striker <st...@apache.org>.
Hi,

Just raising this again.  This time I'll ask differently:
Is anyone opposed to adding this? [feedback please]

I know Ian wants this badly.  I can go either way.
I would really like to see some perf comparisons with
and without it to see if it doesn't do too much damage
to performance (or if it does at all in a measurable fashion).

Sander


> From: Sander Striker [mailto:striker@apache.org]
> Sent: 21 March 2002 09:13

> > From: Bill Stoddard [mailto:bill@wstoddard.com]
> > Sent: 19 March 2002 21:36
> 
> > This could be quite useful for mod_*_cache.  What triggers the free?
> 
> The free is triggered on either apr_pool_clear or apr_pool_destroy,
> when the blocks in the pool are returned to the allocator (using
> apr_allocator_free()).  When adding a block to the freelist causes
> the freelist to be larger than then threshold (set with 
> apr_allocator_set_max_free()), the block will be free()d instead
> of added.
> 
> The default is to have max_free be 0, which translates to unlimited,
> which is the current behaviour.
> 
> Ofcourse this patch will come at a small performance penalty, but
> I'm not sure if it is measurable.
>  
> > Bill
> 
> Sander
> 
> > ----- Original Message ----- 
> > From: "Sander Striker" <st...@apache.org>
> > To: <de...@apr.apache.org>
> > Sent: Tuesday, March 19, 2002 2:45 PM
> > Subject: [PATCH] Maximum free memory in an allocator OR: hifree, revisited
> > 
> > 
> > > Hi,
> > > 
> > > Keep getting the question if the hifree patch is
> > > going in.  So, decided to revisit that patch and
> > > implement it now we have the allocators.
> > > 
> > > For those who didn't follow that thread:
> > > This patch allows the programmer to set a maximum
> > > amount of free bytes allowed on the allocators
> > > freelist.  Anything over it will be free()d back
> > > to the system.
> > > 
> > > Thoughts?
> > > 
> > > Sander
> 

RE: [PATCH] Maximum free memory in an allocator OR: hifree, revisited

Posted by Sander Striker <st...@apache.org>.
> From: Bill Stoddard [mailto:bill@wstoddard.com]
> Sent: 19 March 2002 21:36

> This could be quite useful for mod_*_cache.  What triggers the free?

The free is triggered on either apr_pool_clear or apr_pool_destroy,
when the blocks in the pool are returned to the allocator (using
apr_allocator_free()).  When adding a block to the freelist causes
the freelist to be larger than then threshold (set with 
apr_allocator_set_max_free()), the block will be free()d instead
of added.

The default is to have max_free be 0, which translates to unlimited,
which is the current behaviour.

Ofcourse this patch will come at a small performance penalty, but
I'm not sure if it is measurable.
 
> Bill

Sander

> ----- Original Message ----- 
> From: "Sander Striker" <st...@apache.org>
> To: <de...@apr.apache.org>
> Sent: Tuesday, March 19, 2002 2:45 PM
> Subject: [PATCH] Maximum free memory in an allocator OR: hifree, revisited
> 
> 
> > Hi,
> > 
> > Keep getting the question if the hifree patch is
> > going in.  So, decided to revisit that patch and
> > implement it now we have the allocators.
> > 
> > For those who didn't follow that thread:
> > This patch allows the programmer to set a maximum
> > amount of free bytes allowed on the allocators
> > freelist.  Anything over it will be free()d back
> > to the system.
> > 
> > Thoughts?
> > 
> > Sander


Re: [PATCH] Maximum free memory in an allocator OR: hifree, revisited

Posted by Bill Stoddard <bi...@wstoddard.com>.
This could be quite useful for mod_*_cache.  What triggers the free?

Bill
----- Original Message ----- 
From: "Sander Striker" <st...@apache.org>
To: <de...@apr.apache.org>
Sent: Tuesday, March 19, 2002 2:45 PM
Subject: [PATCH] Maximum free memory in an allocator OR: hifree, revisited


> Hi,
> 
> Keep getting the question if the hifree patch is
> going in.  So, decided to revisit that patch and
> implement it now we have the allocators.
> 
> For those who didn't follow that thread:
> This patch allows the programmer to set a maximum
> amount of free bytes allowed on the allocators
> freelist.  Anything over it will be free()d back
> to the system.
> 
> Thoughts?
> 
> Sander
> 
> Index: include/apr_allocator.h
> ===================================================================
> RCS file: /home/cvs/apr/include/apr_allocator.h,v
> retrieving revision 1.2
> diff -u -r1.2 apr_allocator.h
> --- include/apr_allocator.h     18 Mar 2002 16:24:54 -0000      1.2
> +++ include/apr_allocator.h     19 Mar 2002 19:32:41 -0000
> @@ -143,6 +143,18 @@
>   */
>  APR_DECLARE(apr_pool_t *) apr_allocator_get_owner(apr_allocator_t *allocator);
> 
> +/**
> + * Set the maximum amount of free memory to be held by
> + * the allocator.
> + * @param allocator The allocator to set the max amount for
> + * @param size The maximum amount the allocator is allowed
> + *        to keep.
> + * @remark Passing 0 for size will set the max amount to
> + *         unlimited.
> + */
> +APR_DECLARE(void) apr_allocator_set_max_free(apr_allocator_t *allocator,
> +                                             apr_size_t size);
> +
>  #if APR_HAS_THREADS
>  /**
>   * Set a mutex for the allocator to use
> Index: memory/unix/apr_pools.c
> ===================================================================
> RCS file: /home/cvs/apr/memory/unix/apr_pools.c,v
> retrieving revision 1.163
> diff -u -r1.163 apr_pools.c
> --- memory/unix/apr_pools.c     19 Mar 2002 15:30:07 -0000      1.163
> +++ memory/unix/apr_pools.c     19 Mar 2002 19:32:49 -0000
> @@ -106,6 +106,8 @@
> 
>  struct apr_allocator_t {
>      apr_uint32_t        max_index;
> +    apr_uint32_t        max_free_index;
> +    apr_uint32_t        current_free_index;
>  #if APR_HAS_THREADS
>      apr_thread_mutex_t *mutex;
>  #endif /* APR_HAS_THREADS */
> @@ -179,6 +181,32 @@
>      return allocator->owner;
>  }
> 
> +APR_DECLARE(void) apr_allocator_set_max_free(apr_allocator_t *allocator,
> +                                             apr_size_t size)
> +{
> +    apr_uint32_t max_free_index;
> +
> +#if APR_HAS_THREADS
> +    apr_thread_mutex_t *mutex;
> +
> +    mutex = apr_allocator_get_mutex(allocator);
> +    if (mutex != NULL)
> +        apr_thread_mutex_lock(mutex);
> +#endif /* APR_HAS_THREADS */
> +
> +    max_free_index = APR_ALIGN(size, BOUNDARY_SIZE) >> BOUNDARY_INDEX;
> +    allocator->current_free_index += max_free_index;
> +    allocator->current_free_index -= allocator->max_free_index;
> +    allocator->max_free_index = max_free_index;
> +    if (allocator->current_free_index > max_free_index)
> +        allocator->current_free_index = max_free_index;
> +
> +#if APR_HAS_THREADS
> +    if (mutex != NULL)
> +        apr_thread_mutex_unlock(mutex);
> +#endif
> +}
> +
>  APR_INLINE
>  APR_DECLARE(apr_memnode_t *) apr_allocator_alloc(apr_allocator_t *allocator,
>                                                   apr_size_t size)
> @@ -241,6 +269,10 @@
>                  allocator->max_index = max_index;
>              }
> 
> +            allocator->current_free_index += node->index;
> +            if (allocator->current_free_index > allocator->max_free_index)
> +                allocator->current_free_index = allocator->max_free_index;
> +
>  #if APR_HAS_THREADS
>              if (allocator->mutex)
>                  apr_thread_mutex_unlock(allocator->mutex);
> @@ -277,6 +309,10 @@
>          if (node) {
>              *ref = node->next;
> 
> +            allocator->current_free_index += node->index;
> +            if (allocator->current_free_index > allocator->max_free_index)
> +                allocator->current_free_index = allocator->max_free_index;
> +
>  #if APR_HAS_THREADS
>              if (allocator->mutex)
>                  apr_thread_mutex_unlock(allocator->mutex);
> @@ -312,8 +348,9 @@
>  APR_DECLARE(void) apr_allocator_free(apr_allocator_t *allocator,
>                                       apr_memnode_t *node)
>  {
> -    apr_memnode_t *next;
> +    apr_memnode_t *next, *freelist = NULL;
>      apr_uint32_t index, max_index;
> +    apr_uint32_t max_free_index, current_free_index;
> 
>  #if APR_HAS_THREADS
>      if (allocator->mutex)
> @@ -321,6 +358,8 @@
>  #endif /* APR_HAS_THREADS */
> 
>      max_index = allocator->max_index;
> +    max_free_index = allocator->max_free_index;
> +    current_free_index = allocator->current_free_index;
> 
>      /* Walk the list of submitted nodes and free them one by one,
>       * shoving them in the right 'size' buckets as we go.
> @@ -329,7 +368,12 @@
>          next = node->next;
>          index = node->index;
> 
> -        if (index < MAX_INDEX) {
> +        if (max_free_index != 0 && index > current_free_index) {
> +            node->next = freelist;
> +            freelist = node;
> +            current_free_index -= index;
> +        }
> +        else if (index < MAX_INDEX) {
>              /* Add the node to the appropiate 'size' bucket.  Adjust
>               * the max_index when appropiate.
>               */
> @@ -349,11 +393,18 @@
>      } while ((node = next) != NULL);
> 
>      allocator->max_index = max_index;
> +    allocator->current_free_index = current_free_index;
> 
>  #if APR_HAS_THREADS
>      if (allocator->mutex)
>          apr_thread_mutex_unlock(allocator->mutex);
>  #endif /* APR_HAS_THREADS */
> +
> +    while (freelist != NULL) {
> +        node = freelist;
> +        freelist = node->next;
> +        free(node);
> +    }
>  }
>