Mark Phippard tipped me off to this.
Glad to hear that APR folk took the report seriously enough to investigate
the matter. Did anyone there happen to look at the fact that MaxMemFree
seems to not work at all, or that pools appear not to be automagically
defragmentable?
Justin Erenkrantz wrote:
> 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;
>> }
>>
>>
--
C. Michael Pilato <cmpilato_at_collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Received on 2009-12-14 19:34:43 CET