]> Sergey Matveev's repositories - public-inbox.git/commitdiff
Merge remote-tracking branch 'origin/email-simple-mem' into master
authorEric Wong <e@80x24.org>
Sun, 30 Jun 2019 17:13:30 +0000 (17:13 +0000)
committerEric Wong <e@80x24.org>
Sun, 30 Jun 2019 17:13:30 +0000 (17:13 +0000)
* 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

1  2 
lib/PublicInbox/NNTP.pm

diff --combined lib/PublicInbox/NNTP.pm
index 82762b1a466e2cf392aae9dd6f46fa54868bb633,5a886a3c32be100b0d24f393b8792846d0fcfd24..26bc679f2996b67e0008b8f90bfb1a1d9732521f
@@@ -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;
                        }
                }
        }
        $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 ($$) {