user/dev discussion of public-inbox itself
 help / color / mirror / code / Atom feed
Search results ordered by [date|relevance]  view[summary|nested|Atom feed]
thread overview below | download mbox.gz: |
* Re: message threading in the web UI
  @ 2019-11-16 14:40  5%   ` Ralph Siemsen
  0 siblings, 0 replies; 3+ results
From: Ralph Siemsen @ 2019-11-16 14:40 UTC (permalink / raw)
  To: Eric Wong; +Cc: meta

On Fri, Nov 15, 2019 at 11:52:35PM +0000, Eric Wong wrote:
>Ralph Siemsen <ralph.siemsen@linaro.org> wrote:
>> The way that threads are displayed seems to differ between the "frontpage"
>> (eg. https://public-inbox.org/meta/) versus how they are shown in the
>> "Thread Overview" (at the bottom of the page when viewing a specific
>> thread).
>>
>> On the frontpage, it seems only a subset of messages are displayed (maybe
>> only the direct replies?). The only indication that there are additional
>> hidden messages is the "+" sign, e.g. "(5+ messages)".
>
>Yes, the frontpage only grabs the 200 most recent messages
>(a "time window").

Ah, I see. Makes sense from a programmers point of view of course. 
However as an end user, based on what is shown in the UI, it looks like 
each thread is complete -- the number of messages in each thread varies 
(not artificially capped at say 5 messages per thread). When the thread 
consists of a patch series, the whole series seems to be displayed 
(likely because they are all posted together at the same time).

It is not very obvious that some additional messages (such as replies to 
the individual patches in a series) have been hidden. I only discovered 
it when I used my browser "Find in Page", and it did not find all the 
occurrences (but using "search" box on PI did find them).

>Getting the full count ("M") can be expensive since it needs an
>SQLite COUNT query for every thread...

Understood.

>Perhaps as an interim solution is to change the "+" for
>something more obvious:
>
>"5 or more messages"
>"5 messages in current time window"
>">= 5 messages"
>"at least 5 messages"

While those are a bit more obvious than just a "+" sign, to me at least, 
they are all ambiguous. The number itself is not really helping, since 
it is just a count of the number of lines that are displayed immediately 
below. In other words, the count does not add any information.

So perhaps just remove it entirely? For example:

[PATCH 0/3] start tidying up gzip-related code
 2019-11-16  2:34 UTC
` [PATCH 1/3] mbox: unused mid_clean import
` [PATCH 2/3] mbox: split mboxgz out into a separate file
` [PATCH 3/3] mboxgz: use Compress::Raw::Zlib instead of IO::Compress::Gzip
` <additional replies>

Note the extra line at the end, containing a link to the full thread. 
This essentially replaces the "+" with a more obvious indication that
messages have been omitted from the summary.

>I'm not sure how feasable that is since frontpage performance is
>critical and I spent a fair amount of time making it perform
>acceptably for giant inboxes like LKML.
>
>(Optionally) supporting Cache::FastMmap or other caching
>mechanisms may help, but that's still extra install overhead and
>initial load performance still needs to be taken into account.

Caching the whole page, esp for LKML and the like, certainly makes 
sense, but that is a bigger task for sure.

Thanks for your reply!
-Ralph


^ permalink raw reply	[relevance 5%]

* [PATCH 2/3] mbox: split mboxgz out into a separate file
  2019-11-16  2:34  6% [PATCH 0/3] start tidying up gzip-related code Eric Wong
@ 2019-11-16  2:34  7% ` Eric Wong
  0 siblings, 0 replies; 3+ results
From: Eric Wong @ 2019-11-16  2:34 UTC (permalink / raw)
  To: meta

It'll make using Compress::Raw::Zlib easier, since we
can use that and import constants more easily.
---
 MANIFEST                  |  1 +
 lib/PublicInbox/Mbox.pm   | 64 ++-------------------------------------
 lib/PublicInbox/MboxGz.pm | 64 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 67 insertions(+), 62 deletions(-)
 create mode 100644 lib/PublicInbox/MboxGz.pm

