From fdf90c0ffbf608ed08665eaffa5c750fa5a5bfee Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Tue, 31 Oct 2023 20:42:52 +0000 Subject: 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. --- lib/PublicInbox/DS.pm | 4 ++-- lib/PublicInbox/DSKQXS.pm | 3 ++- lib/PublicInbox/DSPoll.pm | 2 +- lib/PublicInbox/Epoll.pm | 5 ++++- lib/PublicInbox/Select.pm | 2 +- t/ds-poll.t | 14 +++++++------- t/epoll.t | 4 ++-- 7 files changed, 19 insertions(+), 15 deletions(-) diff --git a/lib/PublicInbox/DS.pm b/lib/PublicInbox/DS.pm index b30e9db6..9e1f66c2 100644 --- 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 8ef8ffb6..f84c196a 100644 --- 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 0446df4c..b947f756 100644 --- 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 d55c8535..7e0aa6e7 100644 --- 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 5817c9ef..5cb7aff3 100644 --- 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 7a7e2bcf..321534bd 100644 --- 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 54dc6f47..ab9ac22a 100644 --- 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; -- cgit v1.2.3-24-ge0c7