ds: move maxevents further down the stack

The epoll implementation is the only one which respects the
limit (kevent would, but IO::KQueue does not).  In any case,
I'm not a fan of the maxevents=1000 historical default since
it leads to fairness problems with shared non-blocking listeners
across multiple daemon workers.

diff --git a/lib/PublicInbox/DS.pm b/lib/PublicInbox/DS.pm
index b30e9db64014ebcb6c3e051dea12bee8d6d9280e..9e1f66c2da4edf4a94cbea90fcc752f36a3b3b72 100644 (file)
--- a/lib/PublicInbox/DS.pm
+++ b/lib/PublicInbox/DS.pm
@@ -287,8 +287,8 @@ sub event_loop (;$$) {
        do {
                my $timeout = RunTimers();
 
-               # get up to 1000 FDs representing events
-               $Poller->ep_wait(1000, $timeout, \@active);
+               # grab whatever FDs are ready
+               $Poller->ep_wait($timeout, \@active);
 
                # map all FDs to their associated Perl object
                @active = @DescriptorMap{@active};

diff --git a/lib/PublicInbox/DSKQXS.pm b/lib/PublicInbox/DSKQXS.pm
index 8ef8ffb6f5c2c1a9bc7ae5419804275e6b9a66d9..f84c196a56cdbbcb4b7c3acaa46d626eca5a612e 100644 (file)
--- a/lib/PublicInbox/DSKQXS.pm
+++ b/lib/PublicInbox/DSKQXS.pm
@@ -117,7 +117,8 @@ sub ep_del {
 }
 
 sub ep_wait {
-       my ($self, $maxevents, $timeout_msec, $events) = @_;
+       my ($self, $timeout_msec, $events) = @_;
+       # n.b.: IO::KQueue is hard-coded to return up to 1000 events
        @$events = eval { $self->{kq}->kevent($timeout_msec) };
        if (my $err = $@) {
                # workaround https://rt.cpan.org/Ticket/Display.html?id=116615

diff --git a/lib/PublicInbox/DSPoll.pm b/lib/PublicInbox/DSPoll.pm
index 0446df4c2788044d2055be3b95d110cfee4df10b..b947f75605c37fd05c329f4da90d021806bb5636 100644 (file)
--- a/lib/PublicInbox/DSPoll.pm
+++ b/lib/PublicInbox/DSPoll.pm
@@ -18,7 +18,7 @@ use Errno ();
 sub new { bless {}, __PACKAGE__ } # fd => events
 
 sub ep_wait {
-       my ($self, $maxevents, $timeout_msec, $events) = @_;
+       my ($self, $timeout_msec, $events) = @_;
        my (@pset, $n, $fd, $revents, $nval);
        while (my ($fd, $events) = each %$self) {
                my $pevents = $events & EPOLLIN ? POLLIN : 0;

diff --git a/lib/PublicInbox/Epoll.pm b/lib/PublicInbox/Epoll.pm
index d55c8535d4008d24e5016111af7e5ef1227c8554..7e0aa6e7921d4d5ae7a05aad9a973fd82065ad9e 100644 (file)
--- a/lib/PublicInbox/Epoll.pm
+++ b/lib/PublicInbox/Epoll.pm
@@ -18,6 +18,9 @@ sub new {
 sub ep_add { epoll_ctl(fileno(${$_[0]}), EPOLL_CTL_ADD, fileno($_[1]), $_[2]) }
 sub ep_mod { epoll_ctl(fileno(${$_[0]}), EPOLL_CTL_MOD, fileno($_[1]), $_[2]) }
 sub ep_del { epoll_ctl(fileno(${$_[0]}), EPOLL_CTL_DEL, fileno($_[1]), 0) }
-sub ep_wait { epoll_wait(fileno(${$_[0]}), @_[1, 2, 3]) }
+
+# n.b. maxevents=1000 is the historical default.  maxevents=1 (yes, one)
+# is more fair under load with multiple worker processes sharing one listener
+sub ep_wait { epoll_wait(fileno(${$_[0]}), 1000, @_[1, 2]) }
 
 1;

diff --git a/lib/PublicInbox/Select.pm b/lib/PublicInbox/Select.pm
index 5817c9ef0c7b5e36b5e8b6704def37239f852ebd..5cb7aff3c42c8ecf848abb89c657e620a62e6fa4 100644 (file)
--- a/lib/PublicInbox/Select.pm
+++ b/lib/PublicInbox/Select.pm
@@ -12,7 +12,7 @@ use PublicInbox::Syscall qw(EPOLLONESHOT EPOLLIN EPOLLOUT);
 sub new { bless {}, __PACKAGE__ } # fd => events
 
 sub ep_wait {
-       my ($self, $maxevents, $msec, $events) = @_;
+       my ($self, $msec, $events) = @_;
        my ($rvec, $wvec) = ('', ''); # we don't use EPOLLERR
        while (my ($fd, $ev) = each %$self) {
                vec($rvec, $fd, 1) = 1 if $ev & EPOLLIN;

diff --git a/t/ds-poll.t b/t/ds-poll.t
index 7a7e2bcfa5652715094cca42621e5cd7ed530368..321534bddbe2a9d3f4fc1933b8273fad9f4d6cc4 100644 (file)
--- a/t/ds-poll.t
+++ b/t/ds-poll.t
@@ -16,33 +16,33 @@ pipe($r, $w);
 pipe($x, $y);
 is($p->ep_add($r, EPOLLIN), 0, 'add EPOLLIN');
 my $events = [];
-$p->ep_wait(9, 0, $events);
+$p->ep_wait(0, $events);
 is_deeply($events, [], 'no events set');
 is($p->ep_add($w, EPOLLOUT|EPOLLONESHOT), 0, 'add EPOLLOUT|EPOLLONESHOT');
-$p->ep_wait(9, -1, $events);
+$p->ep_wait(-1, $events);
 is(scalar(@$events), 1, 'got POLLOUT event');
 is($events->[0], fileno($w), '$w ready');
 
-$p->ep_wait(9, 0, $events);
+$p->ep_wait(0, $events);
 is(scalar(@$events), 0, 'nothing ready after oneshot');
 is_deeply($events, [], 'no events set after oneshot');
 
 syswrite($w, '1') == 1 or die;
 for my $t (0..1) {
-       $p->ep_wait(9, $t, $events);
+       $p->ep_wait($t, $events);
        is($events->[0], fileno($r), "level-trigger POLLIN ready #$t");
        is(scalar(@$events), 1, "only event ready #$t");
 }
 syswrite($y, '1') == 1 or die;
 is($p->ep_add($x, EPOLLIN|EPOLLONESHOT), 0, 'EPOLLIN|EPOLLONESHOT add');
-$p->ep_wait(9, -1, $events);
+$p->ep_wait(-1, $events);
 is(scalar @$events, 2, 'epoll_wait has 2 ready');
 my @fds = sort @$events;
 my @exp = sort((fileno($r), fileno($x)));
 is_deeply(\@fds, \@exp, 'got both ready FDs');
 
 is($p->ep_del($r, 0), 0, 'EPOLL_CTL_DEL OK');
-$p->ep_wait(9, 0, $events);
+$p->ep_wait(0, $events);
 is(scalar @$events, 0, 'nothing ready after EPOLL_CTL_DEL');
 
 is($p->ep_add($r, EPOLLIN), 0, 're-add');
@@ -54,7 +54,7 @@ SKIP: {
        my @w;
        eval {
                local $SIG{__WARN__} = sub { push @w, @_ };
-               $p->ep_wait(9, 0, $events);
+               $p->ep_wait(0, $events);
        };
        ok($@, 'error detected from bad FD');
        ok($!{EBADF}, 'EBADF errno set');

diff --git a/t/epoll.t b/t/epoll.t
index 54dc6f47ec793f0ae222e26185c72c49f7f94eea..ab9ac22ac2661092604196f52b722e59d8e4eca5 100644 (file)
--- a/t/epoll.t
+++ b/t/epoll.t
@@ -12,11 +12,11 @@ pipe(my $r, my $w);
 is($ep->ep_add($w, EPOLLOUT), 0, 'epoll_ctl pipe EPOLLOUT');
 
 my @events;
-$ep->ep_wait(100, 10000, \@events);
+$ep->ep_wait(10000, \@events);
 is(scalar(@events), 1, 'got one event');
 is($events[0], fileno($w), 'got expected FD');
 close $w;
-$ep->ep_wait(100, 0, \@events);
+$ep->ep_wait(0, \@events);
 is(scalar(@events), 0, 'epoll_wait timeout');
 
 done_testing;