92/c6f98846b08965c26cc627f04f1a00b1c16fcc

   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 EC828431FAF\r
   6         for <notmuch@notmuchmail.org>; Fri,  7 Dec 2012 15:08:09 -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 bGWztGmn5j8s for <notmuch@notmuchmail.org>;\r
  16         Fri,  7 Dec 2012 15:08:09 -0800 (PST)\r
  17 Received: from mail-la0-f53.google.com (mail-la0-f53.google.com\r
  18         [209.85.215.53]) (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 1AAD2431FAE\r
  21         for <notmuch@notmuchmail.org>; Fri,  7 Dec 2012 15:08:08 -0800 (PST)\r
  22 Received: by mail-la0-f53.google.com with SMTP id w12so759565lag.26\r
  23         for <notmuch@notmuchmail.org>; Fri, 07 Dec 2012 15:08:07 -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=W3GtL9C/N4jEo4hdJ5LuhelJ4ShHs4BfkjWyTZIvI8E=;\r
  29         b=VGCGNtR93kJmRW6nmw3NrzhBhMdxQSNPNNAF72Q/IzN5QRlQP9lHxhg+vLlh3encha\r
  30         pccsvvUmYXSDlLstTaJDYs3iyr1llq61VZsxM2P2oULO4HLBBVzB+0ysKM4/n3tskHhx\r
  31         TWVpfjNs1UQrlgQ/FzaOS9nTLTGoBzIgVU2tXrbfbHZgrv5v0jsccp0pm6YfLst06gqK\r
  32         arG3jqeJkeNSV/AM9mv5pepMqeEqSsvKwPRSoraMSYcLmc6h4YeIVAKlcn/zVYfYFNmc\r
  33         2PNr4Q6m1AQAfvwOUDGcUZIxGeJjKW5lFnFlOZJ/r9ab+y55/jeyqRTddwV/08Yhee31\r
  34         4rdA==\r
  35 Received: by 10.152.122.133 with SMTP id ls5mr7068383lab.9.1354921687320;\r
  36         Fri, 07 Dec 2012 15:08:07 -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 pw17sm5111006lab.5.2012.12.07.15.08.05\r
  40         (version=SSLv3 cipher=OTHER); Fri, 07 Dec 2012 15:08:06 -0800 (PST)\r
  41 From: Jani Nikula <jani@nikula.org>\r
  42 To: david@tethera.net, notmuch@notmuchmail.org\r
  43 Subject: Re: [Patch v3b 9/9] tag-util: optimization of tag application\r
  44 In-Reply-To: <1354843607-17980-10-git-send-email-david@tethera.net>\r
  45 References: <1354843607-17980-1-git-send-email-david@tethera.net>\r
  46         <1354843607-17980-10-git-send-email-david@tethera.net>\r
  47 User-Agent: Notmuch/0.14+138~g7041c56 (http://notmuchmail.org) Emacs/23.4.1\r
  48         (i686-pc-linux-gnu)\r
  49 Date: Sat, 08 Dec 2012 01:08:04 +0200\r
  50 Message-ID: <87lid9xxyj.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  ALoCoQkvdwbESM9NtmeIMe7OgETTO3ZRPHpZ32rwp2oNRFo9TV4CrOsch+70qaNRIhLEDjmGd8O/\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: Fri, 07 Dec 2012 23:08:10 -0000\r
  69 \r
  70 On Fri, 07 Dec 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 >  tag-util.c |   66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\r
  81 >  1 file changed, 66 insertions(+)\r
  82 >\r
  83 > diff --git a/tag-util.c b/tag-util.c\r
  84 > index 932ee7f..3d54e9e 100644\r
  85 > --- a/tag-util.c\r
  86 > +++ b/tag-util.c\r
  87 > @@ -124,6 +124,69 @@ message_error (notmuch_message_t *message,\r
  88 >      fprintf (stderr, "Status: %s\n", notmuch_status_to_string (status));\r
  89 >  }\r
  90 >  \r
  91 > +static int\r
  92 > +makes_changes (notmuch_message_t *message,\r
  93 > +            tag_op_list_t *list,\r
  94 > +            tag_op_flag_t flags)\r
  95 > +{\r
  96 > +\r
  97 > +    size_t i;\r
  98 > +\r
  99 > +    notmuch_tags_t *tags;\r
 100 > +    notmuch_bool_t changes = FALSE;\r
 101 > +\r
 102 > +    /* First, do we delete an existing tag? */\r
 103 > +    changes = FALSE;\r
 104 > +    for (tags = notmuch_message_get_tags (message);\r
 105 > +      ! changes && notmuch_tags_valid (tags);\r
 106 > +      notmuch_tags_move_to_next (tags)) {\r
 107 > +     const char *cur_tag = notmuch_tags_get (tags);\r
 108 > +     int last_op =  (flags & TAG_FLAG_REMOVE_ALL) ? -1 : 0;\r
 109 > +\r
 110 > +     /* slight contortions to count down with an unsigned index */\r
 111 > +     for (i = list->count; i-- > 0; /*nothing*/) {\r
 112 > +         if (strcmp (cur_tag, list->ops[i].tag) == 0) {\r
 113 > +             last_op = list->ops[i].remove ? -1 : 1;\r
 114 > +             break;\r
 115 > +         }\r
 116 > +     }\r
 117 \r
 118 After some eyeballing it looks correct, but not not exactly pretty. If\r
 119 you insist on unsigned, you could also have a regular (i = 0; i <\r
 120 list->count; i++) and use j = list->count - i - 1; within the block. But\r
 121 that's just style bikeshedding after convincing you to use a count down\r
 122 loop in the first place...\r
 123 \r
 124 Otherwise, the patch LGTM.\r
 125 \r
 126 \r
 127 > +\r
 128 > +     changes = (last_op == -1);\r
 129 > +    }\r
 130 > +    notmuch_tags_destroy (tags);\r
 131 > +\r
 132 > +    if (changes)\r
 133 > +     return TRUE;\r
 134 > +\r
 135 > +    /* Now check for adding new tags */\r
 136 > +    for (i = 0; i < list->count; i++) {\r
 137 > +     notmuch_bool_t exists = FALSE;\r
 138 > +\r
 139 > +     if (list->ops[i].remove)\r
 140 > +         continue;\r
 141 > +\r
 142 > +     for (tags = notmuch_message_get_tags (message);\r
 143 > +          notmuch_tags_valid (tags);\r
 144 > +          notmuch_tags_move_to_next (tags)) {\r
 145 > +         const char *cur_tag = notmuch_tags_get (tags);\r
 146 > +         if (strcmp (cur_tag, list->ops[i].tag) == 0) {\r
 147 > +             exists = TRUE;\r
 148 > +             break;\r
 149 > +         }\r
 150 > +     }\r
 151 > +     notmuch_tags_destroy (tags);\r
 152 > +\r
 153 > +     /* the following test is conservative,\r
 154 > +      * in the sense it ignores cases like +foo ... -foo\r
 155 > +      * but this is OK from a correctness point of view\r
 156 > +      */\r
 157 > +     if (! exists)\r
 158 > +         return TRUE;\r
 159 > +    }\r
 160 > +    return FALSE;\r
 161 > +\r
 162 > +}\r
 163 > +\r
 164 >  notmuch_status_t\r
 165 >  tag_op_list_apply (notmuch_message_t *message,\r
 166 >                  tag_op_list_t *list,\r
 167 > @@ -133,6 +196,9 @@ tag_op_list_apply (notmuch_message_t *message,\r
 168 >      notmuch_status_t status = 0;\r
 169 >      tag_operation_t *tag_ops = list->ops;\r
 170 >  \r
 171 > +    if (! (flags & TAG_FLAG_PRE_OPTIMIZED) && ! makes_changes (message, list, flags))\r
 172 > +     return NOTMUCH_STATUS_SUCCESS;\r
 173 > +\r
 174 >      status = notmuch_message_freeze (message);\r
 175 >      if (status) {\r
 176 >       message_error (message, status, "freezing message");\r
 177 > -- \r
 178 > 1.7.10.4\r
 179 >\r
 180 > _______________________________________________\r
 181 > notmuch mailing list\r
 182 > notmuch@notmuchmail.org\r
 183 > http://notmuchmail.org/mailman/listinfo/notmuch\r