From: Eric Wong <e@80x24.org>
Date: Sun, 30 Jun 2019 17:13:30 +0000 (+0000)
Subject: Merge remote-tracking branch 'origin/email-simple-mem' into master
X-Git-Tag: v1.2.0~135
X-Git-Url: http://www.git.stargrave.org/?p=public-inbox.git;a=commitdiff_plain;h=ecea327e3d4386a22652fc08f71ac7d65b8f9b70;hp=-c

Merge remote-tracking branch 'origin/email-simple-mem' into master

* origin/email-simple-mem:
  nntp: reduce syscalls for ARTICLE and BODY
  mbox: split header and body processing
  mbox: use Email::Simple->new to do in-place modifications
  nntp: rework and simplify art_lookup response
---

ecea327e3d4386a22652fc08f71ac7d65b8f9b70
diff --combined lib/PublicInbox/NNTP.pm
index 82762b1a,5a886a3c..26bc679f
--- a/lib/PublicInbox/NNTP.pm
+++ b/lib/PublicInbox/NNTP.pm
@@@ -6,7 -6,7 +6,7 @@@ package PublicInbox::NNTP
  use strict;
  use warnings;
  use base qw(PublicInbox::DS);
 -use fields qw(nntpd article rbuf ng);
 +use fields qw(nntpd article ng);
  use PublicInbox::Search;
  use PublicInbox::Msgmap;
  use PublicInbox::MID qw(mid_escape);
@@@ -38,6 -38,20 +38,6 @@@ my %DISABLED; # = map { $_ => 1 } qw(xo
  my $EXPMAP; # fd -> [ idle_time, $self ]
  my $expt;
  our $EXPTIME = 180; # 3 minutes
 -my $nextt;
 -
 -my $nextq = [];
 -sub next_tick () {
 -	$nextt = undef;
 -	my $q = $nextq;
 -	$nextq = [];
 -	event_step($_) for @$q;
 -}
 -
 -sub requeue ($) {
 -	push @$nextq, $_[0];
 -	$nextt ||= PublicInbox::EvCleanup::asap(*next_tick);
 -}
  
  sub update_idle_time ($) {
  	my ($self) = @_;
@@@ -50,11 -64,14 +50,11 @@@ sub expire_old () 
  	my $exp = $EXPTIME;
  	my $old = $now - $exp;
  	my $nr = 0;
 -	my $closed = 0;
  	my %new;
  	while (my ($fd, $v) = each %$EXPMAP) {
  		my ($idle_time, $nntp) = @$v;
  		if ($idle_time < $old) {
 -			if ($nntp->shutdn) {
 -				$closed++;
 -			} else {
 +			if (!$nntp->shutdn) {
  				++$nr;
  				$new{$fd} = $v;
  			}
@@@ -64,7 -81,14 +64,7 @@@
  		}
  	}
  	$EXPMAP = \%new;
 -	if ($nr) {
 -		$expt = PublicInbox::EvCleanup::later(*expire_old);
 -	} else {
 -		$expt = undef;
 -		# noop to kick outselves out of the loop ASAP so descriptors
 -		# really get closed
 -		PublicInbox::EvCleanup::asap(sub {}) if $closed;
 -	}
 +	$expt = PublicInbox::EvCleanup::later(*expire_old) if $nr;
  }
  
  sub greet ($) { $_[0]->write($_[0]->{nntpd}->{greet}) };
@@@ -75,8 -99,7 +75,8 @@@ sub new ($$$) 
  	my $ev = EPOLLIN;
  	my $wbuf;
  	if (ref($sock) eq 'IO::Socket::SSL' && !$sock->accept_SSL) {
 -		$ev = PublicInbox::TLS::epollbit() or return CORE::close($sock);
 +		return CORE::close($sock) if $! != EAGAIN;
 +		$ev = PublicInbox::TLS::epollbit();
  		$wbuf = [ \&PublicInbox::DS::accept_tls_step, \&greet ];
  	}
  	$self->SUPER::new($sock, $ev | EPOLLONESHOT);
@@@ -487,24 -510,23 +487,23 @@@ find_mid
  found:
  	my $smsg = $ng->over->get_art($n) or return $err;
  	my $msg = $ng->msg_by_smsg($smsg) or return $err;
- 	my $s = Email::Simple->new($msg);
- 	if ($set_headers) {
- 		set_nntp_headers($self, $s->header_obj, $ng, $n, $mid);
  
- 		# must be last
- 		$s->body_set('') if ($set_headers == 2);
- 	}
- 	[ $n, $mid, $s, $smsg->bytes, $smsg->lines, $ng ];
+ 	# Email::Simple->new will modify $msg in-place as documented
+ 	# in its manpage, so what's left is the body and we won't need
+ 	# to call Email::Simple::body(), later
+ 	my $hdr = Email::Simple->new($msg)->header_obj;
+ 	set_nntp_headers($self, $hdr, $ng, $n, $mid) if $set_headers;
+ 	[ $n, $mid, $msg, $hdr ];
  }
  
- sub simple_body_write ($$) {
- 	my ($self, $s) = @_;
- 	my $body = $s->body;
- 	$s->body_set('');
- 	$body =~ s/^\./../smg;
- 	$body =~ s/(?<!\r)\n/\r\n/sg;
- 	msg_more($self, $body);
- 	msg_more($self, "\r\n") unless $body =~ /\r\n\z/s;
+ sub msg_body_write ($$) {
+ 	my ($self, $msg) = @_;
+ 
+ 	# these can momentarily double the memory consumption :<
+ 	$$msg =~ s/^\./../smg;
+ 	$$msg =~ s/(?<!\r)\n/\r\n/sg; # Alpine barfs without this
+ 	$$msg .= "\r\n" unless $$msg =~ /\r\n\z/s;
+ 	msg_more($self, $$msg);
  	'.'
  }
  
@@@ -513,40 -535,40 +512,40 @@@ sub set_art 
  	$self->{article} = $art if defined $art && $art =~ /\A[0-9]+\z/;
  }
  
