[PATCH v2 12/12] lib: Remove unnecessary thread linking steps when using ghost messages

[Mark Walters]

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,
> -  _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,
> +  _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.

Best wishes

Mark

>  
>  /* If not part of any existing thread, generate a new thread ID. */
>  if (thread_id == NULL) {
> -- 
> 2.1.0
>
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH v2 12/12] lib: Remove unnecessary thread linking steps when using ghost messages

[Austin Clements]

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,
> > -_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,
> > +_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 at notmuchmail.org
> > http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v2 12/12] lib: Remove unnecessary thread linking steps when using ghost messages

[Mark Walters]

On Tue, 07 Oct 2014, Austin Clements acleme...@csail.mit.edu wrote:
 From: Austin Clements amdra...@mit.edu

 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.

Best wishes

Mark

  
  /* If not part of any existing thread, generate a new thread ID. */
  if (thread_id == NULL) {
 -- 
 2.1.0

 ___
 notmuch mailing list
 notmuch@notmuchmail.org
 http://notmuchmail.org/mailman/listinfo/notmuch
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v2 12/12] lib: Remove unnecessary thread linking steps when using ghost messages

[Austin Clements]

Quoth Mark Walters on Oct 22 at 12:17 am:
 On Tue, 07 Oct 2014, Austin Clements acleme...@csail.mit.edu wrote:
  From: Austin Clements amdra...@mit.edu
 
  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
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH v2 12/12] lib: Remove unnecessary thread linking steps when using ghost messages

[Austin Clements]

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,
-_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,
+_id);
+   if (status)
+   goto DONE;
+}

 /* If not part of any existing thread, generate a new thread ID. */
 if (thread_id == NULL) {
-- 
2.1.0

[PATCH v2 12/12] lib: Remove unnecessary thread linking steps when using ghost messages

[Austin Clements]

From: Austin Clements amdra...@mit.edu

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;
+}
 
 /* If not part of any existing thread, generate a new thread ID. */
 if (thread_id == NULL) {
-- 
2.1.0

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch