view: msg_html: reduce memory use on reused MIDs

In rare cases where Message-IDs get reused, we do not want to
hold onto the large Email::MIME objects in memory after showing
the first message.  So discard each message as soon as we're
done using it so we can save memory for the next message.

The new and expensive xt/mem-msgview.t test shows a nearly 14MB
reduction for two ~7MB messages.  run_script() also gets
upgraded to make it easier to pass large inputs via IO GLOBs.

diff --git a/MANIFEST b/MANIFEST
index 845fee7b486f80fab935f60e1aa3fb57a2479640..914015adf64eba929de863a0f9115e7461f3c0aa 100644 (file)
--- a/MANIFEST
+++ b/MANIFEST
@@ -296,6 +296,7 @@ t/www_static.t
 t/xcpdb-reshard.t
 xt/git-http-backend.t
 xt/git_async_cmp.t
+xt/mem-msgview.t
 xt/msgtime_cmp.t
 xt/nntpd-validate.t
 xt/perf-msgview.t

diff --git a/lib/PublicInbox/TestCommon.pm b/lib/PublicInbox/TestCommon.pm
index 68785969fc8a27c939c9b74e74ee159181bb73ed..d6d1e9394937c1ea652ccb06b52b41d82cc06470 100644 (file)
--- a/lib/PublicInbox/TestCommon.pm
+++ b/lib/PublicInbox/TestCommon.pm
@@ -195,14 +195,20 @@ sub run_script ($;$$) {
        my $spawn_opt = {};
        for my $fd (0..2) {
                my $redir = $opt->{$fd};
-               next unless ref($redir);
-               open my $fh, '+>', undef or die "open: $!";
-               $fhref->[$fd] = $fh;
-               $spawn_opt->{$fd} = $fh;
-               next if $fd > 0;
-               $fh->autoflush(1);
-               print $fh $$redir or die "print: $!";
-               seek($fh, 0, SEEK_SET) or die "seek: $!";
+               my $ref = ref($redir);
+               if ($ref eq 'SCALAR') {
+                       open my $fh, '+>', undef or die "open: $!";
+                       $fhref->[$fd] = $fh;
+                       $spawn_opt->{$fd} = $fh;
+                       next if $fd > 0;
+                       $fh->autoflush(1);
+                       print $fh $$redir or die "print: $!";
+                       seek($fh, 0, SEEK_SET) or die "seek: $!";
+               } elsif ($ref eq 'GLOB') {
+                       $spawn_opt->{$fd} = $fhref->[$fd] = $redir;
+               } elsif ($ref) {
+                       die "unable to deal with $ref $redir";
+               }
        }
        if ($run_mode == 0) {
                # spawn an independent new process, like real-world use cases:

diff --git a/lib/PublicInbox/View.pm b/lib/PublicInbox/View.pm
index c38a12894b21dbc8fccd0d9f7f747c57f977ac8f..69948c4a07cea5da391241571df9a93b37748b05 100644 (file)
--- a/lib/PublicInbox/View.pm
+++ b/lib/PublicInbox/View.pm
@@ -31,8 +31,8 @@ sub msg_html_i {
                # $more cannot be true w/o $smsg being defined:
                my $upfx = $more ? '../'.mid_escape($ctx->{smsg}->mid).'/' : '';
                $ctx->{tip} .
-                       multipart_text_as_html($ctx->{mime}, $upfx, $ctx) .
-                       '</pre><hr>'
+                       multipart_text_as_html(delete $ctx->{mime}, $upfx,
+                                               $ctx) . '</pre><hr>'
        } elsif ($more && @$more) {
                ++$ctx->{end_nr};
                msg_html_more($ctx, $more, $nr);
@@ -41,7 +41,7 @@ sub msg_html_i {
                # we want to at least show the message if something
                # here crashes:
                eval {
-                       my $hdr = delete($ctx->{mime})->header_obj;
+                       my $hdr = delete($ctx->{hdr});
                        '<pre>' . html_footer($hdr, 1, $ctx) .
                        '</pre>' . msg_reply($ctx, $hdr)
                };
@@ -56,7 +56,8 @@ sub msg_html {
        my ($ctx, $mime, $more, $smsg) = @_;
        my $ibx = $ctx->{-inbox};
        $ctx->{-obfs_ibx} = $ibx->{obfuscate} ? $ibx : undef;
-       $ctx->{tip} = _msg_html_prepare($mime->header_obj, $ctx, $more, 0);
+       my $hdr = $ctx->{hdr} = $mime->header_obj;
+       $ctx->{tip} = _msg_html_prepare($hdr, $ctx, $more, 0);
        $ctx->{more} = $more;
        $ctx->{end_nr} = 2;
        $ctx->{smsg} = $smsg;
@@ -95,8 +96,8 @@ sub msg_html_more {
                my $next = $ibx->over->next_by_mid($mid, \$id, \$prev);
                @$more = $next ? ($id, $prev, $next) : ();
                if ($smsg) {
-                       my $mime = $smsg->{mime};
                        my $upfx = '../' . mid_escape($smsg->mid) . '/';
+                       my $mime = delete $smsg->{mime};
                        _msg_html_prepare($mime->header_obj, $ctx, $more, $nr) .
                                multipart_text_as_html($mime, $upfx, $ctx) .
                                '</pre><hr>'

diff --git a/xt/mem-msgview.t b/xt/mem-msgview.t
new file mode 100644 (file)
index 0000000..1ea0f55
--- /dev/null
+++ b/xt/mem-msgview.t
@@ -0,0 +1,80 @@
+#!perl -w
+# Copyright (C) 2020 all contributors <meta@public-inbox.org>
+# License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
+use strict;
+use IO::Handle; # ->flush
+use Fcntl qw(SEEK_SET);
+use PublicInbox::TestCommon;
+use PublicInbox::Tmpfile;
+use PublicInbox::WWW;
+use Test::More;
+my @mods = qw(DBD::SQLite BSD::Resource);
+require_mods(@mods);
+use_ok($_) for @mods;
+my $lines = $ENV{NR_LINES} // 100000;
+my ($tmpdir, $for_destroy) = tmpdir();
+my $inboxname = 'big';
+my $inboxdir = "$tmpdir/big";
+local $ENV{PI_CONFIG} = "$tmpdir/cfg";
+my $mid = 'test@example.com';
+
+{ # setup
+       open my $fh, '>', "$tmpdir/cfg" or die;
+       print $fh <<EOF or die;
+[publicinboxmda]
+       spamcheck = none
+EOF
+       close $fh or die;
+
+       my $addr = 'n@example.com';
+       ok(run_script([qw(-init -V2 --indexlevel=basic), $inboxname, $inboxdir,
+                       "http://example.com/$inboxname", $addr]),
+               'inbox initialized');
+
+       $fh = tmpfile('big.eml', undef, my $append = 1) or die;
+       printf($fh <<'EOF', $addr, $mid) or die;
+From: Dr. X <x@example.com>
+To: Nikki <%s>
+Date: Tue, 3 May 1988 00:00:00 +0000
+Subject: todo
+Message-ID: <%s>
+
+EOF
+       for (0..$lines) { print $fh 'x' x 72, "\n" or die }
+       $fh->flush or die;
+       sysseek($fh, 0, SEEK_SET) or die;
+       my $env = { ORIGINAL_RECIPIENT => $addr };
+       my $err = '';
+       my $opt = { 0 => $fh, 2 => \$err, run_mode => 0 };
+       ok(run_script([qw(-mda --no-precheck)], $env, $opt),
+               '1st message delivered');
+
+       # resend the message with same mid but different content
+       print $fh "mindcrime\n" or die;
+       $fh->flush or die;
+       sysseek($fh, 0, SEEK_SET) or die;
+       ok(run_script([qw(-mda --no-precheck)], $env, $opt),
+               '2nd message delivered');
+}
+
+my $www = PublicInbox::WWW->new;
+my $env = {
+       PATH_INFO => "/$inboxname/$mid/",
+       REQUEST_URI => "/$inboxname/$mid/",
+       SCRIPT_NAME => '',
+       QUERY_STRING => '',
+       REQUEST_METHOD => 'GET',
+       HTTP_HOST => 'example.com',
+       'psgi.errors' => \*STDERR,
+       'psgi.url_scheme' => 'http',
+};
+my $ru_before = BSD::Resource::getrusage();
+my $res = $www->call($env);
+my $body = $res->[2];
+while (defined(my $x = $body->getline)) {
+}
+$body->close;
+my $ru_after = BSD::Resource::getrusage();
+my $diff = $ru_after->maxrss - $ru_before->maxrss;
+diag "before: ${\$ru_before->maxrss} => ${\$ru_after->maxrss} diff=$diff kB";
+done_testing();