You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@apr.apache.org by jo...@apache.org on 2004/07/07 09:40:12 UTC
cvs commit: apr configure.in
jorton 2004/07/07 00:40:12
Modified: poll/unix poll.c
. configure.in
Log:
* poll/unix/poll.c (backend_cleanup): Only define if using epoll/kqueue.
(apr_pollset_destroy): Only run cleanup if using epoll/kqueue.
(apr_pollset_add): Fix warning for ye olde platforms still using poll().
* configure.in: Just check for kqueue() in an AC_CHECK_FUNCS.
Revision Changes Path
1.46 +7 -1 apr/poll/unix/poll.c
Index: poll.c
===================================================================
RCS file: /home/cvs/apr/poll/unix/poll.c,v
retrieving revision 1.45
retrieving revision 1.46
diff -d -w -u -r1.45 -r1.46
--- poll.c 6 Jul 2004 03:38:06 -0000 1.45
+++ poll.c 7 Jul 2004 07:40:12 -0000 1.46
@@ -374,6 +374,7 @@
#endif
};
+#if defined(HAVE_KQUEUE) || defined(HAVE_EPOLL)
static apr_status_t backend_cleanup(void *p_)
{
apr_pollset_t *pollset = (apr_pollset_t *)p_;
@@ -384,6 +385,7 @@
#endif
return APR_SUCCESS;
}
+#endif /* HAVE_KQUEUE || HAVE_EPOLL */
APR_DECLARE(apr_status_t) apr_pollset_create(apr_pollset_t **pollset,
apr_uint32_t size,
@@ -433,7 +435,11 @@
APR_DECLARE(apr_status_t) apr_pollset_destroy(apr_pollset_t *pollset)
{
+#if defined(HAVE_KQUEUE) || defined(HAVE_EPOLL)
return apr_pool_cleanup_run(pollset->pool, pollset, backend_cleanup);
+#else
+ return APR_SUCCESS;
+#endif
}
APR_DECLARE(apr_status_t) apr_pollset_add(apr_pollset_t *pollset,
@@ -571,7 +577,7 @@
#elif defined(HAVE_EPOLL)
struct epoll_event ev;
int ret = -1;
-#elif defined(HAVE_POLL)
+#elif !defined(HAVE_POLL)
apr_os_sock_t fd;
#endif
1.595 +3 -6 apr/configure.in
Index: configure.in
===================================================================
RCS file: /home/cvs/apr/configure.in,v
retrieving revision 1.594
retrieving revision 1.595
diff -d -w -u -r1.594 -r1.595
--- configure.in 6 Jul 2004 03:38:06 -0000 1.594
+++ configure.in 7 Jul 2004 07:40:12 -0000 1.595
@@ -636,13 +636,10 @@
AC_SUBST(have_sigsuspend)
AC_SUBST(have_sigwait)
-AC_CHECK_FUNCS(poll)
-
-# Checks for the FreeBSD KQueue and Linux epoll interfaces:
-AC_CHECK_FUNC(kevent,
- [AC_DEFINE([HAVE_KQUEUE], 1, [Define if the KQueue interface is supported])])
+AC_CHECK_FUNCS(poll kqueue)
-# epoll* may be available in libc but return ENOSYS on a pre-2.6 kernel.
+# Check for the Linux epoll interface; epoll* may be available in libc
+# but return ENOSYS on a pre-2.6 kernel, so do a run-time check.
AC_CACHE_CHECK([for epoll support], [apr_cv_epoll],
[AC_TRY_RUN([
#include <sys/epoll.h>
Re: cvs commit: apr configure.in
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
--On Thursday, July 8, 2004 8:32 AM +0100 Joe Orton <jo...@redhat.com>
wrote:
> Since we know the cleanup does nothing, defining it unnecessarily wastes
> both a few bytes of code size and a few cycles at run-time. e.g. since
> the cleanup is never registered for the non-epoll/kqueue case, running
> apr_pool_cleanup_destroy() will iterate over the whole cleanup list
> looking for it if apr_poll_destroy() is called.
Well, we follow this idiom several other places in APR - see shmem. (I
half expect you to go through and rewrite shmem now that I point that out.)
> The "unreal?" poll/select implementations will be used on the majority
> of APR platforms and do *not* currently require a cleanup, no "perhaps"
> about it. This is just a minor optimisation with minor cleanliness cost
> for the majority of platforms, I don't think it's a big deal.
It's just that we've never gotten around to enabling the platform-specific
optimizations for the platforms until now. FreeBSD, Darwin, Linux will
already hit them with these latest patches. Eventually, Solaris would too.
(I bet AIX and HP-UX have similar pollset implementations, but I don't know
for sure.) To me, that means the majority of Unix platforms will require a
cleanup - this means the conditional is going to get really long and messy.
So, the extra marginal cost on those platforms using the fallback
implementation is gained back by the ease of maintaining the code by
reducing how many places we need to define the conditionals.
I realize it's a minor point, but I guess I would have liked to have
discussion on these changes first. *shrug* -- justin
Re: cvs commit: apr configure.in
Posted by Joe Orton <jo...@redhat.com>.
On Thu, Jul 08, 2004 at 12:04:48AM -0700, Justin Erenkrantz wrote:
> --On Wednesday, July 7, 2004 7:40 AM +0000 jorton@apache.org wrote:
>
> >jorton 2004/07/07 00:40:12
> >
> > Modified: poll/unix poll.c
> > . configure.in
> > Log:
> > * poll/unix/poll.c (backend_cleanup): Only define if using epoll/kqueue.
> > (apr_pollset_destroy): Only run cleanup if using epoll/kqueue.
> > (apr_pollset_add): Fix warning for ye olde platforms still using poll().
>
> I changed this from Paul's patch on purpose. Why shouldn't we always
> define the cleanup even if we don't have kqueue or epoll?
Since we know the cleanup does nothing, defining it unnecessarily wastes
both a few bytes of code size and a few cycles at run-time. e.g. since
the cleanup is never registered for the non-epoll/kqueue case, running
apr_pool_cleanup_destroy() will iterate over the whole cleanup list
looking for it if apr_poll_destroy() is called.
> If we add Solaris's /dev/poll, we'd need a cleanup. I see only the
> fallback implementation perhaps not needing it - almost any real
> implementation would. I thought doing it your way makes the code a
> lot less clean.
The "unreal?" poll/select implementations will be used on the majority
of APR platforms and do *not* currently require a cleanup, no "perhaps"
about it. This is just a minor optimisation with minor cleanliness cost
for the majority of platforms, I don't think it's a big deal.
joe
Re: cvs commit: apr configure.in
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
--On Wednesday, July 7, 2004 7:40 AM +0000 jorton@apache.org wrote:
> jorton 2004/07/07 00:40:12
>
> Modified: poll/unix poll.c
> . configure.in
> Log:
> * poll/unix/poll.c (backend_cleanup): Only define if using epoll/kqueue.
> (apr_pollset_destroy): Only run cleanup if using epoll/kqueue.
> (apr_pollset_add): Fix warning for ye olde platforms still using poll().
I changed this from Paul's patch on purpose. Why shouldn't we always
define the cleanup even if we don't have kqueue or epoll? If we add
Solaris's /dev/poll, we'd need a cleanup. I see only the fallback
implementation perhaps not needing it - almost any real implementation
would. I thought doing it your way makes the code a lot less clean.
*shrug* -- justin