test repo for public-inbox, cleared periodically
 help / color / mirror / Atom feed
* 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

only message in thread, other threads:[~2024-06-20 22:39 UTC | newest]

Thread overview: (only message) (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20240619234104.80183-1-e@80x24.org>
     [not found] ` <20240619234104.80183-6-e@80x24.org>
2024-06-20 22:39   ` [PATCH 5/5] http: use writev for known Content-Length responses Eric Wong

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).