From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: X-Spam-Status: No, score=-4.0 required=3.0 tests=ALL_TRUSTED,BAYES_00 shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from localhost (dcvr.yhbt.net [127.0.0.1]) by dcvr.yhbt.net (Postfix) with ESMTP id F1D241F463 for ; Sun, 5 Jan 2020 09:51:16 +0000 (UTC) From: Eric Wong To: meta@public-inbox.org Subject: [PATCH] view: msg_html: reduce memory use on reused MIDs Date: Sun, 5 Jan 2020 09:51:16 +0000 Message-Id: <20200105095116.9748-1-e@80x24.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit List-Id: 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. --- MANIFEST | 1 + lib/PublicInbox/TestCommon.pm | 22 ++++++---- lib/PublicInbox/View.pm | 11 ++--- xt/mem-msgview.t | 80 +++++++++++++++++++++++++++++++++++ 4 files changed, 101 insertions(+), 13 deletions(-) create mode 100644 xt/mem-msgview.t diff --git a/MANIFEST b/MANIFEST index 845fee7b..914015ad 100644 --- 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 532cbee6..5bac3e79 100644 --- a/lib/PublicInbox/TestCommon.pm +++ b/lib/PublicInbox/TestCommon.pm @@ -191,14 +191,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 c38a1289..69948c4a 100644 --- 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) . - '
' + multipart_text_as_html(delete $ctx->{mime}, $upfx, + $ctx) . '
' } 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}); '
' . html_footer($hdr, 1, $ctx) .
 			'
' . 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) . '
' diff --git a/xt/mem-msgview.t b/xt/mem-msgview.t new file mode 100644 index 00000000..1ea0f559 --- /dev/null +++ b/xt/mem-msgview.t @@ -0,0 +1,80 @@ +#!perl -w +# Copyright (C) 2020 all contributors +# License: AGPL-3.0+ +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 < +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();