From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <e@yhbt.net>
X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net
X-Spam-Level: 
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 622131F919
	for <meta@public-inbox.org>; Sun,  5 Jul 2020 23:28:10 +0000 (UTC)
From: Eric Wong <e@yhbt.net>
To: meta@public-inbox.org
Subject: [PATCH 23/43] wwwstream: reduce blob fetch paths for ->getline
Date: Sun,  5 Jul 2020 23:27:39 +0000
Message-Id: <20200705232759.3161-24-e@yhbt.net>
In-Reply-To: <20200705232759.3161-1-e@yhbt.net>
References: <20200705232759.3161-1-e@yhbt.net>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
List-Id: <meta.public-inbox.org>

This will make it easier to support asynchronous blob
retrievals.  The `$ctx->{nr}' counter is no longer implicitly
supplied since many users didn't care for it, so stack overhead
is slightly reduced.
---
 Documentation/mknews.perl     |   4 +-
 lib/PublicInbox/Feed.pm       |   3 +-
 lib/PublicInbox/SearchView.pm |  28 +++---
 lib/PublicInbox/View.pm       | 181 +++++++++++++++++-----------------
 lib/PublicInbox/WwwStream.pm  |  19 ++--
 5 files changed, 113 insertions(+), 122 deletions(-)

diff --git a/Documentation/mknews.perl b/Documentation/mknews.perl
index 1bd704e68..51d54b716 100755
--- a/Documentation/mknews.perl
+++ b/Documentation/mknews.perl
@@ -37,7 +37,7 @@ if ($dst eq 'NEWS') {
 	my $ibx = My::MockObject->new(
 		description => 'public-inbox releases',
 		over => undef,
-		search => 1, # for WwwStream:_html_top
+		search => 1, # for WwwStream::html_top
 		base_url => "$base_url/",
 	);
 	$ibx->{-primary_address} = $addr;
@@ -113,7 +113,7 @@ sub html_start {
 	require PublicInbox::WwwStream;
 	$ctx->{www} = My::MockObject->new(style => '');
 	my $www_stream = PublicInbox::WwwStream::init($ctx);
-	print $out $www_stream->_html_top, '<pre>' or die;
+	print $out $www_stream->html_top, '<pre>' or die;
 }
 
 sub html_end {
diff --git a/lib/PublicInbox/Feed.pm b/lib/PublicInbox/Feed.pm
index f25dd267e..b15fc3a09 100644
--- a/lib/PublicInbox/Feed.pm
+++ b/lib/PublicInbox/Feed.pm
@@ -50,7 +50,8 @@ sub generate_html_index {
 }
 
 sub new_html_i {
-	my ($nr, $ctx) = @_;
+	my ($ctx) = @_;
+	return $ctx->html_top if exists $ctx->{-html_tip};
 	my $msgs = $ctx->{msgs};
 	while (my $smsg = shift @$msgs) {
 		my $eml = $ctx->{-inbox}->smsg_eml($smsg) or next;
diff --git a/lib/PublicInbox/SearchView.pm b/lib/PublicInbox/SearchView.pm
index 71c3ae707..eeebdfa31 100644
--- a/lib/PublicInbox/SearchView.pm
+++ b/lib/PublicInbox/SearchView.pm
@@ -10,12 +10,11 @@ use PublicInbox::Smsg;
 use PublicInbox::Hval qw(ascii_html obfuscate_addrs mid_href);
 use PublicInbox::View;
 use PublicInbox::WwwAtomStream;
+use PublicInbox::WwwStream qw(html_oneshot);
 use PublicInbox::SearchThread;
 our $LIM = 200;
 my %rmap_inc;
 
-my $noop = sub {};
-
 sub mbox_results {
 	my ($ctx) = @_;
 	my $q = PublicInbox::SearchQuery->new($ctx->{qp});
@@ -48,7 +47,7 @@ sub sres_top_html {
 		relevance => $q->{r},
 		asc => $asc,
 	};
-	my ($mset, $total, $err, $cb);
+	my ($mset, $total, $err, $html);
 retry:
 	eval {
 		$mset = $srch->query($query, $opts);
@@ -58,8 +57,7 @@ retry:
 	ctx_prepare($q, $ctx);
 	if ($err) {
 		$code = 400;
-		$ctx->{-html_tip} = '<pre>'.err_txt($ctx, $err).'</pre><hr>';
-		$cb = $noop;
+		$html = '<pre>'.err_txt($ctx, $err).'</pre><hr>';
 	} elsif ($total == 0) {
 		if (defined($ctx->{-uxs_retried})) {
 			# undo retry damage:
@@ -70,19 +68,16 @@ retry:
 			goto retry;
 		}
 		$code = 404;
-		$ctx->{-html_tip} = "<pre>\n[No results found]</pre><hr>";
-		$cb = $noop;
+		$html = "<pre>\n[No results found]</pre><hr>";
 	} else {
 		return adump($_[0], $mset, $q, $ctx) if $x eq 'A';
 
 		$ctx->{-html_tip} = search_nav_top($mset, $q, $ctx);
-		if ($x eq 't') {
-			$cb = mset_thread($ctx, $mset, $q);
-		} else {
-			$cb = mset_summary($ctx, $mset, $q);
-		}
+		return mset_thread($ctx, $mset, $q) if $x eq 't';
+		mset_summary($ctx, $mset, $q); # appends to {-html_tip}
+		$html = '';
 	}
-	PublicInbox::WwwStream::response($ctx, $code, $cb);
+	html_oneshot($ctx, $code);
 }
 
 # display non-nested search results similar to what users expect from
@@ -122,7 +117,7 @@ sub mset_summary {
 		$$res .= "$pfx  - by $f @ $date UTC [$pct%]\n\n";
 	}
 	$$res .= search_nav_bot($mset, $q);
-	$noop;
+	undef;
 }
 
 # shorten "/full/path/to/Foo/Bar.pm" to "Foo/Bar.pm" so error
@@ -292,12 +287,13 @@ sub mset_thread {
 
 	@$msgs = reverse @$msgs if $r;
 	$ctx->{msgs} = $msgs;
-	\&mset_thread_i;
+	PublicInbox::WwwStream::response($ctx, 200, \&mset_thread_i);
 }
 
 # callback for PublicInbox::WwwStream::getline
 sub mset_thread_i {
-	my ($nr, $ctx) = @_;
+	my ($ctx) = @_;
+	return $ctx->html_top if exists $ctx->{-html_tip};
 	my $msgs = $ctx->{msgs} or return;
 	while (my $smsg = pop @$msgs) {
 		my $eml = $ctx->{-inbox}->smsg_eml($smsg) or next;
diff --git a/lib/PublicInbox/View.pm b/lib/PublicInbox/View.pm
index 4d6f44e0b..243528263 100644
--- a/lib/PublicInbox/View.pm
+++ b/lib/PublicInbox/View.pm
@@ -27,60 +27,60 @@ use constant TCHILD => '` ';
 sub th_pfx ($) { $_[0] == 0 ? '' : TCHILD };
 
 sub msg_page_i {
-	my ($nr, $ctx) = @_;
-	if (my $more = delete $ctx->{more}) { # unlikely
-		# fake an EOF if $more retrieval fails;
-		eval { msg_page_more($ctx, $nr, @$more) };
-	} elsif (my $hdr = delete $ctx->{hdr}) {
-		# fake an EOF if generating the footer fails;
-		# we want to at least show the message if something
-		# here crashes:
-		eval { html_footer($ctx, $hdr) };
-	} else {
-		undef
+	my ($ctx) = @_;
+	my $cur = delete $ctx->{smsg} or return; # undef: done
+	my $nxt;
+	if (my $over = $ctx->{-inbox}->over) {
+		$nxt = $ctx->{smsg} = $over->next_by_mid(@{$ctx->{next_arg}});
 	}
+	$ctx->{mhref} = ($ctx->{nr} || $nxt) ?
+			"../${\mid_href($cur->{mid})}/" : '';
+	my $eml = $ctx->{-inbox}->smsg_eml($cur) or return;
+	my $hdr = $eml->header_obj;
+	my $obuf = $ctx->{obuf} = _msg_page_prepare_obuf($hdr, $ctx);
+	multipart_text_as_html($eml, $ctx);
+	delete $ctx->{obuf};
+	$$obuf .= '</pre><hr>';
+	# we want to at least show the message if something
+	# here crashes:
+	eval { $$obuf .= html_footer($ctx, $ctx->{first_hdr}) } if !$nxt;
+	$$obuf;
+}
+
+# /$INBOX/$MESSAGE_ID/ for unindexed v1 inboxes
+sub no_over_i {
+	my ($ctx) = @_;
+	my $eml = delete $ctx->{eml} or return;
+	my $hdr = $eml->header_obj;
+	$ctx->{mhref} = '';
+	my $obuf = $ctx->{obuf} = _msg_page_prepare_obuf($hdr, $ctx);
+	multipart_text_as_html($eml, $ctx);
+	delete $ctx->{obuf};
+	$$obuf .= '</pre><hr>';
+	eval { $$obuf .= html_footer($ctx, $hdr) };
+	$$obuf
+}
+
+sub no_over_html ($) {
+	my ($ctx) = @_;
+	my $bref = $ctx->{-inbox}->msg_by_mid($ctx->{mid}) or return; # 404
+	$ctx->{eml} = PublicInbox::Eml->new($bref);
+	PublicInbox::WwwStream::response($ctx, 200, \&no_over_i);
 }
 
 # public functions: (unstable)
 
 sub msg_page {
 	my ($ctx) = @_;
-	my $mid = $ctx->{mid};
 	my $ibx = $ctx->{-inbox};
-	my ($smsg, $first, $next);
-	if (my $over = $ibx->over) {
-		my ($id, $prev);
-		$smsg = $over->next_by_mid($mid, \$id, \$prev) or return;
-		$first = $ibx->msg_by_smsg($smsg) or return;
-		$next = $over->next_by_mid($mid, \$id, \$prev);
-		$ctx->{more} = [ $id, $prev, $next ] if $next;
-	} else {
-		$first = $ibx->msg_by_mid($mid) or return;
-	}
-	my $mime = PublicInbox::Eml->new($first);
 	$ctx->{-obfs_ibx} = $ibx->{obfuscate} ? $ibx : undef;
-	my $hdr = $ctx->{hdr} = $mime->header_obj;
-	$ctx->{obuf} = _msg_page_prepare_obuf($hdr, $ctx, 0);
-	$ctx->{smsg} = $smsg;
-	# $next cannot be true w/o $smsg being defined:
-	$ctx->{mhref} = $next ? '../'.mid_href($smsg->{mid}).'/' : '';
-	multipart_text_as_html($mime, $ctx);
-	$ctx->{-html_tip} = (${delete $ctx->{obuf}} .= '</pre><hr>');
+	my $over = $ibx->over or return no_over_html($ctx);
+	my ($id, $prev);
+	my $next_arg = $ctx->{next_arg} = [ $ctx->{mid}, \$id, \$prev ];
+	$ctx->{smsg} = $over->next_by_mid(@$next_arg) or return;
 	PublicInbox::WwwStream::response($ctx, 200, \&msg_page_i);
 }
 
-sub msg_page_more { # cold
-	my ($ctx, $nr, $id, $prev, $smsg) = @_;
-	my $ibx = $ctx->{-inbox};
-	my $next = $ibx->over->next_by_mid($ctx->{mid}, \$id, \$prev);
-	$ctx->{more} = [ $id, $prev, $next ] if $next;
-	my $eml = $ibx->smsg_eml($smsg) or return '';
-	$ctx->{mhref} = '../' . mid_href($smsg->{mid}) . '/';
-	$ctx->{obuf} = _msg_page_prepare_obuf($eml->header_obj, $ctx, $nr);
-	multipart_text_as_html($eml, $ctx);
-	${delete $ctx->{obuf}} .= '</pre><hr>';
-}
-
 # /$INBOX/$MESSAGE_ID/#R
 sub msg_reply ($$) {
 	my ($ctx, $hdr) = @_;
@@ -377,42 +377,40 @@ sub thread_eml_entry {
 	$beg . '<pre>' . eml_entry($ctx, $smsg, $eml, 0) . '</pre>' . $end;
 }
 
-sub stream_thread_i { # PublicInbox::WwwStream::getline callback
-	my ($nr, $ctx) = @_;
-	return unless exists($ctx->{skel});
-	my $q = $ctx->{-queue};
+sub next_in_queue ($;$) {
+	my ($q, $ghost_ok) = @_;
 	while (@$q) {
-		my $level = shift @$q;
-		my $node = shift @$q or next;
+		my ($level, $smsg) = splice(@$q, 0, 2);
 		my $cl = $level + 1;
-		unshift @$q, map { ($cl, $_) } @{$node->{children}};
-		if (my $eml = $ctx->{-inbox}->smsg_eml($node)) {
-			return thread_eml_entry($ctx, $level, $node, $eml);
-		} else {
-			return ghost_index_entry($ctx, $level, $node);
-		}
+		unshift @$q, map { ($cl, $_) } @{$smsg->{children}};
+		return ($level, $smsg) if $ghost_ok || exists($smsg->{blob});
 	}
-	join('', thread_adj_level($ctx, 0)) . ${delete $ctx->{skel}};
+	undef;
 }
 
-sub stream_thread ($$) {
-	my ($rootset, $ctx) = @_;
-	my $ibx = $ctx->{-inbox};
-	my @q = map { (0, $_) } @$rootset;
-	my ($smsg, $eml, $level);
-	while (@q) {
-		$level = shift @q;
-		$smsg = shift @q or next;
-		my $cl = $level + 1;
-		unshift @q, map { ($cl, $_) } @{$smsg->{children}};
-		$eml = $ibx->smsg_eml($smsg) and last;
+sub stream_thread_i { # PublicInbox::WwwStream::getline callback
+	my ($ctx) = @_;
+	return unless exists($ctx->{skel});
+	my $nr = $ctx->{nr}++;
+	my ($level, $smsg) = next_in_queue($ctx->{-queue}, $nr);
+
+	$smsg or return
+		join('', thread_adj_level($ctx, 0)) . ${delete $ctx->{skel}};
+
+	my $eml = $ctx->{-inbox}->smsg_eml($smsg) or return
+		ghost_index_entry($ctx, $level, $smsg);
+
+	if ($nr == 0) {
+		$ctx->{-title_html} = ascii_html($smsg->{subject});
+		$ctx->html_top . thread_eml_entry($ctx, $level, $smsg, $eml);
+	} else {
+		thread_eml_entry($ctx, $level, $smsg, $eml);
 	}
-	return missing_thread($ctx) unless $eml;
+}
 
-	$ctx->{-obfs_ibx} = $ibx->{obfuscate} ? $ibx : undef;
-	$ctx->{-title_html} = ascii_html($smsg->{subject});
-	$ctx->{-html_tip} = thread_eml_entry($ctx, $level, $smsg, $eml);
-	$ctx->{-queue} = \@q;
+sub stream_thread ($$) {
+	my ($rootset, $ctx) = @_;
+	$ctx->{-queue} = [ map { (0, $_) } @$rootset ];
 	PublicInbox::WwwStream::response($ctx, 200, \&stream_thread_i);
 }
 
@@ -451,22 +449,21 @@ sub thread_html {
 	return stream_thread($rootset, $ctx) unless $ctx->{flat};
 
 	# flat display: lazy load the full message from smsg
-	my ($smsg, $eml);
-	while ($smsg = shift @$msgs) {
-		$eml = $ibx->smsg_eml($smsg) and last;
-	}
-	return missing_thread($ctx) unless $smsg;
-	$ctx->{-title_html} = ascii_html($smsg->{subject});
-	$ctx->{-html_tip} = '<pre>'.eml_entry($ctx, $smsg, $eml, scalar @$msgs);
 	$ctx->{msgs} = $msgs;
+	$ctx->{-html_tip} = '<pre>';
 	PublicInbox::WwwStream::response($ctx, 200, \&thread_html_i);
 }
 
 sub thread_html_i { # PublicInbox::WwwStream::getline callback
-	my ($nr, $ctx) = @_;
+	my ($ctx) = @_;
 	my $msgs = $ctx->{msgs} or return;
 	while (my $smsg = shift @$msgs) {
 		my $eml = $ctx->{-inbox}->smsg_eml($smsg) or next;
+		if (exists $ctx->{-html_tip}) {
+			$ctx->{-title_html} = ascii_html($smsg->{subject});
+			return $ctx->html_top .
+				eml_entry($ctx, $smsg, $eml, scalar @$msgs);
+		}
 		return eml_entry($ctx, $smsg, $eml, scalar @$msgs);
 	}
 	my ($skel) = delete @$ctx{qw(skel msgs)};
@@ -624,23 +621,23 @@ sub add_text_body { # callback for each_part
 }
 
 sub _msg_page_prepare_obuf {
-	my ($hdr, $ctx, $nr) = @_;
+	my ($hdr, $ctx) = @_;
 	my $over = $ctx->{-inbox}->over;
 	my $obfs_ibx = $ctx->{-obfs_ibx};
 	my $rv = '';
 	my $mids = mids_for_index($hdr);
-	if ($nr == 0) {
-		if ($ctx->{more}) {
+	my $nr = $ctx->{nr}++;
+	if ($nr) { # unlikely
+		$rv .= '<pre>';
+	} else {
+		$ctx->{first_hdr} = $hdr;
+		if ($ctx->{smsg}) {
 			$rv .=
 "<pre>WARNING: multiple messages have this Message-ID\n</pre>";
 		}
 		$rv .= "<pre\nid=b>"; # anchor for body start
-	} else {
-		$rv .= '<pre>';
-	}
-	if ($over) {
-		$ctx->{-upfx} = '../';
 	}
+	$ctx->{-upfx} = '../' if $over;
 	my @title; # (Subject[0], From[0])
 	for my $v ($hdr->header('From')) {
 		my @n = PublicInbox::Address::names($v);
@@ -681,7 +678,10 @@ sub _msg_page_prepare_obuf {
 		obfuscate_addrs($obfs_ibx, $v) if $obfs_ibx; # possible :P
 		$rv .= "Date: $v\n";
 	}
-	$ctx->{-title_html} = join(' - ', @title);
+	if (!$nr) { # first (and only) message, common case
+		$ctx->{-title_html} = join(' - ', @title);
+		$rv = $ctx->html_top . $rv;
+	}
 	if (scalar(@$mids) == 1) { # common case
 		my $mhtml = ascii_html($mids->[0]);
 		$rv .= "Message-ID: &lt;$mhtml&gt; ";
@@ -1160,8 +1160,9 @@ sub pagination_footer ($$) {
 	"<hr><pre>page: $next$prev</pre>";
 }
 
-sub index_nav { # callback for WwwStream
-	my (undef, $ctx) = @_;
+sub index_nav { # callback for WwwStream::getline
+	my ($ctx) = @_;
+	return $ctx->html_top if exists $ctx->{-html_tip};
 	pagination_footer($ctx, '.')
 }
 
diff --git a/lib/PublicInbox/WwwStream.pm b/lib/PublicInbox/WwwStream.pm
index c80440d14..4d82cbb48 100644
--- a/lib/PublicInbox/WwwStream.pm
+++ b/lib/PublicInbox/WwwStream.pm
@@ -31,7 +31,6 @@ sub init {
 	my ($ctx, $cb) = @_;
 	$ctx->{cb} = $cb;
 	$ctx->{base_url} = base_url($ctx);
-	$ctx->{nr} = 0;
 	bless $ctx, __PACKAGE__;
 }
 
@@ -43,7 +42,7 @@ sub response {
 	[ $code, $h, $ctx ]
 }
 
-sub _html_top ($) {
+sub html_top ($) {
 	my ($ctx) = @_;
 	my $ibx = $ctx->{-inbox};
 	my $desc = ascii_html($ibx->description);
@@ -159,15 +158,9 @@ EOF
 # callback for HTTP.pm (and any other PSGI servers)
 sub getline {
 	my ($ctx) = @_;
-	my $nr = $ctx->{nr}++;
-
-	my $buf = do {
-		if ($nr == 0) {
-			_html_top($ctx);
-		} elsif (my $middle = $ctx->{cb}) {
-			$middle->($nr, $ctx);
-		}
-	} // (delete($ctx->{cb}) ? _html_end($ctx) : undef);
+	my $cb = $ctx->{cb};
+	my $buf = $cb->($ctx) if $cb;
+	$buf //= delete($ctx->{cb}) ? _html_end($ctx) : undef;
 
 	# gzf may be GzipFilter, `undef' or `0'
 	my $gzf = $ctx->{gzf} or return $buf;
@@ -185,12 +178,12 @@ sub html_oneshot ($$;$) {
 	my $h = [ 'Content-Type' => 'text/html; charset=UTF-8',
 		'Content-Length' => undef ];
 	if (my $gzf = gzf_maybe($h, $ctx->{env})) {
-		$gzf->zmore(_html_top($ctx));
+		$gzf->zmore(html_top($ctx));
 		$gzf->zmore($$sref) if $sref;
 		$x[0] = $gzf->zflush(_html_end($ctx));
 		$h->[3] = length($x[0]);
 	} else {
-		@x = (_html_top($ctx), $sref ? $$sref : (), _html_end($ctx));
+		@x = (html_top($ctx), $sref ? $$sref : (), _html_end($ctx));
 		$h->[3] += bytes::length($_) for @x;
 	}
 	[ $code, $h, \@x ]