On Fri, 07 Dec 2012, david@tethera.net wrote: > From: David Bremner <bremner@debian.org> > > The idea is not to bother with restore operations if they don't change > the set of tags. This is actually a relatively common case. > > In order to avoid fancy datastructures, this method is quadratic in > the number of tags; at least on my mail database this doesn't seem to > be a big problem. > --- > tag-util.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 66 insertions(+) > > diff --git a/tag-util.c b/tag-util.c > index 932ee7f..3d54e9e 100644 > --- a/tag-util.c > +++ b/tag-util.c > @@ -124,6 +124,69 @@ message_error (notmuch_message_t *message, > fprintf (stderr, "Status: %s\n", notmuch_status_to_string (status)); > } > > +static int > +makes_changes (notmuch_message_t *message, > + tag_op_list_t *list, > + tag_op_flag_t flags) > +{ > + > + size_t i; > + > + notmuch_tags_t *tags; > + notmuch_bool_t changes = FALSE; > + > + /* First, do we delete an existing tag? */ > + changes = FALSE; > + for (tags = notmuch_message_get_tags (message); > + ! changes && notmuch_tags_valid (tags); > + notmuch_tags_move_to_next (tags)) { > + const char *cur_tag = notmuch_tags_get (tags); > + int last_op = (flags & TAG_FLAG_REMOVE_ALL) ? -1 : 0; > + > + /* slight contortions to count down with an unsigned index */ > + for (i = list->count; i-- > 0; /*nothing*/) { > + if (strcmp (cur_tag, list->ops[i].tag) == 0) { > + last_op = list->ops[i].remove ? -1 : 1; > + break; > + } > + } After some eyeballing it looks correct, but not not exactly pretty. If you insist on unsigned, you could also have a regular (i = 0; i < list->count; i++) and use j = list->count - i - 1; within the block. But that's just style bikeshedding after convincing you to use a count down loop in the first place... Otherwise, the patch LGTM. > + > + changes = (last_op == -1); > + } > + notmuch_tags_destroy (tags); > + > + if (changes) > + return TRUE; > + > + /* Now check for adding new tags */ > + for (i = 0; i < list->count; i++) { > + notmuch_bool_t exists = FALSE; > + > + if (list->ops[i].remove) > + continue; > + > + for (tags = notmuch_message_get_tags (message); > + notmuch_tags_valid (tags); > + notmuch_tags_move_to_next (tags)) { > + const char *cur_tag = notmuch_tags_get (tags); > + if (strcmp (cur_tag, list->ops[i].tag) == 0) { > + exists = TRUE; > + break; > + } > + } > + notmuch_tags_destroy (tags); > + > + /* the following test is conservative, > + * in the sense it ignores cases like +foo ... -foo > + * but this is OK from a correctness point of view > + */ > + if (! exists) > + return TRUE; > + } > + return FALSE; > + > +} > + > notmuch_status_t > tag_op_list_apply (notmuch_message_t *message, > tag_op_list_t *list, > @@ -133,6 +196,9 @@ tag_op_list_apply (notmuch_message_t *message, > notmuch_status_t status = 0; > tag_operation_t *tag_ops = list->ops; > > + if (! (flags & TAG_FLAG_PRE_OPTIMIZED) && ! makes_changes (message, list, flags)) > + return NOTMUCH_STATUS_SUCCESS; > + > status = notmuch_message_freeze (message); > if (status) { > message_error (message, status, "freezing message"); > -- > 1.7.10.4 > > _______________________________________________ > notmuch mailing list > notmuch@notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch