fa/e087e3fa22b5ff3cd2bc2b55b9dffb849330f2

   1 Return-Path: <jani@nikula.org>\r
   2 X-Original-To: notmuch@notmuchmail.org\r
   3 Delivered-To: notmuch@notmuchmail.org\r
   4 Received: from localhost (localhost [127.0.0.1])\r
   5         by olra.theworths.org (Postfix) with ESMTP id 7045D431FAF\r
   6         for <notmuch@notmuchmail.org>; Sun,  2 Dec 2012 06:42:23 -0800 (PST)\r
   7 X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
   8 X-Spam-Flag: NO\r
   9 X-Spam-Score: -0.7\r
  10 X-Spam-Level: \r
  11 X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5\r
  12         tests=[RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled\r
  13 Received: from olra.theworths.org ([127.0.0.1])\r
  14         by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
  15         with ESMTP id CIJgjHjkayEV for <notmuch@notmuchmail.org>;\r
  16         Sun,  2 Dec 2012 06:42:19 -0800 (PST)\r
  17 Received: from mail-lb0-f181.google.com (mail-lb0-f181.google.com\r
  18         [209.85.217.181]) (using TLSv1 with cipher RC4-SHA (128/128 bits))\r
  19         (No client certificate requested)\r
  20         by olra.theworths.org (Postfix) with ESMTPS id B2ADA431FAE\r
  21         for <notmuch@notmuchmail.org>; Sun,  2 Dec 2012 06:42:18 -0800 (PST)\r
  22 Received: by mail-lb0-f181.google.com with SMTP id ge1so1790567lbb.26\r
  23         for <notmuch@notmuchmail.org>; Sun, 02 Dec 2012 06:42:17 -0800 (PST)\r
  24 X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;\r
  25         d=google.com; s=20120113;\r
  26         h=from:to:cc:subject:in-reply-to:references:user-agent:date\r
  27         :message-id:mime-version:content-type:x-gm-message-state;\r
  28         bh=XxZDRzfk1zO5LuVqfSjhThU6FJaR8zl08ZgYXq3yTVk=;\r
  29         b=gdbKe4ojBA4kpYhNG84JmRyfoqhZgIjwPbxpGxQ4ftrS0GwXL8s5wsoyqk3h/z6B/F\r
  30         md5GDJVQ+TdeY2z/jU0PB/4oUuNwM0FI6rb5MG2P9zMYE663+bqCmclImGYJweAe3m4G\r
  31         YtaAZjLvo1wFIRakXaldFIFfNX6LgJsdiUgXrnmxbjXgtuE1RI3WjPvCg+mbjquPhBAv\r
  32         SFi5+F6PsdOoeE7+9J8cSgk8JO40WieF75MJZyNUOnqS+NueFojnNjPk1yjDglbPThaw\r
  33         PW5gh9m6uGLCKqYo/V3gdcFbAKVPIW/xIqA+rT8NUYyconiFOePnBYCfV9b0rGlH3U6g\r
  34         NADg==\r
  35 Received: by 10.112.49.167 with SMTP id v7mr3165890lbn.122.1354459335873;\r
  36         Sun, 02 Dec 2012 06:42:15 -0800 (PST)\r
  37 Received: from localhost (dsl-hkibrasgw4-fe51df00-27.dhcp.inet.fi.\r
  38         [80.223.81.27])\r
  39         by mx.google.com with ESMTPS id v6sm4197127lbf.11.2012.12.02.06.42.13\r
  40         (version=SSLv3 cipher=OTHER); Sun, 02 Dec 2012 06:42:14 -0800 (PST)\r
  41 From: Jani Nikula <jani@nikula.org>\r
  42 To: david@tethera.net, notmuch@notmuchmail.org\r
  43 Subject: Re: [Patch v2 17/17] tag-util: optimization of tag application\r
  44 In-Reply-To: <1353792017-31459-18-git-send-email-david@tethera.net>\r
  45 References: <1353792017-31459-1-git-send-email-david@tethera.net>\r
  46         <1353792017-31459-18-git-send-email-david@tethera.net>\r
  47 User-Agent: Notmuch/0.14+124~g3b17402 (http://notmuchmail.org) Emacs/23.4.1\r
  48         (i686-pc-linux-gnu)\r
  49 Date: Sun, 02 Dec 2012 16:42:12 +0200\r
  50 Message-ID: <87txs4cy7v.fsf@nikula.org>\r
  51 MIME-Version: 1.0\r
  52 Content-Type: text/plain; charset=us-ascii\r
  53 X-Gm-Message-State:\r
  54  ALoCoQkWGwgv9AMGeWAoWMdfxZHI42CEc1twFZ3so6RaVfwuYvj/X2nIFeLg94O1+7DB9VI+X16k\r
  55 Cc: David Bremner <bremner@debian.org>\r
  56 X-BeenThere: notmuch@notmuchmail.org\r
  57 X-Mailman-Version: 2.1.13\r
  58 Precedence: list\r
  59 List-Id: "Use and development of the notmuch mail system."\r
  60         <notmuch.notmuchmail.org>\r
  61 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  62         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  63 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  64 List-Post: <mailto:notmuch@notmuchmail.org>\r
  65 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  66 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  67         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  68 X-List-Received-Date: Sun, 02 Dec 2012 14:42:23 -0000\r
  69 \r
  70 On Sat, 24 Nov 2012, david@tethera.net wrote:\r
  71 > From: David Bremner <bremner@debian.org>\r
  72 >\r
  73 > The idea is not to bother with restore operations if they don't change\r
  74 > the set of tags. This is actually a relatively common case.\r
  75 >\r
  76 > In order to avoid fancy datastructures, this method is quadratic in\r
  77 > the number of tags; at least on my mail database this doesn't seem to\r
  78 > be a big problem.\r
  79 \r
  80 It's bound to be slower than the original optimization using strcmp, but\r
  81 that was based on a number of assumptions we can no longer make. In any\r
  82 case, not doing message sync is the key optimization.\r
  83 \r
  84 One side effect of any optimization (also the existing one) is not doing\r
  85 maildir sync from tags to flags when there are no tag changes. This just\r
  86 emphasizes that one should never do dump or restore with the mail store\r
  87 and database out of sync. I don't think we emphasize the importance of\r
  88 running notmuch new before dump and restore enough.\r
  89 \r
  90 A few more comments below.\r
  91 \r
  92 BR,\r
  93 Jani.\r
  94 \r
  95 > ---\r
  96 >  notmuch-tag.c |    2 +-\r
  97 >  tag-util.c    |   59 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++\r
  98 >  2 files changed, 60 insertions(+), 1 deletion(-)\r
  99 >\r
 100 > diff --git a/notmuch-tag.c b/notmuch-tag.c\r
 101 > index 8a8af0b..e4fca67 100644\r
 102 > --- a/notmuch-tag.c\r
 103 > +++ b/notmuch-tag.c\r
 104 > @@ -140,7 +140,7 @@ tag_query (void *ctx, notmuch_database_t *notmuch, const char *query_string,\r
 105 >        notmuch_messages_valid (messages) && ! interrupted;\r
 106 >        notmuch_messages_move_to_next (messages)) {\r
 107 >       message = notmuch_messages_get (messages);\r
 108 > -     tag_op_list_apply (message, tag_ops, flags);\r
 109 > +     tag_op_list_apply (message, tag_ops, flags | TAG_FLAG_PRE_OPTIMIZED);\r
 110 >       notmuch_message_destroy (message);\r
 111 >      }\r
 112 >  \r
 113 > diff --git a/tag-util.c b/tag-util.c\r
 114 > index 287cc67..2bb8355 100644\r
 115 > --- a/tag-util.c\r
 116 > +++ b/tag-util.c\r
 117 > @@ -111,6 +111,62 @@ message_error (notmuch_message_t *message,\r
 118 >      fprintf (stderr, "Status: %s\n", notmuch_status_to_string (status));\r
 119 >  }\r
 120 >  \r
 121 > +static int\r
 122 > +makes_changes (notmuch_message_t *message,\r
 123 > +            tag_op_list_t *list,\r
 124 > +            tag_op_flag_t flags)\r
 125 > +{\r
 126 > +\r
 127 > +    int i;\r
 128 > +\r
 129 > +    notmuch_tags_t *tags;\r
 130 > +    notmuch_bool_t changes = FALSE;\r
 131 > +\r
 132 > +    /* First, do we delete an existing tag? */\r
 133 > +    changes = FALSE;\r
 134 > +    for (tags = notmuch_message_get_tags (message);\r
 135 > +      ! changes && notmuch_tags_valid (tags);\r
 136 > +      notmuch_tags_move_to_next (tags)) {\r
 137 > +     const char *cur_tag = notmuch_tags_get (tags);\r
 138 > +     int last_op =  (flags & TAG_FLAG_REMOVE_ALL) ? -1 : 0;\r
 139 > +\r
 140 > +     for (i = 0; i < list->count; i++) {\r
 141 > +         if (strcmp (cur_tag, list->ops[i].tag) == 0) {\r
 142 > +             last_op = list->ops[i].remove ? -1 : 1;\r
 143 > +         }\r
 144 > +     }\r
 145 \r
 146 If you looped from end to beginning, you could break on first match.\r
 147 \r
 148 > +\r
 149 > +     changes = (last_op == -1);\r
 150 > +    }\r
 151 > +    notmuch_tags_destroy (tags);\r
 152 > +\r
 153 > +    if (changes)\r
 154 > +     return TRUE;\r
 155 > +\r
 156 > +    /* Now check for adding new tags */\r
 157 > +    for (i = 0; i < list->count; i++) {\r
 158 > +     notmuch_bool_t exists = FALSE;\r
 159 > +\r
 160 \r
 161 I think you can do:\r
 162 \r
 163         if (list->ops[i].remove)\r
 164             continue;\r
 165 \r
 166 here and remove the check on .remove below.\r
 167 \r
 168 > +     for (tags = notmuch_message_get_tags (message);\r
 169 > +          notmuch_tags_valid (tags);\r
 170 > +          notmuch_tags_move_to_next (tags)) {\r
 171 > +         const char *cur_tag = notmuch_tags_get (tags);\r
 172 > +         if (strcmp (cur_tag, list->ops[i].tag) == 0) {\r
 173 > +             exists = TRUE;\r
 174 > +             break;\r
 175 > +         }\r
 176 > +     }\r
 177 > +     notmuch_tags_destroy (tags);\r
 178 > +\r
 179 > +     /* the following test is conservative, it's ok to think we\r
 180 > +      * make changes when we don't */\r
 181 \r
 182 I think it's "too" conservative only when the input has e.g. "+foo\r
 183 -foo", but I wouldn't worry about optimizing that.\r
 184 \r
 185 > +     if ( ! exists && ! list->ops[i].remove )\r
 186 > +         return TRUE;\r
 187 > +    }\r
 188 > +    return FALSE;\r
 189 > +\r
 190 > +}\r
 191 > +\r
 192 >  notmuch_status_t\r
 193 >  tag_op_list_apply (notmuch_message_t *message,\r
 194 >                  tag_op_list_t *list,\r
 195 > @@ -121,6 +177,9 @@ tag_op_list_apply (notmuch_message_t *message,\r
 196 >      notmuch_status_t status = 0;\r
 197 >      tag_operation_t *tag_ops = list->ops;\r
 198 >  \r
 199 > +    if (! (flags & TAG_FLAG_PRE_OPTIMIZED) && ! makes_changes (message, list, flags))\r
 200 > +     return NOTMUCH_STATUS_SUCCESS;\r
 201 > +\r
 202 >      status = notmuch_message_freeze (message);\r
 203 >      if (status) {\r
 204 >       message_error (message, status, "freezing message");\r
 205 > -- \r
 206 > 1.7.10.4\r
 207 >\r
 208 > _______________________________________________\r
 209 > notmuch mailing list\r
 210 > notmuch@notmuchmail.org\r
 211 > http://notmuchmail.org/mailman/listinfo/notmuch\r