[PATCH 0/4] use ALL to speedup -nntpd and -imapd

[PATCH 0/4] use ALL to speedup -nntpd and -imapd

[Eric Wong]

Just a normal "public-inbox-extindex --all" invocation should be
enough to trigger these optimizations, no --reindex necessary.

-imapd startup is around ~3x faster, NNTP LIST is ~40x faster.

Eric Wong (4):
  nntpd: do not delete newsgroup name from inbox object
  miscidx: index inbox min/max article numbers
  nntp: speed up group listings via ->ALL->misc
  imapd: use nntpd_cache to speed up startup/reload time

 lib/PublicInbox/IMAP.pm       |  17 +++---
 lib/PublicInbox/IMAPD.pm      | 100 +++++++++++-----------------------
 lib/PublicInbox/Inbox.pm      |  10 ++++
 lib/PublicInbox/MiscIdx.pm    |   8 ++-
 lib/PublicInbox/MiscSearch.pm |  21 ++++---
 lib/PublicInbox/Msgmap.pm     |  12 ++--
 lib/PublicInbox/NNTP.pm       |  26 ++++++---
 lib/PublicInbox/NNTPD.pm      |   1 -
 lib/PublicInbox/Search.pm     |   3 +-
 9 files changed, 96 insertions(+), 102 deletions(-)

[PATCH 1/4] nntpd: do not delete newsgroup name from inbox object

[Eric Wong]

While PublicInbox::NNTP doesn't use it, config sharing inside
public-inbox-netd will mean inbox objects also get shared.
---
 lib/PublicInbox/NNTPD.pm | 1 -
 1 file changed, 1 deletion(-)

