apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Brian Pane <bri...@apache.org>
Subject Re: [design] work around new apr_poll leakage?
Date Fri, 19 Jul 2002 04:21:18 GMT
I'm almost in complete agreement with this proposal.  We
need a wrapper object to hold the temp pollset so that we
can re-use it for all subsequent poll calls on the same
fd set.

The one problem with this design is that the structures
are transparent.  It's a bad idea to expose the representation
of the poll set as a vector of apr_pollfd_t elements.  If we
later add /dev/poll support, we'll need to call an ioctl when
the app adds a descriptor to an existing poll set.  We can make
that happen if the app adds descriptors by calling a function,
but not if it passes in a vector.  Yes, we could add a separate
type of object for /dev/poll, and make that one nontransparent,
but then our poll wrapper and our /dev/poll wrapper would have
very different interfaces.

  -1 on the current apr_poll code with the memory leak
  +1 on Bill's proposal to add the apr_pollfd_set_t wrapper object
  -0.5 on making the structures transparent


William A. Rowe, Jr. wrote:

> Ok, we have real problems with the palloc in the new design of 
> apr_poll().
> 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 as
> 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 several
> 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 for
> 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 elements.
>   . 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