* Re: [PATCH 5/5] http: use writev for known Content-Length responses
[not found] ` <20240619234104.80183-6-e@80x24.org>
@ 2024-06-20 22:39 ` Eric Wong
0 siblings, 0 replies; only message in thread
From: Eric Wong @ 2024-06-20 22:39 UTC (permalink / raw)
To: test
Eric Wong <e@80x24.org> 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;
^ permalink raw reply [flat|nested] only message in thread