From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: X-Spam-Status: No, score=-4.0 required=3.0 tests=ALL_TRUSTED,AWL,BAYES_00 shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from localhost (dcvr.yhbt.net [127.0.0.1]) by dcvr.yhbt.net (Postfix) with ESMTP id 751C51F9F9 for ; Mon, 10 Jun 2019 05:18:48 +0000 (UTC) From: Eric Wong To: meta@public-inbox.org Subject: [PATCH 9/9] ds: stop caring about event flags set by epoll/poll/kqueue Date: Mon, 10 Jun 2019 05:18:46 +0000 Message-Id: <20190610051846.26757-10-e@80x24.org> In-Reply-To: <20190610051846.26757-1-e@80x24.org> References: <20190610051846.26757-1-e@80x24.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit List-Id: If we got something to write, then write it. Otherwise, try reading; and continue dealing with errors which normally occur along the way. Trying to read requests while we need to buffer in luserspace is suicidal from a memory management standpoint. The only adjustment needed for existing callers is EvCleanup; where we need to ensure we're always calling the dummy EvCleanup::event_write callback to accomplish nothing. --- lib/PublicInbox/DS.pm | 52 +++++++++++------------------------- lib/PublicInbox/EvCleanup.pm | 8 ++++-- 2 files changed, 21 insertions(+), 39 deletions(-) diff --git a/lib/PublicInbox/DS.pm b/lib/PublicInbox/DS.pm index 9277981b..2f028a36 100644 --- a/lib/PublicInbox/DS.pm +++ b/lib/PublicInbox/DS.pm @@ -239,15 +239,18 @@ sub RunTimers { return $timeout; } -# Placeholder callback when we hit POLLERR/POLLHUP or other unrecoverable -# errors. Shouldn't be needed in the future. -sub event_end ($) { +sub event_step ($) { my ($self) = @_; return if $self->{closed}; - $self->{wbuf} = []; - $self->{wbuf_off} = 0; - # we're screwed if a read handler can't handle POLLERR/POLLHUP-type errors + my $wbuf = $self->{wbuf}; + if (@$wbuf) { + $self->event_write; + return if $self->{closed} || scalar(@$wbuf); + } + + # only read more requests if we've drained the write buffer, + # otherwise we can be buffering infinitely w/o backpressure $self->event_read; } @@ -270,19 +273,7 @@ sub EpollEventLoop { # that ones in the front triggered unregister-interest actions. if we # can't find the %sock entry, it's because we're no longer interested # in that event. - my PublicInbox::DS $pob = $DescriptorMap{$ev->[0]}; - my $code; - my $state = $ev->[1]; - - DebugLevel >= 1 && $class->DebugMsg("Event: fd=%d (%s), state=%d \@ %s\n", - $ev->[0], ref($pob), $ev->[1], time); - - # standard non-profiling codepat - $pob->event_read if $state & EPOLLIN && ! $pob->{closed}; - $pob->event_write if $state & EPOLLOUT && ! $pob->{closed}; - if ($state & (EPOLLERR|EPOLLHUP) && ! $pob->{closed}) { - event_end($pob); - } + event_step($DescriptorMap{$ev->[0]}); } return unless PostEventLoop(); } @@ -327,11 +318,7 @@ sub PollEventLoop { my ($fd, $state) = splice(@poll, 0, 2); next unless $state; - $pob = $DescriptorMap{$fd}; - - $pob->event_read if $state & POLLIN && ! $pob->{closed}; - $pob->event_write if $state & POLLOUT && ! $pob->{closed}; - event_end($pob) if $state & (POLLERR|POLLHUP) && ! $pob->{closed}; + event_step($DescriptorMap{$fd}); } return unless PostEventLoop(); @@ -359,16 +346,7 @@ sub KQueueEventLoop { foreach my $kev (@ret) { my ($fd, $filter, $flags, $fflags) = @$kev; - my PublicInbox::DS $pob = $DescriptorMap{$fd}; - - DebugLevel >= 1 && $class->DebugMsg("Event: fd=%d (%s), flags=%d \@ %s\n", - $fd, ref($pob), $flags, time); - - $pob->event_read if $filter == IO::KQueue::EVFILT_READ() && !$pob->{closed}; - $pob->event_write if $filter == IO::KQueue::EVFILT_WRITE() && !$pob->{closed}; - if ($flags == IO::KQueue::EV_EOF() && !$pob->{closed}) { - event_end($pob); - } + event_step($DescriptorMap{$fd}); } return unless PostEventLoop(); } @@ -672,11 +650,11 @@ sub on_incomplete_write { =head2 (VIRTUAL) C<< $obj->event_read() >> Readable event handler. Concrete deriviatives of PublicInbox::DS should -provide an implementation of this. The default implementation will die if -called. +provide an implementation of this. The default implementation is a noop +if called. =cut -sub event_read { die "Base class event_read called for $_[0]\n"; } +sub event_read {} # noop =head2 C<< $obj->event_write() >> diff --git a/lib/PublicInbox/EvCleanup.pm b/lib/PublicInbox/EvCleanup.pm index afed24ff..f76fb681 100644 --- a/lib/PublicInbox/EvCleanup.pm +++ b/lib/PublicInbox/EvCleanup.pm @@ -6,7 +6,6 @@ package PublicInbox::EvCleanup; use strict; use warnings; use base qw(PublicInbox::DS); -use fields qw(rd); my $ENABLED; sub enabled { $ENABLED } @@ -25,7 +24,12 @@ sub once_init () { pipe($r, $w) or die "pipe: $!"; fcntl($w, 1031, 4096) if $^O eq 'linux'; # 1031: F_SETPIPE_SZ $self->SUPER::new($w); - $self->{rd} = $r; # never read, since we never write.. + + # always writable, since PublicInbox::EvCleanup::event_write + # never drains wbuf. We can avoid wasting a hash slot by + # stuffing the read-end of the pipe into the never-to-be-touched + # wbuf + push @{$self->{wbuf}}, $r; $self; } -- EW