apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ryan Bloom" <...@covalent.net>
Subject RE: [design] work around new apr_poll leakage?
Date Mon, 22 Jul 2002 03:19:23 GMT

Okay, so this is essentially exactly what we had before I re-wrote
apr_poll() a few weeks ago.  There are some differences, such as keeping
the allocated structure, but at the end of the day any implementation
based on this design will have the same problem that the previous design
had, namely performance.

So, the problem that this design is trying to solve is a memory leak.  I
would rather just focus on fixing that problem.  So, there are a couple
of things here,  1)  The apr_pool_t that is in the current code is only
there because I needed it for the apr_palloc(), if that call is removed
so should the apr_pool_t.  2)  This is a multi-pronged approach.  The
assumption is that modern platforms will get the most benefit, but all
platforms will have the leak removed.  With no further ado, the approach
I would suggest.

The idea is that we will still end up creating the pollfd_t on every
call to apr_poll(), this does have a performance penalty, but on modern
platforms, that performance penalty is constant.  The only real
difference is how you allocate the array.  There are three options:

1)  C99 specifies Variable Length Arrays (VLAs) specifically to solve
this problem.  If your compiler supports it (gcc does on Linux at least,
as does MSVC I believe), then we create an array on the stack with a
size of the num argument.  (Obviously, we would need some autoconf
macros to determine if the platform supports VLAs.)

2)  For platforms that don't support VLAs, we create an array of size
FOO (value to be determined later).  Because this is allocated on the
stack, we just need to fill it out and call poll().

3)  If the caller needs more than FOO elements, we use malloc() and
free() to allocate the pollfd_t array.  This is an isolated call to
malloc/free, only required on platforms that don't support VLAs, and can
be optimized out if the user sets FOO correctly at compile time.  (For
Apache, the default will most likely be < 10, but we can experiment for
the best value).

Obviously FOO will need to be named better, but that is only important
if we take this route.


> Ok, we have real problems with the palloc in the new design of
> But it's fundamentally better than falling back on poll() because our
> design
> just sucks, performance-wise.  Ryan speaks of alloca, but that won't
> work on all platforms, most especially stack-impaired platforms such
> Netware without the ability to grow the stack.
> So here's a design suggestion.  Take the apr_pollfd_t element;
> struct apr_pollfd_t {
>      apr_pool_t *p;
>      apr_datatype_e desc_type;
>      apr_int16_t reqevents;
>      apr_int16_t rtnevents;
>      apr_descriptor desc;
> };
> If we pull the apr_pool_t *p [which is WAY overkill if you have
> dozen to hundreds of descriptors you might be polling against] and
> create a brand new apr_pollfd_set_t [also transparent!]...
> typedef struct {
>      apr_pool_t *p;
>      apr_pollfd_internal_t *int;
>      apr_pollfd_t *first;
>      int count;
> } apr_pollfd_set_t;
> Modify apr_poll_setup to return a new apr_pollfd_set_t, with *first
> already pointing at a newly allocated apr_pollfd_t array, and set up
> apr_pollfd_internal_t to NULL.
> The *int pointer tracks APR's internal count and arrays that it needs.
> If the count to an invocation of apr_poll() exceeds our internally
> allocated
> count, THEN we go and allocate a larger internal structure.  But not
> every pass into apr_poll().
>    . we stop the leak.  Once we grow to some given *int pollset size,
>      we won't be growing anymore.
>    . we lighten the apr_pollfd_t elements by one pool pointer.
>    . using *first, we can change our offset into a huge array of
>    . using count, we can resize the array we are interested in.
>    . neat feature, we actually can have several apr_pollfd_set_t
>      indexes floating around, neatly pointing into different parts
>      of a huge apr_pollfd_t array.
>    . if not using apr_poll_setup(), the user is absolutely responsible
>      for initializing *v to NULL.
>    . two structures to think about.  Not complex structures, but two,
>      none the less.
> Comments or ideas?  This was idle brainstorming.
> Bill
>    .

View raw message