Hi,
a call to apr_pollset_add or apr_pollset_remove with both APR_POLLIN
and APR_POLLOUT event filters requested will result in two kevent
system calls, where only one is needed.
Attached patch will fix that. I've tested both the pollset and pollcb
API's with this patch on Mac OS X with a modified serf client (with
10..100..1000 connections) and everything works as expected.
It's difficult to measure the performance benefits this brings. While
I see that the nr. of calls to kevent goes down and the time spent in
that call too, the margin of error on the total runtime of my tests is
much bigger than the time gained with the reduced nr. of kevent alls.
But, given that one of the benefits of kqueue is the possibility to
modify multiple event filters with one system call, I think making
this change is the right thing to do.
regards,
Lieven
[[[
Combine multiple kevent calls into one.
* poll/unix/kqueue.c
(struct apr_pollset_private_t): Remove the kevent member variable here, it
can be defined in the function scope of functions where needed.
(impl_pollset_add, impl_pollset_remove,
impl_pollcb_add, impl_pollcb_remove): Create both the event structures for
APR_POLLIN and APR_POLLOUT first, then call kevent once with both.
]]]
Index: poll/unix/kqueue.c
===================================================================
--- poll/unix/kqueue.c (revision 1592163)
+++ poll/unix/kqueue.c (working copy)
@@ -46,7 +46,6 @@ static apr_int16_t get_kqueue_revent(apr_int16_t e
struct apr_pollset_private_t
{
int kqueue_fd;
- struct kevent kevent;
apr_uint32_t setsize;
struct kevent *ke_set;
apr_pollfd_t *result_set;
@@ -137,6 +136,8 @@ static apr_status_t impl_pollset_add(apr_pollset_t
{
apr_os_sock_t fd;
pfd_elem_t *elem;
+ struct kevent ev[2];
+ apr_uint16_t nchanges = 0;
apr_status_t rv = APR_SUCCESS;
pollset_lock_rings();
@@ -159,21 +160,17 @@ static apr_status_t impl_pollset_add(apr_pollset_t
}
if (descriptor->reqevents & APR_POLLIN) {
- EV_SET(&pollset->p->kevent, fd, EVFILT_READ, EV_ADD, 0, 0, elem);
-
- if (kevent(pollset->p->kqueue_fd, &pollset->p->kevent, 1, NULL, 0,
- NULL) == -1) {
- rv = apr_get_netos_error();
- }
+ EV_SET(&ev[0], fd, EVFILT_READ, EV_ADD, 0, 0, elem);
+ nchanges++;
}
if (descriptor->reqevents & APR_POLLOUT && rv == APR_SUCCESS) {
- EV_SET(&pollset->p->kevent, fd, EVFILT_WRITE, EV_ADD, 0, 0, elem);
+ EV_SET(&ev[nchanges++], fd, EVFILT_WRITE, EV_ADD, 0, 0,
+ elem);
+ }
- if (kevent(pollset->p->kqueue_fd, &pollset->p->kevent, 1, NULL, 0,
- NULL) == -1) {
- rv = apr_get_netos_error();
- }
+ if (kevent(pollset->p->kqueue_fd, &ev[0], nchanges, NULL, 0, NULL) == -1) {
+ rv = apr_get_netos_error();
}
if (rv == APR_SUCCESS) {
@@ -193,6 +190,8 @@ static apr_status_t impl_pollset_remove(apr_pollse
{
pfd_elem_t *ep;
apr_status_t rv;
+ struct kevent ev[2];
+ apr_uint16_t nchanges = 0;
apr_os_sock_t fd;
pollset_lock_rings();
@@ -206,21 +205,16 @@ static apr_status_t impl_pollset_remove(apr_pollse
rv = APR_NOTFOUND; /* unless at least one of the specified conditions is */
if (descriptor->reqevents & APR_POLLIN) {
- EV_SET(&pollset->p->kevent, fd, EVFILT_READ, EV_DELETE, 0, 0, NULL);
-
- if (kevent(pollset->p->kqueue_fd, &pollset->p->kevent, 1, NULL, 0,
- NULL) != -1) {
- rv = APR_SUCCESS;
- }
+ EV_SET(&ev[0], fd, EVFILT_READ, EV_DELETE, 0, 0, NULL);
+ nchanges++;
}
if (descriptor->reqevents & APR_POLLOUT) {
- EV_SET(&pollset->p->kevent, fd, EVFILT_WRITE, EV_DELETE, 0, 0, NULL);
+ EV_SET(&ev[nchanges++], fd, EVFILT_WRITE, EV_DELETE, 0, 0, NULL);
+ }
- if (kevent(pollset->p->kqueue_fd, &pollset->p->kevent, 1, NULL, 0,
- NULL) != -1) {
- rv = APR_SUCCESS;
- }
+ if (kevent(pollset->p->kqueue_fd, &ev[0], nchanges, NULL, 0, NULL) != -1) {
+ rv = APR_SUCCESS;
}
for (ep = APR_RING_FIRST(&(pollset->p->query_ring));
@@ -357,7 +351,8 @@ static apr_status_t impl_pollcb_add(apr_pollcb_t *
apr_pollfd_t *descriptor)
{
apr_os_sock_t fd;
- struct kevent ev;
+ struct kevent ev[2];
+ apr_uint16_t nchanges = 0;
apr_status_t rv = APR_SUCCESS;
if (descriptor->desc_type == APR_POLL_SOCKET) {
@@ -368,21 +363,18 @@ static apr_status_t impl_pollcb_add(apr_pollcb_t *
}
if (descriptor->reqevents & APR_POLLIN) {
- EV_SET(&ev, fd, EVFILT_READ, EV_ADD, 0, 0, descriptor);
-
- if (kevent(pollcb->fd, &ev, 1, NULL, 0, NULL) == -1) {
- rv = apr_get_netos_error();
- }
+ EV_SET(&ev[0], fd, EVFILT_READ, EV_ADD, 0, 0, descriptor);
+ nchanges++;
}
- if (descriptor->reqevents & APR_POLLOUT && rv == APR_SUCCESS) {
- EV_SET(&ev, fd, EVFILT_WRITE, EV_ADD, 0, 0, descriptor);
-
- if (kevent(pollcb->fd, &ev, 1, NULL, 0, NULL) == -1) {
- rv = apr_get_netos_error();
- }
+ if (descriptor->reqevents & APR_POLLOUT) {
+ EV_SET(&ev[nchanges++], fd, EVFILT_WRITE, EV_ADD, 0, 0, descriptor);
}
-
+
+ if (kevent(pollcb->fd, &ev[0], nchanges, NULL, 0, NULL) == -1) {
+ rv = apr_get_netos_error();
+ }
+
return rv;
}
@@ -390,7 +382,8 @@ static apr_status_t impl_pollcb_remove(apr_pollcb_
apr_pollfd_t *descriptor)
{
apr_status_t rv;
- struct kevent ev;
+ struct kevent ev[2];
+ apr_uint16_t nchanges = 0;
apr_os_sock_t fd;
if (descriptor->desc_type == APR_POLL_SOCKET) {
@@ -402,21 +395,18 @@ static apr_status_t impl_pollcb_remove(apr_pollcb_
rv = APR_NOTFOUND; /* unless at least one of the specified conditions is */
if (descriptor->reqevents & APR_POLLIN) {
- EV_SET(&ev, fd, EVFILT_READ, EV_DELETE, 0, 0, NULL);
-
- if (kevent(pollcb->fd, &ev, 1, NULL, 0, NULL) != -1) {
- rv = APR_SUCCESS;
- }
+ EV_SET(&ev[0], fd, EVFILT_READ, EV_DELETE, 0, 0, NULL);
+ nchanges++;
}
if (descriptor->reqevents & APR_POLLOUT) {
- EV_SET(&ev, fd, EVFILT_WRITE, EV_DELETE, 0, 0, NULL);
-
- if (kevent(pollcb->fd, &ev, 1, NULL, 0, NULL) != -1) {
- rv = APR_SUCCESS;
- }
+ EV_SET(&ev[nchanges++], fd, EVFILT_WRITE, EV_DELETE, 0, 0, NULL);
}
-
+
+ if (kevent(pollcb->fd, &ev[0], nchanges, NULL, 0, NULL) != -1) {
+ rv = APR_SUCCESS;
+ }
+
return rv;
}
