apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ryan Bloom" <...@covalent.net>
Subject RE: New apr_poll() implementation
Date Wed, 10 Jul 2002 00:59:05 GMT
> From: Sander Striker [mailto:striker@apache.org]
> > From: Ryan Bloom [mailto:rbb@covalent.net]
> > Sent: 09 July 2002 15:46
> >> From: Ryan Bloom [mailto:rbb@covalent.net]
> >>> From: Justin Erenkrantz [mailto:jerenkrantz@apache.org]
> >>> On Sat, Jul 06, 2002 at 12:11:59PM -0700, Ryan Bloom wrote:
> >>>> parameters.  I would like to fix that mistake for apr_poll now,
> >>>> long as we are changing the implementation.
> >>>
> >>> Getting back to this conversation for a brief second, I think the
> >>> additional parameter with the fd count is unneeded (but for a
> >>> different reason than IN/OUT).  The count should be stored in
> >>> apr_pollfd_t - it does not need to be passed into apr_poll().
> >>
> >> The count should absolutely NOT be stored in apr_pollfd_t.  That is
> >> user owned structure, not an APR owned structure.
> >
> > I should clarify this.  The whole point of this change, is that the
> > can do the following:
> [...]
> > That code will work.  Notice that no where before the apr_poll, did
> > tell the poll implementation the size of the pollset.  Nor can we.
> > is where the real performance benefits of this patch come in.
> > the user has complete control over the pollset, we can remove all
> > one pool allocation, and that one can be removed with an
> > for small pollsets.
> >
> > Now, we can make macros to add sockets and files, but we should not
> > be telling APR how many descriptors we are polling until we call
> > apr_poll().  The functions that currently setup the pollset and add
> > sockets and set events and revents are all deprecated, and were only
> > left in the patch to allow for as much source backwards compat as
> > possible while the patch is being discussed.  Once the patch has
> > committed, I will go through the source and start to remove those
> > functions completely.  The only real example of apr_poll() that is a
> > part of the patch is wait_for_io_or_timeout.  I probably didn't make
> > that clear enough.
> Ah, no.  Without the accessor functions it doesn't make much sense
> to store the pollset size in apr_pollfd_t.  I surely didn't see
> the accessor functions comming.  Not sure if I like that (+0, -0).  I
> see the perf benefit, but if it makes for a better API, I don't know.

You can keep the accessor functions if you want to, but you don't need
them, and they shouldn't be functions regardless.  The whole point of
the new poll implementation is that it opens up the structure for
anybody to manage the memory.  If we keep the accessors, then we should
use macros not functions.

Either way, the setup function goes away, because the user can allocate
the memory, and there is no place to give the size of the structure to
the apr_pollfd_t before the call to apr_poll().


View raw message