From: Austin Clements Date: Wed, 22 Oct 2014 01:51:01 +0000 (+2000) Subject: Re: [PATCH v2 12/12] lib: Remove unnecessary thread linking steps when using ghost... X-Git-Url: http://git.tremily.us/?a=commitdiff_plain;h=50cfb1f08ea0b0bff071565a9349044a581b7b52;p=notmuch-archives.git Re: [PATCH v2 12/12] lib: Remove unnecessary thread linking steps when using ghost messages --- diff --git a/22/0a6b0fe3b96320630eaa0f8726eaaf47dc5e46 b/22/0a6b0fe3b96320630eaa0f8726eaaf47dc5e46 new file mode 100644 index 000000000..ee3ba14a2 --- /dev/null +++ b/22/0a6b0fe3b96320630eaa0f8726eaaf47dc5e46 @@ -0,0 +1,138 @@ +Return-Path: +X-Original-To: notmuch@notmuchmail.org +Delivered-To: notmuch@notmuchmail.org +Received: from localhost (localhost [127.0.0.1]) + by olra.theworths.org (Postfix) with ESMTP id 1DCF0431FB6 + for ; Tue, 21 Oct 2014 18:51:09 -0700 (PDT) +X-Virus-Scanned: Debian amavisd-new at olra.theworths.org +X-Spam-Flag: NO +X-Spam-Score: -2.3 +X-Spam-Level: +X-Spam-Status: No, score=-2.3 tagged_above=-999 required=5 + tests=[RCVD_IN_DNSWL_MED=-2.3] autolearn=disabled +Received: from olra.theworths.org ([127.0.0.1]) + by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024) + with ESMTP id hdepnvN9TTz6 for ; + Tue, 21 Oct 2014 18:51:01 -0700 (PDT) +Received: from outgoing.csail.mit.edu (outgoing.csail.mit.edu [128.30.2.149]) + by olra.theworths.org (Postfix) with ESMTP id A9137431FAE + for ; Tue, 21 Oct 2014 18:51:01 -0700 (PDT) +Received: from [104.131.20.129] (helo=awakeningjr) + by outgoing.csail.mit.edu with esmtpsa (TLS1.0:RSA_AES_128_CBC_SHA1:16) + (Exim 4.72) (envelope-from ) + id 1Xgl4n-0005IZ-A2; Tue, 21 Oct 2014 21:51:01 -0400 +Received: from amthrax by awakeningjr with local (Exim 4.84) + (envelope-from ) + id 1Xgl4n-0000IQ-1j; Tue, 21 Oct 2014 21:51:01 -0400 +Date: Tue, 21 Oct 2014 21:51:01 -0400 +From: Austin Clements +To: Mark Walters +Subject: Re: [PATCH v2 12/12] lib: Remove unnecessary thread linking steps + when using ghost messages +Message-ID: <20141022015101.GF7970@csail.mit.edu> +References: <1412637438-4821-1-git-send-email-aclements@csail.mit.edu> + <1412637438-4821-13-git-send-email-aclements@csail.mit.edu> + <87wq7thv27.fsf@qmul.ac.uk> +MIME-Version: 1.0 +Content-Type: text/plain; charset=us-ascii +Content-Disposition: inline +In-Reply-To: <87wq7thv27.fsf@qmul.ac.uk> +User-Agent: Mutt/1.5.23 (2014-03-12) +Cc: notmuch@notmuchmail.org +X-BeenThere: notmuch@notmuchmail.org +X-Mailman-Version: 2.1.13 +Precedence: list +List-Id: "Use and development of the notmuch mail system." + +List-Unsubscribe: , + +List-Archive: +List-Post: +List-Help: +List-Subscribe: , + +X-List-Received-Date: Wed, 22 Oct 2014 01:51:09 -0000 + +Quoth Mark Walters on Oct 22 at 12:17 am: +> On Tue, 07 Oct 2014, Austin Clements wrote: +> > From: Austin Clements +> > +> > Previously, it was necessary to link new messages to children to work +> > around some (though not all) problems with the old metadata-based +> > approach to stored thread IDs. With ghost messages, this is no longer +> > necessary, so don't bother with child linking when ghost messages are +> > in use. +> > --- +> > lib/database.cc | 21 +++++++++++++++++---- +> > 1 file changed, 17 insertions(+), 4 deletions(-) +> > +> > diff --git a/lib/database.cc b/lib/database.cc +> > index 1316529..6e51a72 100644 +> > --- a/lib/database.cc +> > +++ b/lib/database.cc +> > @@ -2169,10 +2169,23 @@ _notmuch_database_link_message (notmuch_database_t *notmuch, +> > if (status) +> > goto DONE; +> > +> > - status = _notmuch_database_link_message_to_children (notmuch, message, +> > - &thread_id); +> > - if (status) +> > - goto DONE; +> > + if (! (notmuch->features & NOTMUCH_FEATURE_GHOSTS)) { +> > + /* In general, it shouldn't be necessary to link children, +> > + * since the earlier indexing of those children will have +> > + * stored a thread ID for the missing parent. However, prior +> > + * to ghost messages, these stored thread IDs were NOT +> > + * rewritten during thread merging (and there was no +> > + * performant way to do so), so if indexed children were +> > + * pulled into a different thread ID by a merge, it was +> > + * necessary to pull them *back* into the stored thread ID of +> > + * the parent. With ghost messages, we just rewrite the +> > + * stored thread IDs during merging, so this workaround isn't +> > + * necessary. */ +> > + status = _notmuch_database_link_message_to_children (notmuch, message, +> > + &thread_id); +> > + if (status) +> > + goto DONE; +> > + } +> +> Ok so this basically answers my earlier comment. It might be worth +> updating the big comment at the start of the function to match the new +> code though. + +Like so? + +diff --git a/lib/database.cc b/lib/database.cc +index d063ec8..3601f9d 100644 +--- a/lib/database.cc ++++ b/lib/database.cc +@@ -2136,11 +2136,11 @@ _consume_metadata_thread_id (void *ctx, notmuch_database_t *notmuch, + * reference 'message'. + * + * In all cases, we assign to the current message the first thread ID +- * found (through either parent or child). We will also merge any +- * existing, distinct threads where this message belongs to both, +- * (which is not uncommon when messages are processed out of order). ++ * found. We will also merge any existing, distinct threads where this ++ * message belongs to both, (which is not uncommon when messages are ++ * processed out of order). + * +- * Finally, if no thread ID has been found through parent or child, we ++ * Finally, if no thread ID has been found through referenced messages, we + * call _notmuch_message_generate_thread_id to generate a new thread + * ID. This should only happen for new, top-level messages, (no + * References or In-Reply-To header in this message, and no previously + + +> Best wishes +> +> Mark +> +> > +> > /* If not part of any existing thread, generate a new thread ID. */ +> > if (thread_id == NULL) { +> > +> > _______________________________________________ +> > notmuch mailing list +> > notmuch@notmuchmail.org +> > http://notmuchmail.org/mailman/listinfo/notmuch