diff --git a/lib/PublicInbox/NNTPD.pm b/lib/PublicInbox/NNTPD.pm
index 15a72bac..4f550bb0 100644
--- a/lib/PublicInbox/NNTPD.pm
+++ b/lib/PublicInbox/NNTPD.pm
@@ -48,7 +48,6 @@ sub refresh_groups {
 			$ibx->base_url;
 		} else {
 			delete $groups->{$ngname};
-			delete $ibx->{newsgroup};
 			# Note: don't be tempted to delete more for memory
 			# savings just yet: NNTP, IMAP, and WWW may all
 			# run in the same process someday.

[PATCH 2/4] miscidx: index inbox min/max article numbers

[Eric Wong]

This will be used to speed up NNTP group listings and IMAP startup
with thousands of inboxes.
---
 lib/PublicInbox/Inbox.pm      | 10 ++++++++++
 lib/PublicInbox/MiscIdx.pm    |  8 ++++++--
 lib/PublicInbox/MiscSearch.pm | 21 ++++++++++++---------
 lib/PublicInbox/Msgmap.pm     | 12 ++++++++----
 lib/PublicInbox/Search.pm     |  3 ++-
 5 files changed, 38 insertions(+), 16 deletions(-)

diff --git a/lib/PublicInbox/Inbox.pm b/lib/PublicInbox/Inbox.pm
index 0ad68810..3f70e69d 100644
--- a/lib/PublicInbox/Inbox.pm
+++ b/lib/PublicInbox/Inbox.pm
@@ -409,6 +409,16 @@ sub uidvalidity { $_[0]->{uidvalidity} //= eval { $_[0]->mm->created_at } }
 
 sub eidx_key { $_[0]->{newsgroup} // $_[0]->{inboxdir} }
 
+# only used by NNTP, so we need ->mm anyways
+sub art_min { $_[0]->{-art_min} //= eval { $_[0]->mm(1)->min } }
+
+# used by IMAP, too, which tries to avoid ->mm (but ->{mm} is likely
+# faster since it's smaller iff available)
+sub art_max {
+	$_[0]->{-art_max} //= eval { $_[0]->{mm}->max } //
+				eval { $_[0]->over(1)->max };
+}
+
 sub mailboxid { # rfc 8474, 8620, 8621
 	my ($self, $imap_slice) = @_;
 	my $pfx = defined($imap_slice) ? $self->{newsgroup} : $self->{name};
diff --git a/lib/PublicInbox/MiscIdx.pm b/lib/PublicInbox/MiscIdx.pm
index 5faf5c66..76b33b16 100644
--- a/lib/PublicInbox/MiscIdx.pm
+++ b/lib/PublicInbox/MiscIdx.pm
@@ -108,12 +108,16 @@ EOF
 	$doc->add_boolean_term('Q'.$eidx_key); # uniQue id
 	$doc->add_boolean_term('T'.'inbox'); # Type
 
+	# force reread from disk, {description} could be loaded from {misc}
+	delete @$ibx{qw(-art_min -art_max description)};
 	if (defined($ibx->{newsgroup}) && $ibx->nntp_usable) {
 		$doc->add_boolean_term('T'.'newsgroup'); # additional Type
+		my $n = $ibx->art_min;
+		add_val($doc, $PublicInbox::MiscSearch::ART_MIN, $n) if $n;
+		$n = $ibx->art_max;
+		add_val($doc, $PublicInbox::MiscSearch::ART_MAX, $n) if $n;
 	}
 
-	# force reread from disk, {description} could be loaded from {misc}
-	delete $ibx->{description};
 	my $desc = $ibx->description;
 
 	# description = S/Subject (or title)
diff --git a/lib/PublicInbox/MiscSearch.pm b/lib/PublicInbox/MiscSearch.pm
index c6d2a062..5fb47d03 100644
--- a/lib/PublicInbox/MiscSearch.pm
+++ b/lib/PublicInbox/MiscSearch.pm
@@ -1,4 +1,4 @@
-# Copyright (C) 2020-2021 all contributors <meta@public-inbox.org>
+# Copyright (C) all contributors <meta@public-inbox.org>
 # License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
 
 # read-only counterpart to MiscIdx
@@ -11,6 +11,8 @@ my $json;
 # Xapian value columns:
 our $MODIFIED = 0;
 our $UIDVALIDITY = 1; # (created time)
+our $ART_MIN = 2; # NNTP article number
+our $ART_MAX = 3; # NNTP article number
 
 # avoid conflicting with message Search::prob_prefix for UI/UX reasons
 my %PROB_PREFIX = (
@@ -87,14 +89,13 @@ sub ibx_data_once {
 	my $term = 'Q'.$ibx->eidx_key; # may be {inboxdir}, so private
 	my $head = $xdb->postlist_begin($term);
 	my $tail = $xdb->postlist_end($term);
-	if ($head != $tail) {
-		my $doc = $xdb->get_document($head->get_docid);
-		$ibx->{uidvalidity} //= int_val($doc, $UIDVALIDITY);
-		$ibx->{-modified} = int_val($doc, $MODIFIED);
-		$doc->get_data;
-	} else {
-		undef;
-	}
+	return if $head == $tail;
+	my $doc = $xdb->get_document($head->get_docid);
+	$ibx->{uidvalidity} //= int_val($doc, $UIDVALIDITY);
+	$ibx->{-modified} = int_val($doc, $MODIFIED);
+	$ibx->{-art_min} = int_val($doc, $ART_MIN);
+	$ibx->{-art_max} = int_val($doc, $ART_MAX);
+	$doc->get_data;
 }
 
 sub doc2ibx_cache_ent { # @_ == ($self, $doc) OR ($doc)
@@ -109,6 +110,8 @@ sub doc2ibx_cache_ent { # @_ == ($self, $doc) OR ($doc)
 	{
 		uidvalidity => int_val($doc, $UIDVALIDITY),
 		-modified => int_val($doc, $MODIFIED),
+		-art_min => int_val($doc, $ART_MIN), # may be undef
+		-art_max => int_val($doc, $ART_MAX), # may be undef
 		# extract description from manifest.js.gz epoch description
 		description => $d
 	};
diff --git a/lib/PublicInbox/Msgmap.pm b/lib/PublicInbox/Msgmap.pm
index 1041cd17..cb4bb295 100644
--- a/lib/PublicInbox/Msgmap.pm
+++ b/lib/PublicInbox/Msgmap.pm
@@ -144,13 +144,17 @@ sub max {
 	$sth->fetchrow_array // 0;
 }
 
-sub minmax {
-	# breaking MIN and MAX into separate queries speeds up from 250ms
-	# to around 700us with 2.7million messages.
+sub min {
 	my $sth = $_[0]->{dbh}->prepare_cached('SELECT MIN(num) FROM msgmap',
 						undef, 1);
 	$sth->execute;
-	($sth->fetchrow_array // 0, max($_[0]));
+	$sth->fetchrow_array // 0;
+}
+
+sub minmax {
+	# breaking MIN and MAX into separate queries speeds up from 250ms
+	# to around 700us with 2.7million messages.
+	(min($_[0]), max($_[0]));
 }
 
 sub mid_delete {
diff --git a/lib/PublicInbox/Search.pm b/lib/PublicInbox/Search.pm
index b6141f68..2feb3e13 100644
--- a/lib/PublicInbox/Search.pm
+++ b/lib/PublicInbox/Search.pm
@@ -543,9 +543,10 @@ sub help {
 	\@ret;
 }
 
+# always returns a scalar value
 sub int_val ($$) {
 	my ($doc, $col) = @_;
-	my $val = $doc->get_value($col) or return; # undefined is '' in Xapian
+	my $val = $doc->get_value($col) or return undef; # undef is '' in Xapian
 	sortable_unserialise($val) + 0; # PV => IV conversion
 }
 

[PATCH 3/4] nntp: speed up group listings via ->ALL->misc

[Eric Wong]

By taking advantage of the new ART_MIN/ART_MAX value in MiscIdx,
we can avoid the overhead of opening per-inbox msgmap DB files.
The result gives us a ~40 speedup with 50K newgroups.
---
 lib/PublicInbox/NNTP.pm | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/lib/PublicInbox/NNTP.pm b/lib/PublicInbox/NNTP.pm
index 9ae1353a..524784cb 100644
--- a/lib/PublicInbox/NNTP.pm
+++ b/lib/PublicInbox/NNTP.pm
@@ -123,7 +123,7 @@ sub names2ibx ($;$) {
 sub list_active_i { # "LIST ACTIVE" and also just "LIST" (no args)
 	my ($self, $ibxs) = @_;
 	my @window = splice(@$ibxs, 0, 1000);
-	$self->msg_more(join('', map { group_line($_) } @window));
+	emit_group_lines($self, \@window);
 	scalar @$ibxs; # continue if there's more
 }
 
@@ -244,19 +244,27 @@ sub parse_time ($$;$) {
 	}
 }
 
-sub group_line ($) {
-	my ($ibx) = @_;
-	my ($min, $max) = $ibx->mm(1)->minmax;
-	"$ibx->{newsgroup} $max $min n\r\n";
+sub emit_group_lines {
+	my ($self, $ibxs) = @_;
+	my ($min, $max);
+	my $ALL = $self->{nntpd}->{pi_cfg}->ALL;
+	my $misc = $ALL->misc if $ALL;
+	my $buf = '';
+	for my $ibx (@$ibxs) {
+		$misc ? $misc->inbox_data($ibx) :
+			delete(@$ibx{qw(-art_min -art_max)});
+		($min, $max) = ($ibx->art_min, $ibx->art_max);
+		$buf .= "$ibx->{newsgroup} $max $min n\r\n";
+	}
+	$self->msg_more($buf);
 }
 
 sub newgroups_i {
 	my ($self, $ts, $ibxs) = @_;
 	my @window = splice(@$ibxs, 0, 1000);
-	$self->msg_more(join('', map { group_line($_) } grep {
-		 (eval { $_->uidvalidity } // 0) > $ts
-	} @window));
-	scalar @$ibxs;
+	@window = grep { (eval { $_->uidvalidity } // 0) > $ts } @window;
+	emit_group_lines($self, \@window);
+	scalar @$ibxs; # any more?
 }
 
 sub cmd_newgroups ($$$;$$) {

[PATCH 4/4] imapd: use nntpd_cache to speed up startup/reload time

[Eric Wong]

ConfigIter was still too slow despite being fair.  The addition of
ART_MIN in ALL->misc means it can be used as a startup/reload cache
for -imapd, too.

This results in a ~3x faster startup for -imapd with 50K inboxes.
---
 lib/PublicInbox/IMAP.pm  |  17 +++----
 lib/PublicInbox/IMAPD.pm | 100 +++++++++++++--------------------------
 2 files changed, 41 insertions(+), 76 deletions(-)

diff --git a/lib/PublicInbox/IMAP.pm b/lib/PublicInbox/IMAP.pm
index 19ead70c..9955984b 100644
--- a/lib/PublicInbox/IMAP.pm
+++ b/lib/PublicInbox/IMAP.pm
@@ -350,12 +350,12 @@ sub idle_done ($$) {
 	"$idle_tag OK Idle done\r\n";
 }
 
-sub ensure_slices_exist ($$$) {
-	my ($imapd, $ibx, $max) = @_;
-	defined(my $mb_top = $ibx->{newsgroup}) or return;
+sub ensure_slices_exist ($$) {
+	my ($imapd, $ibx) = @_;
+	my $mb_top = $ibx->{newsgroup} // return;
 	my $mailboxes = $imapd->{mailboxes};
 	my @created;
-	for (my $i = int($max/UID_SLICE); $i >= 0; --$i) {
+	for (my $i = int($ibx->art_max/UID_SLICE); $i >= 0; --$i) {
 		my $sub_mailbox = "$mb_top.$i";
 		last if exists $mailboxes->{$sub_mailbox};
 		$mailboxes->{$sub_mailbox} = $ibx;
@@ -387,7 +387,8 @@ sub inbox_lookup ($$;$) {
 			my $uid_end = $uid_base + UID_SLICE;
 			$exists = $over->imap_exists($uid_base, $uid_end);
 		}
-		ensure_slices_exist($self->{imapd}, $ibx, $over->max);
+		delete $ibx->{-art_max};
+		ensure_slices_exist($self->{imapd}, $ibx);
 	} else {
 		if ($examine) {
 			$self->{uid_base} = $uid_base;
@@ -396,9 +397,9 @@ sub inbox_lookup ($$;$) {
 		}
 		# if "INBOX.foo.bar" is selected and "INBOX.foo.bar.0",
 		# check for new UID ranges (e.g. "INBOX.foo.bar.1")
-		if (my $z = $self->{imapd}->{mailboxes}->{"$mailbox.0"}) {
-			ensure_slices_exist($self->{imapd}, $z,
-						$z->over(1)->max);
+		if (my $ibx = $self->{imapd}->{mailboxes}->{"$mailbox.0"}) {
+			delete $ibx->{-art_max};
+			ensure_slices_exist($self->{imapd}, $ibx);
 		}
 	}
 	($ibx, $exists, $uidmax + 1, $uid_base);
diff --git a/lib/PublicInbox/IMAPD.pm b/lib/PublicInbox/IMAPD.pm
index 6038fd88..5368ff04 100644
--- a/lib/PublicInbox/IMAPD.pm
+++ b/lib/PublicInbox/IMAPD.pm
@@ -6,7 +6,6 @@ package PublicInbox::IMAPD;
 use strict;
 use v5.10.1;
 use PublicInbox::Config;
-use PublicInbox::ConfigIter;
 use PublicInbox::InboxIdle;
 use PublicInbox::IMAP;
 use PublicInbox::DummyInbox;
@@ -15,7 +14,7 @@ my $dummy = bless { uidvalidity => 0 }, 'PublicInbox::DummyInbox';
 sub new {
 	my ($class) = @_;
 	bless {
-		mailboxes => {},
+		# mailboxes => {},
 		err => \*STDERR,
 		out => \*STDOUT,
 		# ssl_ctx_opt => { SSL_cert_file => ..., SSL_key_file => ... }
@@ -25,53 +24,45 @@ sub new {
 }
 
 sub imapd_refresh_ibx { # pi_cfg->each_inbox cb
-	my ($ibx, $imapd) = @_;
-	my $ngname = $ibx->{newsgroup} or return;
+	my ($ibx, $imapd, $cache, $dummies) = @_;
+	my $ngname = $ibx->{newsgroup} // return;
 
 	# We require lower-case since IMAP mailbox names are
 	# case-insensitive (but -nntpd matches INN in being
-	# case-sensitive
+	# case-sensitive)
 	if ($ngname =~ m![^a-z0-9/_\.\-\~\@\+\=:]! ||
 			# don't confuse with 50K slices
 			$ngname =~ /\.[0-9]+\z/) {
 		warn "mailbox name invalid: newsgroup=`$ngname'\n";
 		return;
 	}
-	$ibx->over or return;
-	$ibx->{over} = undef;
-
-	# RFC 3501 2.3.1.1 -  "A good UIDVALIDITY value to use in
-	# this case is a 32-bit representation of the creation
-	# date/time of the mailbox"
-	eval { $ibx->uidvalidity };
-	my $mm = delete($ibx->{mm}) or return;
-	defined($ibx->{uidvalidity}) or return;
-	PublicInbox::IMAP::ensure_slices_exist($imapd, $ibx, $mm->max);
-
-	# preload to avoid fragmentation:
-	$ibx->description;
-	$ibx->base_url;
-
-	# ensure dummies are selectable
-	my $dummies = $imapd->{dummies};
-	do {
-		$dummies->{$ngname} = $dummy;
-	} while ($ngname =~ s/\.[^\.]+\z//);
+	my $ce = $cache->{$ngname};
+	%$ibx = (%$ibx, %$ce) if $ce;
+	# only valid if msgmap and over works:
+	if (defined($ibx->uidvalidity)) {
+		# fill ->{mailboxes}:
+		PublicInbox::IMAP::ensure_slices_exist($imapd, $ibx);
+		# preload to avoid fragmentation:
+		$ibx->description;
+		$ibx->base_url;
+		# ensure dummies are selectable:
+		do {
+			$dummies->{$ngname} = $dummy;
+		} while ($ngname =~ s/\.[^\.]+\z//);
+	}
+	delete @$ibx{qw(mm over)};
 }
 
-sub imapd_refresh_finalize {
-	my ($imapd, $pi_cfg) = @_;
-	my $mailboxes;
-	if (my $next = delete $imapd->{imapd_next}) {
-		$imapd->{mailboxes} = delete $next->{mailboxes};
-		$mailboxes = delete $next->{dummies};
-	} else {
-		$mailboxes = delete $imapd->{dummies};
-	}
-	%$mailboxes = (%$mailboxes, %{$imapd->{mailboxes}});
-	$imapd->{mailboxes} = $mailboxes;
-	$imapd->{mailboxlist} = [
-		map { $_->[2] }
+sub refresh_groups {
+	my ($self, $sig) = @_;
+	my $pi_cfg = PublicInbox::Config->new;
+	my $mailboxes = $self->{mailboxes} = {};
+	my $cache = eval { $pi_cfg->ALL->misc->nntpd_cache_load } // {};
+	my $dummies = {};
+	$pi_cfg->each_inbox(\&imapd_refresh_ibx, $self, $cache, $dummies);
+	%$dummies = (%$dummies, %$mailboxes);
+	$mailboxes = $self->{mailboxes} = $dummies;
+	@{$self->{mailboxlist}} = map { $_->[2] }
 		sort { $a->[0] cmp $b->[0] || $a->[1] <=> $b->[1] }
 		map {
 			my $u = $_; # capitalize "INBOX" for user-familiarity
@@ -85,40 +76,13 @@ sub imapd_refresh_finalize {
 				[ $1, $2 + 0,
 				  qq[* LIST (\\HasNoChildren) "." $u\r\n] ]
 			}
-		} keys %$mailboxes
-	];
-	$imapd->{pi_cfg} = $pi_cfg;
-	if (my $idler = $imapd->{idler}) {
+		} keys %$mailboxes;
+	$self->{pi_cfg} = $pi_cfg;
+	if (my $idler = $self->{idler}) {
 		$idler->refresh($pi_cfg);
 	}
 }
 
-sub imapd_refresh_step { # PublicInbox::ConfigIter cb
-	my ($pi_cfg, $section, $imapd) = @_;
-	if (defined($section)) {
-		return if $section !~ m!\Apublicinbox\.([^/]+)\z!;
-		my $ibx = $pi_cfg->lookup_name($1) or return;
-		imapd_refresh_ibx($ibx, $imapd->{imapd_next});
-	} else { # undef == "EOF"
-		imapd_refresh_finalize($imapd, $pi_cfg);
-	}
-}
-
-sub refresh_groups {
-	my ($self, $sig) = @_;
-	my $pi_cfg = PublicInbox::Config->new;
-	if ($sig) { # SIGHUP is handled through the event loop
-		$self->{imapd_next} = { dummies => {}, mailboxes => {} };
-		my $iter = PublicInbox::ConfigIter->new($pi_cfg,
-						\&imapd_refresh_step, $self);
-		$iter->event_step;
-	} else { # initial start is synchronous
-		$self->{dummies} = {};
-		$pi_cfg->each_inbox(\&imapd_refresh_ibx, $self);
-		imapd_refresh_finalize($self, $pi_cfg);
-	}
-}
-
 sub idler_start {
 	$_[0]->{idler} //= PublicInbox::InboxIdle->new($_[0]->{pi_cfg});
 }

Re: [PATCH 0/4] use ALL to speedup -nntpd and -imapd

[Kyle Meyer]

Eric Wong writes:

> Just a normal "public-inbox-extindex --all" invocation should be
> enough to trigger these optimizations, no --reindex necessary.
>
> -imapd startup is around ~3x faster, NNTP LIST is ~40x faster.

Nice, thank you!