diff --git a/MANIFEST b/MANIFEST
index ef8538b4..689d3d4e 100644
--- a/MANIFEST
+++ b/MANIFEST
@@ -119,6 +119,7 @@ lib/PublicInbox/MDA.pm
 lib/PublicInbox/MID.pm
 lib/PublicInbox/MIME.pm
 lib/PublicInbox/Mbox.pm
+lib/PublicInbox/MboxGz.pm
 lib/PublicInbox/MsgIter.pm
 lib/PublicInbox/MsgTime.pm
 lib/PublicInbox/Msgmap.pm
diff --git a/lib/PublicInbox/Mbox.pm b/lib/PublicInbox/Mbox.pm
index 9e808c09..42ed8c5d 100644
--- a/lib/PublicInbox/Mbox.pm
+++ b/lib/PublicInbox/Mbox.pm
@@ -136,7 +136,7 @@ sub msg_body ($) {
 
 sub thread_mbox {
 	my ($ctx, $over, $sfx) = @_;
-	eval { require IO::Compress::Gzip };
+	eval { require PublicInbox::MboxGz };
 	return sub { need_gzip(@_) } if $@;
 	my $mid = $ctx->{mid};
 	my $msgs = $over->get_thread($mid, {});
@@ -196,7 +196,7 @@ sub mbox_all_ids {
 sub mbox_all {
 	my ($ctx, $query) = @_;
 
-	eval { require IO::Compress::Gzip };
+	eval { require PublicInbox::MboxGz };
 	return sub { need_gzip(@_) } if $@;
 	return mbox_all_ids($ctx) if $query eq '';
 	my $opts = { mset => 2 };
@@ -239,63 +239,3 @@ EOF
 }
 
 1;
-
-package PublicInbox::MboxGz;
-use strict;
-use warnings;
-use PublicInbox::Hval qw/to_filename/;
-
-sub new {
-	my ($class, $ctx, $cb) = @_;
-	my $buf = '';
-	$ctx->{base_url} = $ctx->{-inbox}->base_url($ctx->{env});
-	bless {
-		buf => \$buf,
-		gz => IO::Compress::Gzip->new(\$buf, Time => 0),
-		cb => $cb,
-		ctx => $ctx,
-	}, $class;
-}
-
-sub response {
-	my ($class, $ctx, $cb, $fn) = @_;
-	my $body = $class->new($ctx, $cb);
-	# http://www.iana.org/assignments/media-types/application/gzip
-	my @h = qw(Content-Type application/gzip);
-	if ($fn) {
-		$fn = to_filename($fn);
-		push @h, 'Content-Disposition', "inline; filename=$fn.mbox.gz";
-	}
-	[ 200, \@h, $body ];
-}
-
-# called by Plack::Util::foreach or similar
-sub getline {
-	my ($self) = @_;
-	my $ctx = $self->{ctx} or return;
-	my $gz = $self->{gz};
-	while (my $smsg = $self->{cb}->()) {
-		my $mref = $ctx->{-inbox}->msg_by_smsg($smsg) or next;
-		my $h = Email::Simple->new($mref)->header_obj;
-		$gz->write(PublicInbox::Mbox::msg_hdr($ctx, $h, $smsg->{mid}));
-		$gz->write(PublicInbox::Mbox::msg_body($$mref));
-
-		my $bref = $self->{buf};
-		if (length($$bref) >= 8192) {
-			my $ret = $$bref; # copy :<
-			${$self->{buf}} = '';
-			return $ret;
-		}
-
-		# be fair to other clients on public-inbox-httpd:
-		return '';
-	}
-	delete($self->{gz})->close;
-	# signal that we're done and can return undef next call:
-	delete $self->{ctx};
-	${delete $self->{buf}};
-}
-
-sub close {} # noop
-
-1;
diff --git a/lib/PublicInbox/MboxGz.pm b/lib/PublicInbox/MboxGz.pm
new file mode 100644
index 00000000..2919ad6a
--- /dev/null
+++ b/lib/PublicInbox/MboxGz.pm
@@ -0,0 +1,64 @@
+# Copyright (C) 2015-2019 all contributors <meta@public-inbox.org>
+# License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
+package PublicInbox::MboxGz;
+use strict;
+use warnings;
+use Email::Simple;
+use PublicInbox::Hval qw/to_filename/;
+use PublicInbox::Mbox;
+use IO::Compress::Gzip;
+
+sub new {
+	my ($class, $ctx, $cb) = @_;
+	my $buf = '';
+	$ctx->{base_url} = $ctx->{-inbox}->base_url($ctx->{env});
+	bless {
+		buf => \$buf,
+		gz => IO::Compress::Gzip->new(\$buf, Time => 0),
+		cb => $cb,
+		ctx => $ctx,
+	}, $class;
+}
+
+sub response {
+	my ($class, $ctx, $cb, $fn) = @_;
+	my $body = $class->new($ctx, $cb);
+	# http://www.iana.org/assignments/media-types/application/gzip
+	my @h = qw(Content-Type application/gzip);
+	if ($fn) {
+		$fn = to_filename($fn);
+		push @h, 'Content-Disposition', "inline; filename=$fn.mbox.gz";
+	}
+	[ 200, \@h, $body ];
+}
+
+# called by Plack::Util::foreach or similar
+sub getline {
+	my ($self) = @_;
+	my $ctx = $self->{ctx} or return;
+	my $gz = $self->{gz};
+	while (my $smsg = $self->{cb}->()) {
+		my $mref = $ctx->{-inbox}->msg_by_smsg($smsg) or next;
+		my $h = Email::Simple->new($mref)->header_obj;
+		$gz->write(PublicInbox::Mbox::msg_hdr($ctx, $h, $smsg->{mid}));
+		$gz->write(PublicInbox::Mbox::msg_body($$mref));
+
+		my $bref = $self->{buf};
+		if (length($$bref) >= 8192) {
+			my $ret = $$bref; # copy :<
+			${$self->{buf}} = '';
+			return $ret;
+		}
+
+		# be fair to other clients on public-inbox-httpd:
+		return '';
+	}
+	delete($self->{gz})->close;
+	# signal that we're done and can return undef next call:
+	delete $self->{ctx};
+	${delete $self->{buf}};
+}
+
+sub close {} # noop
+
+1;

^ permalink raw reply related	[relevance 7%]

* [PATCH 0/3] start tidying up gzip-related code
@ 2019-11-16  2:34  6% Eric Wong
  2019-11-16  2:34  7% ` [PATCH 2/3] mbox: split mboxgz out into a separate file Eric Wong
  0 siblings, 1 reply; 3+ results
From: Eric Wong @ 2019-11-16  2:34 UTC (permalink / raw)
  To: meta

Starting with the mbox.gz stuff, first.  Gettig rid of
Plack::Middleware::Deflater is a long-term goal since we can
take advantage of doing gzip during HTML/XML rendering to
reduce memory usage.

Eric Wong (3):
  mbox: unused mid_clean import
  mbox: split mboxgz out into a separate file
  mboxgz: use Compress::Raw::Zlib instead of IO::Compress::Gzip

 MANIFEST                  |  1 +
 lib/PublicInbox/Mbox.pm   | 68 +++----------------------------------
 lib/PublicInbox/MboxGz.pm | 71 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 76 insertions(+), 64 deletions(-)
 create mode 100644 lib/PublicInbox/MboxGz.pm


^ permalink raw reply	[relevance 6%]

Results 1-3 of 3 | reverse | options above
-- pct% links below jump to the message on this page, permalinks otherwise --
2019-11-15 21:28     message threading in the web UI Ralph Siemsen
2019-11-15 23:52     ` Eric Wong
2019-11-16 14:40  5%   ` Ralph Siemsen
2019-11-16  2:34  6% [PATCH 0/3] start tidying up gzip-related code Eric Wong
2019-11-16  2:34  7% ` [PATCH 2/3] mbox: split mboxgz out into a separate file Eric Wong

Code repositories for project(s) associated with this public inbox

	https://80x24.org/public-inbox.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).