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/02/06 13:24:36 UTC
[PATCH] Free memory over a certain threshold back to the system
Hi,
The 'high free' patch. I'm not sure we want this.
It may hide pools abuse problems.
Sander
Index: memory/unix/apr_pools.c
===================================================================
RCS file: /home/cvs/apr/memory/unix/apr_pools.c,v
retrieving revision 1.149
diff -u -r1.149 apr_pools.c
--- memory/unix/apr_pools.c 5 Feb 2002 12:09:43 -0000 1.149
+++ memory/unix/apr_pools.c 6 Feb 2002 11:39:52 -0000
@@ -92,13 +92,19 @@
* Magic numbers
*/
-#define MIN_ALLOC 8192
-#define MAX_INDEX 20
+#define MIN_ALLOC 8192
+#define MAX_INDEX 20
+/* New nodes are rounded up to the next BOUNDARY_SIZE (4096) */
#define BOUNDARY_INDEX 12
#define BOUNDARY_SIZE (1 << BOUNDARY_INDEX)
-
+/* Free blocks when the total number of free bytes goes
+ * over DEFAULT_MAX_FREE * BOUNDARY_SIZE (2MB)
+ */
+#define DEFAULT_MAX_FREE 512
+
+
/*
* Macros and defines
*/
@@ -129,6 +135,7 @@
struct allocator_t {
apr_uint32_t max_index;
+ apr_uint32_t max_free;
#if APR_HAS_THREADS
apr_thread_mutex_t *mutex;
#endif
@@ -206,12 +213,13 @@
#if !APR_POOL_DEBUG
static allocator_t global_allocator = {
- 0, /* max_index */
+ 0, /* max_index */
+ DEFAULT_MAX_FREE, /* max_free */
#if APR_HAS_THREADS
- NULL, /* mutex */
-#endif
- NULL, /* owner */
- { NULL } /* free[0] */
+ NULL, /* mutex */
+#endif /* APR_HAS_THREADS */
+ NULL, /* owner */
+ { NULL } /* free[0] */
};
#endif /* !APR_POOL_DEBUG */
@@ -241,6 +249,7 @@
return APR_SUCCESS;
memset(&global_allocator, 0, SIZEOF_ALLOCATOR_T);
+ global_allocator.max_free = DEFAULT_MAX_FREE;
if ((rv = apr_pool_create_ex(&global_pool, NULL, NULL, APR_POOL_FDEFAULT)) != APR_SUCCESS) {
return rv;
@@ -355,6 +364,8 @@
allocator->max_index = max_index;
}
+ allocator->max_free += node->index;
+
#if APR_HAS_THREADS
if (allocator->mutex)
apr_thread_mutex_unlock(allocator->mutex);
@@ -391,6 +402,8 @@
if (node) {
*ref = node->next;
+ allocator->max_free += node->index;
+
#if APR_HAS_THREADS
if (allocator->mutex)
apr_thread_mutex_unlock(allocator->mutex);
@@ -424,8 +437,8 @@
static APR_INLINE void node_free(allocator_t *allocator, node_t *node)
{
- node_t *next;
- apr_uint32_t index, max_index;
+ node_t *next, *free_nodes = NULL;
+ apr_uint32_t index, max_index, max_free;
#if APR_HAS_THREADS
if (allocator->mutex)
@@ -433,6 +446,7 @@
#endif
max_index = allocator->max_index;
+ max_free = allocator->max_free;
/* Walk the list of submitted nodes and free them one by one,
* shoving them in the right 'size' buckets as we go.
@@ -441,31 +455,47 @@
next = node->next;
index = node->index;
- if (index < MAX_INDEX) {
- /* Add the node to the appropiate 'size' bucket. Adjust
- * the max_index when appropiate.
- */
- if ((node->next = allocator->free[index]) == NULL && index > max_index) {
- max_index = index;
- }
- allocator->free[index] = node;
+ if (max_free < index) {
+ node->next = free_nodes;
+ free_nodes = node;
}
else {
- /* This node is too large to keep in a specific size bucket,
- * just add it to the sink (at index 0).
- */
- node->next = allocator->free[0];
- allocator->free[0] = node;
+ max_free -= index;
+
+ if (index < MAX_INDEX) {
+ /* Add the node to the appropiate 'size' bucket. Adjust
+ * the max_index when appropiate.
+ */
+ if ((node->next = allocator->free[index]) == NULL
+ && index > max_index) {
+ max_index = index;
+ }
+ allocator->free[index] = node;
+ }
+ else {
+ /* This node is too large to keep in a specific size bucket,
+ * just add it to the sink (at index 0).
+ */
+ node->next = allocator->free[0];
+ allocator->free[0] = node;
+ }
}
}
while ((node = next) != NULL);
allocator->max_index = max_index;
+ allocator->max_free = max_free;
#if APR_HAS_THREADS
if (allocator->mutex)
apr_thread_mutex_unlock(allocator->mutex);
#endif
+
+ while (free_nodes != NULL) {
+ node = free_nodes;
+ free_nodes = node->next;
+ free(node);
+ }
}
APR_DECLARE(void *) apr_palloc(apr_pool_t *pool, apr_size_t size)
@@ -682,6 +712,7 @@
memset(new_allocator, 0, SIZEOF_ALLOCATOR_T);
new_allocator->owner = pool;
+ new_allocator->max_free = DEFAULT_MAX_FREE;
pool->allocator = new_allocator;
pool->active = pool->self = node;
Re: [PATCH] Free memory over a certain threshold back to the system
Posted by Ron Park <ro...@cnet.com>.
Ryan Bloom wrote:
>
> > -----Original Message-----
> > From: Justin Erenkrantz [mailto:jerenkrantz@ebuilt.com]
> > Sent: Wednesday, February 06, 2002 8:59 AM
> > To: Sander Striker
> > Cc: dev@apr.apache.org
> > Subject: Re: [PATCH] Free memory over a certain threshold back to the
> > system
> >
> > On Wed, Feb 06, 2002 at 01:24:36PM +0100, Sander Striker wrote:
> > > Hi,
> > >
> > > The 'high free' patch. I'm not sure we want this.
> > > It may hide pools abuse problems.
> >
> > Yeah, I agree we don't really want this. If we run out of memory
> > with the normal pools, it means that the lifetimes are most likely
> > incorrect.
> >
> > But, perhaps this could be a #define with a debug option? -- justin
>
> We definitely don't want this IMHO. If the pools are filling up, then
> either pools are the incorrect model for your app, or they aren't being
> cleared often enough. I would much rather not have this as an option at
> all, because that just encourages people to use it. :-)
I was under the impression this patch simply kept the free lists from growing
and staying excessively large, not 'filling up pools'. There are some apps
which can and should use pools but which, during the course of their lifetime,
have periods of heavy usage and thus create an abnormally large number of
pools;
since these pools are never truly freed and hang around in the free_list
forever,
the process clings to this memory even if it's never needed again.
For instance, a process which accepts requests into an internal queue and
processes
them in seperate threads at a later time. If the arrival rate (temporarily)
exceeds
the processing rate, the queues (and the associated pools) will grow
excessively
large. Without this patch, even when all the pools are destroyed, the process
will
still hold onto that memory.
As someone who's got a process that is suffering from this behavior, I was
looking forward to this patch... perhaps it could be added as a compile time
option?
Is the pool model fundamentally flawed for this type of program? I'd hate to
have
to strip all of APR out of my code; other than this, it's worked perfectly for
me.
Thanks,
Ron
--
Ronald K. Park, Jr.
ronp@cnet.com CNET Networks / CNET.com / ZDNet / mySimon
908.541.3738 The source for computers and technology.
Re: [PATCH] Free memory over a certain threshold back to the system
Posted by Ian Holsman <ia...@apache.org>.
Ryan Bloom wrote:
>>-----Original Message-----
>>From: Justin Erenkrantz [mailto:jerenkrantz@ebuilt.com]
>>Sent: Wednesday, February 06, 2002 8:59 AM
>>To: Sander Striker
>>Cc: dev@apr.apache.org
>>Subject: Re: [PATCH] Free memory over a certain threshold back to the
>>system
>>
>>On Wed, Feb 06, 2002 at 01:24:36PM +0100, Sander Striker wrote:
>>
>>>Hi,
>>>
>>>The 'high free' patch. I'm not sure we want this.
>>>It may hide pools abuse problems.
>>>
>>Yeah, I agree we don't really want this. If we run out of memory
>>with the normal pools, it means that the lifetimes are most likely
>>incorrect.
>>
>>But, perhaps this could be a #define with a debug option? -- justin
>>
>
> We definitely don't want this IMHO. If the pools are filling up, then
> either pools are the incorrect model for your app, or they aren't being
> cleared often enough. I would much rather not have this as an option at
> all, because that just encourages people to use it. :-)
>
> Ryan
>
that's not the case at all.
it will stop people using pool (and probably the APR) for application
which have a large variance in their memory requirements over time.
by not having this patch in your forcing the application to always grow
over time, or at the least always stay at it's high water mark.
Take for instance a message queue. normally the message queue is running
at 2-3 messages in the queue..something borks up on the GET side causing
the messages to pile up to 2k. the GET side then is fixed and empties
the queue..
if we had the himem-free patch APR would be a great in here.. without it
the size of the queue will allways be 2k instead of shrinking to 2-3.
so..
i'm +1 on this patch
>
>
Re: [PATCH] Free memory over a certain threshold back to the system
Posted by Ron Park <ro...@cnet.com>.
I know I would like to see the allocator more exposed. For instance, for
some pools, I'd like to be able to initialize it with much less than the
8K default. Sure I'm running on an 8G machine, but why waste space I know
I won't need. :)
Thanks,
Ron
Sander Striker wrote:
>
> > From: Brian Pane [mailto:brian.pane@cnet.com]
> > Sent: 07 February 2002 04:21
>
> >> It is the memory on the freelists that grows without bounds. The patch
> >> free()s memory that is added to a freelist when the freelist already
> >> holds a certain amount of free memory. This is very usefull when you
> >> have peak memory usage as some people pointed out.
> >>
> >> The pools model is something you are stuck with it if you want to use APR.
> >> And it is not always pools abuse which gives you big chunks of mem on
> >> the freelist. Although I must admit that I would recommend some
> >> serious review of the code which shows this behaviour.
> >>
> >> Anyhow, I'd like to add a configure option which you can feed the
> >> amount of memory you want as a maximum on a freelist, defaulting
> >> to infinite.
> >>
> >> Thoughts?
> >>
> >> Sander
> >
> > I like the idea of a configurable maximum. It probably should
> > be run-time configurable rather than build-time configurable,
> > though. It would be even better if the max free list size could
> > be specified on a per-allocator basis, so that the application
> > developer could tailor the properties of each free list to the
> > needs of the app.
> >
> > --Brian
>
> That certainly can be coded up. We only need to think about an API.
> Currently I am playing with the idea to factor the allocator out,
> and expose it. This would give us the opurtunity to add a set_max_free
> function in there. Placing it in the apr_pools namespace is something
> I'd rather not do.
>
> Sander
--
Ronald K. Park, Jr.
ronp@cnet.com CNET Networks / CNET.com / ZDNet / mySimon
908.541.3738 The source for computers and technology.
RE: [PATCH] Free memory over a certain threshold back to the system
Posted by Sander Striker <st...@apache.org>.
> From: Brian Pane [mailto:brian.pane@cnet.com]
> Sent: 07 February 2002 04:21
>> It is the memory on the freelists that grows without bounds. The patch
>> free()s memory that is added to a freelist when the freelist already
>> holds a certain amount of free memory. This is very usefull when you
>> have peak memory usage as some people pointed out.
>>
>> The pools model is something you are stuck with it if you want to use APR.
>> And it is not always pools abuse which gives you big chunks of mem on
>> the freelist. Although I must admit that I would recommend some
>> serious review of the code which shows this behaviour.
>>
>> Anyhow, I'd like to add a configure option which you can feed the
>> amount of memory you want as a maximum on a freelist, defaulting
>> to infinite.
>>
>> Thoughts?
>>
>> Sander
>
> I like the idea of a configurable maximum. It probably should
> be run-time configurable rather than build-time configurable,
> though. It would be even better if the max free list size could
> be specified on a per-allocator basis, so that the application
> developer could tailor the properties of each free list to the
> needs of the app.
>
> --Brian
That certainly can be coded up. We only need to think about an API.
Currently I am playing with the idea to factor the allocator out,
and expose it. This would give us the opurtunity to add a set_max_free
function in there. Placing it in the apr_pools namespace is something
I'd rather not do.
Sander
Re: [PATCH] Free memory over a certain threshold back to the system
Posted by Brian Pane <br...@cnet.com>.
Sander Striker wrote:
>>From: Ryan Bloom [mailto:rbb@covalent.net]
>>Sent: 06 February 2002 18:02
>>
>>>-----Original Message-----
>>>From: Justin Erenkrantz [mailto:jerenkrantz@ebuilt.com]
>>>Sent: Wednesday, February 06, 2002 8:59 AM
>>>To: Sander Striker
>>>Cc: dev@apr.apache.org
>>>Subject: Re: [PATCH] Free memory over a certain threshold back to the
>>>system
>>>
>>>On Wed, Feb 06, 2002 at 01:24:36PM +0100, Sander Striker wrote:
>>>
>>>>Hi,
>>>>
>>>>The 'high free' patch. I'm not sure we want this.
>>>>It may hide pools abuse problems.
>>>>
>>>Yeah, I agree we don't really want this. If we run out of memory
>>>with the normal pools, it means that the lifetimes are most likely
>>>incorrect.
>>>
>>>But, perhaps this could be a #define with a debug option? -- justin
>>>
>>We definitely don't want this IMHO. If the pools are filling up, then
>>either pools are the incorrect model for your app, or they aren't being
>>cleared often enough. I would much rather not have this as an option at
>>all, because that just encourages people to use it. :-)
>>
>>Ryan
>>
>
>It is the memory on the freelists that grows without bounds. The patch
>free()s memory that is added to a freelist when the freelist already
>holds a certain amount of free memory. This is very usefull when you
>have peak memory usage as some people pointed out.
>
>The pools model is something you are stuck with it if you want to use APR.
>And it is not always pools abuse which gives you big chunks of mem on
>the freelist. Although I must admit that I would recommend some
>serious review of the code which shows this behaviour.
>
>Anyhow, I'd like to add a configure option which you can feed the
>amount of memory you want as a maximum on a freelist, defaulting
>to infinite.
>
>Thoughts?
>
I like the idea of a configurable maximum. It probably should
be run-time configurable rather than build-time configurable,
though. It would be even better if the max free list size could
be specified on a per-allocator basis, so that the application
developer could tailor the properties of each free list to the
needs of the app.
--Brian
RE: [PATCH] Free memory over a certain threshold back to the system
Posted by Sander Striker <st...@apache.org>.
> From: Ryan Bloom [mailto:rbb@covalent.net]
> Sent: 06 February 2002 18:02
>
> > -----Original Message-----
> > From: Justin Erenkrantz [mailto:jerenkrantz@ebuilt.com]
> > Sent: Wednesday, February 06, 2002 8:59 AM
> > To: Sander Striker
> > Cc: dev@apr.apache.org
> > Subject: Re: [PATCH] Free memory over a certain threshold back to the
> > system
> >
> > On Wed, Feb 06, 2002 at 01:24:36PM +0100, Sander Striker wrote:
> > > Hi,
> > >
> > > The 'high free' patch. I'm not sure we want this.
> > > It may hide pools abuse problems.
> >
> > Yeah, I agree we don't really want this. If we run out of memory
> > with the normal pools, it means that the lifetimes are most likely
> > incorrect.
> >
> > But, perhaps this could be a #define with a debug option? -- justin
>
> We definitely don't want this IMHO. If the pools are filling up, then
> either pools are the incorrect model for your app, or they aren't being
> cleared often enough. I would much rather not have this as an option at
> all, because that just encourages people to use it. :-)
>
> Ryan
It is the memory on the freelists that grows without bounds. The patch
free()s memory that is added to a freelist when the freelist already
holds a certain amount of free memory. This is very usefull when you
have peak memory usage as some people pointed out.
The pools model is something you are stuck with it if you want to use APR.
And it is not always pools abuse which gives you big chunks of mem on
the freelist. Although I must admit that I would recommend some
serious review of the code which shows this behaviour.
Anyhow, I'd like to add a configure option which you can feed the
amount of memory you want as a maximum on a freelist, defaulting
to infinite.
Thoughts?
Sander
RE: [PATCH] Free memory over a certain threshold back to the system
Posted by Ryan Bloom <rb...@covalent.net>.
> -----Original Message-----
> From: Justin Erenkrantz [mailto:jerenkrantz@ebuilt.com]
> Sent: Wednesday, February 06, 2002 8:59 AM
> To: Sander Striker
> Cc: dev@apr.apache.org
> Subject: Re: [PATCH] Free memory over a certain threshold back to the
> system
>
> On Wed, Feb 06, 2002 at 01:24:36PM +0100, Sander Striker wrote:
> > Hi,
> >
> > The 'high free' patch. I'm not sure we want this.
> > It may hide pools abuse problems.
>
> Yeah, I agree we don't really want this. If we run out of memory
> with the normal pools, it means that the lifetimes are most likely
> incorrect.
>
> But, perhaps this could be a #define with a debug option? -- justin
We definitely don't want this IMHO. If the pools are filling up, then
either pools are the incorrect model for your app, or they aren't being
cleared often enough. I would much rather not have this as an option at
all, because that just encourages people to use it. :-)
Ryan
Re: [PATCH] Free memory over a certain threshold back to the system
Posted by Justin Erenkrantz <je...@ebuilt.com>.
On Wed, Feb 06, 2002 at 01:24:36PM +0100, Sander Striker wrote:
> Hi,
>
> The 'high free' patch. I'm not sure we want this.
> It may hide pools abuse problems.
Yeah, I agree we don't really want this. If we run out of memory
with the normal pools, it means that the lifetimes are most likely
incorrect.
But, perhaps this could be a #define with a debug option? -- justin