- sub _header ($) {
- 	my $hdr = $_[0]->header_obj->as_string;
+ sub msg_hdr_write ($$$) {
+ 	my ($self, $hdr, $body_follows) = @_;
+ 	$hdr = $hdr->as_string;
  	utf8::encode($hdr);
- 	$hdr =~ s/(?<!\r)\n/\r\n/sg;
+ 	$hdr =~ s/(?<!\r)\n/\r\n/sg; # Alpine barfs without this
  
  	# for leafnode compatibility, we need to ensure Message-ID headers
  	# are only a single line.  We can't subclass Email::Simple::Header
  	# and override _default_fold_at in here, either; since that won't
  	# affect messages already in the archive.
  	$hdr =~ s/^(Message-ID:)[ \t]*\r\n[ \t]+([^\r]+)\r\n/$1 $2\r\n/igsm;
- 
- 	$hdr
+ 	$hdr .= "\r\n" if $body_follows;
+ 	msg_more($self, $hdr);
  }
  
  sub cmd_article ($;$) {
  	my ($self, $art) = @_;
  	my $r = art_lookup($self, $art, 1);
  	return $r unless ref $r;
- 	my ($n, $mid, $s) = @$r;
+ 	my ($n, $mid, $msg, $hdr) = @$r;
  	set_art($self, $art);
  	more($self, "220 $n <$mid> article retrieved - head and body follow");
- 	msg_more($self, _header($s));
- 	msg_more($self, "\r\n");
- 	simple_body_write($self, $s);
+ 	msg_hdr_write($self, $hdr, 1);
+ 	msg_body_write($self, $msg);
  }
  
  sub cmd_head ($;$) {
  	my ($self, $art) = @_;
  	my $r = art_lookup($self, $art, 2);
  	return $r unless ref $r;
- 	my ($n, $mid, $s) = @$r;
+ 	my ($n, $mid, undef, $hdr) = @$r;
  	set_art($self, $art);
  	more($self, "221 $n <$mid> article retrieved - head follows");
- 	msg_more($self, _header($s));
+ 	msg_hdr_write($self, $hdr, 0);
  	'.'
  }
  
@@@ -554,17 -576,17 +553,17 @@@ sub cmd_body ($;$) 
  	my ($self, $art) = @_;
  	my $r = art_lookup($self, $art, 0);
  	return $r unless ref $r;
- 	my ($n, $mid, $s) = @$r;
+ 	my ($n, $mid, $msg) = @$r;
  	set_art($self, $art);
  	more($self, "222 $n <$mid> article retrieved - body follows");
- 	simple_body_write($self, $s);
+ 	msg_body_write($self, $msg);
  }
  
  sub cmd_stat ($;$) {
  	my ($self, $art) = @_;
  	my $r = art_lookup($self, $art, 0);
  	return $r unless ref $r;
- 	my ($n, $mid, undef) = @$r;
+ 	my ($n, $mid) = @$r;
  	set_art($self, $art);
  	"223 $n <$mid> article retrieved - request text separately";
  }
@@@ -632,12 -654,12 +631,12 @@@ sub long_response ($$) 
  			push @$wbuf, $long_cb;
  
  			# wbuf may be populated by $cb, no need to rearm if so:
 -			requeue($self) if scalar(@$wbuf) == 1;
 +			$self->requeue if scalar(@$wbuf) == 1;
  		} else { # all done!
  			$long_cb = undef;
  			res($self, '.');
  			out($self, " deferred[$fd] done - %0.6f", now() - $t0);
 -			requeue($self) unless $self->{wbuf};
 +			$self->requeue unless $self->{wbuf};
  		}
  	};
  	$self->write($long_cb); # kick off!
@@@ -792,7 -814,7 +791,7 @@@ sub hdr_mid_prefix ($$$$$) 
  }
  
  sub hdr_mid_response ($$$$$$) {
- 	my ($self, $xhdr, $ng, $n, $mid, $v) = @_; # r: art_lookup result
+ 	my ($self, $xhdr, $ng, $n, $mid, $v) = @_;
  	my $res = '';
  	if ($xhdr) {
  		$res .= r221 . "\r\n";
@@@ -892,7 -914,7 +891,7 @@@ sub cmd_starttls ($) 
  		return '580 can not initiate TLS negotiation';
  	res($self, '382 Continue with TLS negotiation');
  	$self->{sock} = IO::Socket::SSL->start_SSL($sock, %$opt);
 -	requeue($self) if PublicInbox::DS::accept_tls_step($self);
 +	$self->requeue if PublicInbox::DS::accept_tls_step($self);
  	undef;
  }
  
@@@ -962,12 -984,16 +961,12 @@@ sub event_step 
  	return $self->close if $r < 0;
  	my $len = bytes::length($$rbuf);
  	return $self->close if ($len >= LINE_MAX);
 -	if ($len) {
 -		$self->{rbuf} = $rbuf;
 -	} else {
 -		delete $self->{rbuf};
 -	}
 +	$self->rbuf_idle($rbuf);
  	update_idle_time($self);
  
  	# maybe there's more pipelined data, or we'll have
  	# to register it for socket-readiness notifications
 -	requeue($self) unless $self->{wbuf};
 +	$self->requeue unless $self->{wbuf};
  }
  
  sub not_idle_long ($$) {