So is this a patch that Google would be willing to donate to APR?

C. Michael Pilato 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 <suss...@google.com>
>    Date: Mon, 14 Dec 2009 09:46:45 -0600
>    Message-ID: <944683b40912140746t7763f6f8i5af9c549a8b68...@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 d...@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
>
>   

Reply via email to