* [PATCH 1/4] nntpd: do not delete newsgroup name from inbox object
2022-08-03 20:03 [PATCH 0/4] use ALL to speedup -nntpd and -imapd Eric Wong
@ 2022-08-03 20:03 ` Eric Wong
2022-08-03 20:03 ` [PATCH 2/4] miscidx: index inbox min/max article numbers Eric Wong
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Eric Wong @ 2022-08-03 20:03 UTC (permalink / raw)
To: meta
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.
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/4] miscidx: index inbox min/max article numbers
2022-08-03 20:03 [PATCH 0/4] use ALL to speedup -nntpd and -imapd Eric Wong
2022-08-03 20:03 ` [PATCH 1/4] nntpd: do not delete newsgroup name from inbox object Eric Wong
@ 2022-08-03 20:03 ` Eric Wong
2022-08-03 20:03 ` [PATCH 3/4] nntp: speed up group listings via ->ALL->misc Eric Wong
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Eric Wong @ 2022-08-03 20:03 UTC (permalink / raw)
To: meta
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
}
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 3/4] nntp: speed up group listings via ->ALL->misc
2022-08-03 20:03 [PATCH 0/4] use ALL to speedup -nntpd and -imapd Eric Wong
2022-08-03 20:03 ` [PATCH 1/4] nntpd: do not delete newsgroup name from inbox object Eric Wong
2022-08-03 20:03 ` [PATCH 2/4] miscidx: index inbox min/max article numbers Eric Wong
@ 2022-08-03 20:03 ` Eric Wong
2022-08-03 20:03 ` [PATCH 4/4] imapd: use nntpd_cache to speed up startup/reload time Eric Wong
2022-08-03 23:50 ` [PATCH 0/4] use ALL to speedup -nntpd and -imapd Kyle Meyer
4 siblings, 0 replies; 6+ messages in thread
From: Eric Wong @ 2022-08-03 20:03 UTC (permalink / raw)
To: meta
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 ($$$;$$) {
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 4/4] imapd: use nntpd_cache to speed up startup/reload time
2022-08-03 20:03 [PATCH 0/4] use ALL to speedup -nntpd and -imapd Eric Wong
` (2 preceding siblings ...)
2022-08-03 20:03 ` [PATCH 3/4] nntp: speed up group listings via ->ALL->misc Eric Wong
@ 2022-08-03 20:03 ` Eric Wong
2022-08-03 23:50 ` [PATCH 0/4] use ALL to speedup -nntpd and -imapd Kyle Meyer
4 siblings, 0 replies; 6+ messages in thread
From: Eric Wong @ 2022-08-03 20:03 UTC (permalink / raw)
To: meta
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});
}
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 0/4] use ALL to speedup -nntpd and -imapd
2022-08-03 20:03 [PATCH 0/4] use ALL to speedup -nntpd and -imapd Eric Wong
` (3 preceding siblings ...)
2022-08-03 20:03 ` [PATCH 4/4] imapd: use nntpd_cache to speed up startup/reload time Eric Wong
@ 2022-08-03 23:50 ` Kyle Meyer
4 siblings, 0 replies; 6+ messages in thread
From: Kyle Meyer @ 2022-08-03 23:50 UTC (permalink / raw)
To: Eric Wong; +Cc: meta
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!
^ permalink raw reply [flat|nested] 6+ messages in thread