[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: apr pools & memory leaks (revisited, if briefly)

From: Justin Erenkrantz <justin_at_erenkrantz.com>
Date: Mon, 14 Dec 2009 10:26:34 -0800

Just to note - in Amsterdam earlier this year, the APR/httpd folks
ripped out pools and replaced it with a straight malloc/free
implementation and the performance across the board was horrific even
with the "best" malloc replacement libraries. So, that code was
reverted...see:

http://mail-archives.apache.org/mod_mbox/apr-dev/200903.mbox/%3c49CB671F.9040801@apache.org%3e

HTH. -- justin

On Mon, Dec 14, 2009 at 8:23 AM, C. Michael Pilato <cmpilato_at_collab.net> wrote:
> Some time ago, Ben Collins-Sussman shared with us some of the Google teams'
> findings about APR's memory subsystem[1].  In his mail, he says (among other
> things) the following:
>
>   Over at Google, we simply hacked APR to *never* hold on to blocks
>   for recycling. Essentially, this makes apr_pool_destroy() always
>   free() the block, and makes apr_pool_create() always call malloc()
>   malloc.  Poof, all the memory leak went away instantly.
>
>   What was more troubling is that the use of the MaxMemFree directive --
>   which is supposed to limit the total size of the 'free memory'
>   recycling list -- didn't seem to work for us. What we need to do is
>   go back and debug this more carefully, and see if it's a bug in APR,
>   apache, or just in our testing methodology.
>
> I came across this mail thread again recently as part of my own
> investigation into Apache's seeming ignorance of the MaxMemFree directive.
> So I poked Ben to see if he would/could share the patch to APR that they
> wound up with.  He responded with the following (and the attached patch,
> which I had to reformat to get to a apply cleanly):
>
>   From: Ben Collins-Sussman <sussman_at_google.com>
>   Date: Mon, 14 Dec 2009 09:46:45 -0600
>   Message-ID: <944683b40912140746t7763f6f8i5af9c549a8b68e90_at_mail.gmail.com>
>   Subject: Re: Got Apache insta-free patch?
>
>   Crazy simple patch.  Basically, if memory_recycling is disabled,
>   then we prevent any freed-up pool from *ever* be pushed on to the
>   'free list' of pools to recycle.  The result is that all calls to
>   apr_pool_destroy() causes the actual unix free() call to happen.
>   (And likewise, because the 'free list' of recyclable pools is
>   always empty, every call to apr_pool_create() causes malloc() to
>   happen.)
>
>   Why this works: apr's recycling logic is ridiculously primitive.
>   If there are 50 pools on the 'free list' which are each 2K, and a
>   request comes in for a new pool that needs 20k, *none of those
>   recyclable pools* get recombined at all.  They just sit there
>   forever, while apr malloc's a new 20k pool.  The result is that for
>   really long-running processes, apr just gradually holds on to more
>   and more memory over time, ever more fragmented.  This isn't a
>   problem with normal worker/slave mode, since slave httpd processes
>   die and get respawned after N requests.  But if you run in
>   multithreaded mode (which is the default on Windows, I think), then
>   it's just all one big process that runs forever, leaking forever.
>
>   Why we feel this patch is fine for production: the whole apr pool
>   system was written in the mid-90's, back when malloc() and free()
>   were seriously expensive and made your app noticeably slower.  The
>   concept of recycling was seen as a way to prevent these expensive
>   operations.  These days there is *no* speed hit at all that we can
>   see.  Computers and OSes are just so much faster.
>
>   Feel free to post this to the whole dev_at_subversion list if you
>   wish.
>
> I'm "feeling free" to share this with y'all.  I'm not vouching for its
> accuracy -- I haven't had a chance to test it myself yet.  But it's
> Christmas time, so have a cup of good cheer and some more free() calls on
> Google.
>
> [1] http://svn.haxx.se/dev/archive-2008-10/0070.shtml
>
> --
> C. Michael Pilato <cmpilato_at_collab.net>
> CollabNet   <>   www.collab.net   <>   Distributed Development On Demand
>
> diff -ru srclib/apr/include/apr_pools.h srclib/apr/include/apr_pools.h
> --- srclib/apr/include/apr_pools.h      2008-07-14 11:07:00.000000000 -0400
> +++ srclib/apr/include/apr_pools.h      2009-12-14 10:52:16.000000000 -0500
> @@ -765,6 +765,14 @@
>
>  #endif /* APR_POOL_DEBUG or DOXYGEN */
>
> +
> +/**
> + * Enable or disable recycling of memory in pools.
> + * @param enable Non-zero to enable recycling, 0 to disable it
> + */
> +APR_DECLARE(void) apr_set_memory_recycling(int enable);
> +
> +
>  /** @} */
>
>  #ifdef __cplusplus
> diff -ru srclib/apr/memory/unix/apr_pools.c srclib/apr/memory/unix/apr_pools.c
> --- srclib/apr/memory/unix/apr_pools.c  2008-07-19 07:54:52.000000000 -0400
> +++ srclib/apr/memory/unix/apr_pools.c  2009-12-14 10:53:06.000000000 -0500
> @@ -40,6 +40,18 @@
>  #endif
>
>
> +
> +/*
> + * Runtime disabling of recycling
> + */
> +
> +static int memory_recycling = 1;
> +
> +void apr_set_memory_recycling(int enable)
> +{
> +  memory_recycling = enable;
> +}
> +
>  /*
>  * Magic numbers
>  */
> @@ -354,8 +366,9 @@
>         next = node->next;
>         index = node->index;
>
> -        if (max_free_index != APR_ALLOCATOR_MAX_FREE_UNLIMITED
> -            && index > current_free_index) {
> +        if (!memory_recycling ||
> +            (max_free_index != APR_ALLOCATOR_MAX_FREE_UNLIMITED
> +             && index > current_free_index)) {
>             node->next = freelist;
>             freelist = node;
>         }
>
>
Received on 2009-12-14 19:27:12 CET

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.