From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: X-Spam-Status: No, score=-4.2 required=3.0 tests=ALL_TRUSTED,AWL,BAYES_00, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF, T_SCC_BODY_TEXT_LINE shortcircuit=no autolearn=ham autolearn_force=no version=3.4.6 Received: from localhost (dcvr.yhbt.net [127.0.0.1]) by dcvr.yhbt.net (Postfix) with ESMTP id 7FEBFD482F; Thu, 20 Jun 2024 22:39:12 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=80x24.org; s=selector1; t=1718923152; bh=FsfVzL7xPIPqsFQq9y137eP+XYVUSjix6Qm7rwztf80=; h=Date:From:To:Subject:References:In-Reply-To:From; b=CzMRFuDu80cjpAHaKp7ac1LNnbG5g5R8rkkWATz+rGoRijEU7yiZAwwN81C7YXxO3 YO/6c6mRVdrzqgAKwOtvMRD15HFSUHNwrN5YRzEVWeYQFvYmRetwaUJHPupeyWCaNi mPx1g56MVNHBXVrQNjt7GYPlJCpCBCrCxZ+vrmGw= Date: Thu, 20 Jun 2024 22:39:12 +0000 From: Eric Wong To: test@public-inbox.org Subject: Re: [PATCH 5/5] http: use writev for known Content-Length responses Message-ID: <20240620223912.M396971@dcvr> References: <20240619234104.80183-1-e@80x24.org> <20240619234104.80183-6-e@80x24.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20240619234104.80183-6-e@80x24.org> List-Id: Eric Wong wrote: > We could use sendmsg(2) without MSG_MORE here, too, but > writev(2) is simpler to setup and call and we may want to use it > with pipes or regular files in the future, too, not just sockets. OK... (test) > --- > devel/sysdefs-list | 1 + > lib/PublicInbox/DS.pm | 78 ++++++++++++++++++++++---------------- > lib/PublicInbox/HTTP.pm | 9 +++-- > lib/PublicInbox/Syscall.pm | 29 +++++++++++++- > t/syscall.t | 8 +++- > 5 files changed, 87 insertions(+), 38 deletions(-) > > diff --git a/devel/sysdefs-list b/devel/sysdefs-list > index ba51de6c..1345e0ba 100755 > --- a/devel/sysdefs-list > +++ b/devel/sysdefs-list > @@ -152,6 +152,7 @@ int main(void) > > D(SYS_sendmsg); > D(SYS_recvmsg); > + D(SYS_writev); > > STRUCT_BEGIN(struct flock); > PR_NUM(l_start); > diff --git a/lib/PublicInbox/DS.pm b/lib/PublicInbox/DS.pm > index c5f44860..f807c626 100644 > --- a/lib/PublicInbox/DS.pm > +++ b/lib/PublicInbox/DS.pm > @@ -38,6 +38,7 @@ use Carp qw(carp croak); > use List::Util qw(sum); > our @EXPORT_OK = qw(now msg_more awaitpid add_timer add_uniq_timer); > my $sendmsg_more = PublicInbox::Syscall->can('sendmsg_more'); > +my $writev = PublicInbox::Syscall->can('writev'); > > my $nextq; # queue for next_tick > my $reap_armed; > @@ -551,41 +552,54 @@ sub write { > } > } > > +sub _iov_write ($$@) { > + my ($self, $cb) = (shift, shift); > + my ($tip, $tmpio, $s, $exp); > + $s = $cb->($self->{sock}, @_); > + if (defined $s) { > + $exp = sum(map length, @_); > + return 1 if $s == $exp; > + while (@_) { > + $tip = shift; > + if ($s >= length($tip)) { # fully written > + $s -= length($tip); > + } else { # first partial write > + $tmpio = tmpio $self, \$tip, $s, @_ or return 0; > + last; > + } > + } > + $tmpio // return drop $self, "BUG: tmpio on $s != $exp"; > + } elsif ($! == EAGAIN) { > + $tip = shift; > + $tmpio = tmpio $self, \$tip, 0, @_ or return 0; > + } else { # client disconnected > + return $self->close; > + } > + push @{$self->{wbuf}}, $tmpio; # autovivifies > + epwait $self->{sock}, EPOLLOUT|EPOLLONESHOT; > + 0; > +} > + > sub msg_more ($@) { > my $self = shift; > - my ($sock, $wbuf); > - $sock = $self->{sock} or return 1; > - $wbuf = $self->{wbuf}; > - > + my $sock = $self->{sock} or return 1; > + my $wbuf = $self->{wbuf}; > if ($sendmsg_more && (!defined($wbuf) || !scalar(@$wbuf)) && > - !$sock->can('stop_SSL')) { > - my ($s, $tip, $tmpio); > - $s = $sendmsg_more->($sock, @_); > - if (defined $s) { > - my $exp = sum(map length, @_); > - return 1 if $s == $exp; > - while (@_) { > - $tip = shift; > - if ($s >= length($tip)) { # fully written > - $s -= length($tip); > - } else { # first partial write > - $tmpio = tmpio $self, \$tip, $s, @_ > - or return 0; > - last; > - } > - } > - $tmpio // return drop $self, "BUG: tmpio on $s != $exp"; > - } elsif ($! == EAGAIN) { > - $tip = shift; > - $tmpio = tmpio $self, \$tip, 0, @_ or return 0; > - } else { # client disconnected > - return $self->close; > - } > - push @{$self->{wbuf}}, $tmpio; # autovivifies > - epwait $sock, EPOLLOUT|EPOLLONESHOT; > - 0; > - } else { > - # don't redispatch into NNTPdeflate::write > + !$sock->can('stop_SSL')) { > + _iov_write $self, $sendmsg_more, @_; > + } else { # don't redispatch into NNTPdeflate::write > + PublicInbox::DS::write($self, join('', @_)); > + } > +} > + > +sub writev ($@) { > + my $self = shift; > + my $sock = $self->{sock} or return 1; > + my $wbuf = $self->{wbuf}; > + if ($writev && (!defined($wbuf) || !scalar(@$wbuf)) && > + !$sock->can('stop_SSL')) { > + _iov_write $self, $writev, @_; > + } else { # don't redispatch into NNTPdeflate::write > PublicInbox::DS::write($self, join('', @_)); > } > } > diff --git a/lib/PublicInbox/HTTP.pm b/lib/PublicInbox/HTTP.pm > index d0e5aa29..73632aee 100644 > --- a/lib/PublicInbox/HTTP.pm > +++ b/lib/PublicInbox/HTTP.pm > @@ -190,9 +190,10 @@ sub response_header_write ($$$) { > my $conn = $env->{HTTP_CONNECTION} || ''; > my $term = defined($len) || $chunked; > my $prot_persist = ($proto eq 'HTTP/1.1') && ($conn !~ /\bclose\b/i); > - my $alive; > + my ($alive, $res_body); > if (!$term && ref($res->[2]) eq 'ARRAY') { > - $len = sum0(map length, @{$res->[2]}); > + ($res_body, $res->[2]) = ($res->[2], []); > + $len = sum0(map length, @$res_body); > $h .= "Content-Length: $len\r\n"; > $term = 1; > } > @@ -210,7 +211,9 @@ sub response_header_write ($$$) { > } > $h .= 'Date: ' . http_date() . "\r\n\r\n"; > > - if (($len || $chunked) && $env->{REQUEST_METHOD} ne 'HEAD') { > + if ($res_body) { > + $self->writev($h, @$res_body); > + } elsif (($len || $chunked) && $env->{REQUEST_METHOD} ne 'HEAD') { > msg_more($self, $h); > } else { > $self->write(\$h); > diff --git a/lib/PublicInbox/Syscall.pm b/lib/PublicInbox/Syscall.pm > index 80b46c19..ebcedb89 100644 > --- a/lib/PublicInbox/Syscall.pm > +++ b/lib/PublicInbox/Syscall.pm > @@ -61,7 +61,7 @@ our ($SYS_epoll_create, > $SYS_recvmsg); > > my $SYS_fstatfs; # don't need fstatfs64, just statfs.f_type > -my ($FS_IOC_GETFLAGS, $FS_IOC_SETFLAGS); > +my ($FS_IOC_GETFLAGS, $FS_IOC_SETFLAGS, $SYS_writev); > my $SFD_CLOEXEC = 02000000; # Perl does not expose O_CLOEXEC > our $no_deprecated = 0; > > @@ -95,6 +95,7 @@ if ($^O eq "linux") { > $SYS_fstatfs = 100; > $SYS_sendmsg = 370; > $SYS_recvmsg = 372; > + $SYS_writev = 146; > $INOTIFY = { # usage: `use constant $PublicInbox::Syscall::INOTIFY' > SYS_inotify_init1 => 332, > SYS_inotify_add_watch => 292, > @@ -111,6 +112,7 @@ if ($^O eq "linux") { > $SYS_fstatfs = 138; > $SYS_sendmsg = 46; > $SYS_recvmsg = 47; > + $SYS_writev = 20; > $INOTIFY = { > SYS_inotify_init1 => 294, > SYS_inotify_add_watch => 254, > @@ -127,6 +129,7 @@ if ($^O eq "linux") { > $SYS_fstatfs = 138; > $SYS_sendmsg = 0x40000206; > $SYS_recvmsg = 0x40000207; > + $SYS_writev = 0x40000204; > $FS_IOC_GETFLAGS = 0x80046601; > $FS_IOC_SETFLAGS = 0x40046602; > $INOTIFY = { > @@ -164,6 +167,7 @@ if ($^O eq "linux") { > $SYS_fstatfs = 100; > $SYS_sendmsg = 341; > $SYS_recvmsg = 342; > + $SYS_writev = 146; > $FS_IOC_GETFLAGS = 0x40086601; > $FS_IOC_SETFLAGS = 0x80086602; > $INOTIFY = { > @@ -179,6 +183,7 @@ if ($^O eq "linux") { > $SYS_signalfd4 = 313; > $SYS_renameat2 //= 357; > $SYS_fstatfs = 100; > + $SYS_writev = 146; > $FS_IOC_GETFLAGS = 0x40086601; > $FS_IOC_SETFLAGS = 0x80086602; > } elsif ($machine =~ m/^s390/) { # untested, no machine on cfarm > @@ -191,6 +196,7 @@ if ($^O eq "linux") { > $SYS_fstatfs = 100; > $SYS_sendmsg = 370; > $SYS_recvmsg = 372; > + $SYS_writev = 146; > } elsif ($machine eq 'ia64') { # untested, no machine on cfarm > $SYS_epoll_create = 1243; > $SYS_epoll_ctl = 1244; > @@ -216,6 +222,7 @@ if ($^O eq "linux") { > $SYS_fstatfs = 44; > $SYS_sendmsg = 211; > $SYS_recvmsg = 212; > + $SYS_writev = 66; > $INOTIFY = { > SYS_inotify_init1 => 26, > SYS_inotify_add_watch => 27, > @@ -233,6 +240,7 @@ if ($^O eq "linux") { > $SYS_fstatfs = 100; > $SYS_sendmsg = 296; > $SYS_recvmsg = 297; > + $SYS_writev = 146; > } elsif ($machine =~ m/^mips64/) { # cfarm only has 32-bit userspace > $SYS_epoll_create = 5207; > $SYS_epoll_ctl = 5208; > @@ -243,6 +251,7 @@ if ($^O eq "linux") { > $SYS_fstatfs = 5135; > $SYS_sendmsg = 5045; > $SYS_recvmsg = 5046; > + $SYS_writev = 5019; > $FS_IOC_GETFLAGS = 0x40046601; > $FS_IOC_SETFLAGS = 0x80046602; > } elsif ($machine =~ m/^mips/) { # 32-bit, tested on mips64 cfarm host > @@ -255,6 +264,7 @@ if ($^O eq "linux") { > $SYS_fstatfs = 4100; > $SYS_sendmsg = 4179; > $SYS_recvmsg = 4177; > + $SYS_writev = 4146; > $FS_IOC_GETFLAGS = 0x40046601; > $FS_IOC_SETFLAGS = 0x80046602; > $SIGNUM{WINCH} = 20; > @@ -287,6 +297,7 @@ EOM > # (I'm assuming Dragonfly copies FreeBSD, here, too) > $SYS_recvmsg = 27; > $SYS_sendmsg = 28; > + $SYS_writev = 121; > } > > BEGIN { > @@ -466,8 +477,9 @@ sub CMSG_LEN ($) { CMSG_ALIGN_SIZEOF_cmsghdr + $_[0] } > use constant msg_controllen_max => > CMSG_SPACE(10 * SIZEOF_int) + SIZEOF_cmsghdr; # space for 10 FDs > > -if (defined($SYS_sendmsg) && defined($SYS_recvmsg)) { > no warnings 'once'; > + > +if (defined($SYS_sendmsg) && defined($SYS_recvmsg)) { > require PublicInbox::CmdIPC4; > > *send_cmd4 = sub ($$$$;$) { > @@ -548,6 +560,19 @@ require PublicInbox::CmdIPC4; > }; > } > > +if (defined($SYS_writev)) { > +*writev = sub { > + my $fh = shift; > + use bytes qw(length substr); > + my $iov = join('', map { pack 'P'.TMPL_size_t, $_, length } @_); > + my $w; > + do { > + $w = syscall($SYS_writev, fileno($fh), $iov, scalar(@_)); > + } while ($w < 0 && $!{EINTR}); > + $w < 0 ? undef : $w; > +}; > +} > + > 1; > > =head1 WARRANTY > diff --git a/t/syscall.t b/t/syscall.t > index 19390d09..8f0e9efa 100644 > --- a/t/syscall.t > +++ b/t/syscall.t > @@ -4,11 +4,17 @@ use Test::More; > use PublicInbox::Syscall; > use Socket qw(AF_UNIX SOCK_STREAM); > my $sendmsg_more = PublicInbox::Syscall->can('sendmsg_more') or > - plan skip_all => "sendmsg syscalls not defined on $^O"; > + plan skip_all => "sendmsg syscall not defined on $^O"; > +my $writev = PublicInbox::Syscall->can('writev') or > + plan skip_all => "writev syscall not defined on $^O"; > > socketpair(my $s1, my $s2, AF_UNIX, SOCK_STREAM, 0); > is $sendmsg_more->($s1, 'hello', 'world'), 10, 'sendmsg_more expected size'; > is sysread($s2, my $buf, 11), 10, 'reader got expected size from sendmsg_more'; > is $buf, 'helloworld', 'sendmsg_more sent expected message'; > > +is $writev->($s1, 'hello', 'world'), 10, 'writev expected size'; > +is sysread($s2, $buf, 11), 10, 'reader got expected size from writev'; > +is $buf, 'helloworld', 'writev sent expected message'; > + > done_testing;