Re: [PATCH] tag: Automatically limit to messages whose tags will actually change.
authorJani Nikula <jani@nikula.org>
Wed, 9 Nov 2011 08:46:02 +0000 (08:46 +0000)
committerW. Trevor King <wking@tremily.us>
Fri, 7 Nov 2014 17:40:02 +0000 (09:40 -0800)
2f/062ccea49480ba30281b6ea64f0e570da294da [new file with mode: 0644]

diff --git a/2f/062ccea49480ba30281b6ea64f0e570da294da b/2f/062ccea49480ba30281b6ea64f0e570da294da
new file mode 100644 (file)
index 0000000..f00dbf2
--- /dev/null
@@ -0,0 +1,219 @@
+Return-Path: <jani@nikula.org>\r
+X-Original-To: notmuch@notmuchmail.org\r
+Delivered-To: notmuch@notmuchmail.org\r
+Received: from localhost (localhost [127.0.0.1])\r
+       by olra.theworths.org (Postfix) with ESMTP id B73B7431FD0\r
+       for <notmuch@notmuchmail.org>; Wed,  9 Nov 2011 00:46:10 -0800 (PST)\r
+X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
+X-Spam-Flag: NO\r
+X-Spam-Score: -0.7\r
+X-Spam-Level: \r
+X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5\r
+       tests=[RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled\r
+Received: from olra.theworths.org ([127.0.0.1])\r
+       by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
+       with ESMTP id A-XrQYTBeypi for <notmuch@notmuchmail.org>;\r
+       Wed,  9 Nov 2011 00:46:08 -0800 (PST)\r
+Received: from mail-vx0-f181.google.com (mail-vx0-f181.google.com\r
+       [209.85.220.181]) (using TLSv1 with cipher RC4-SHA (128/128 bits))\r
+       (No client certificate requested)\r
+       by olra.theworths.org (Postfix) with ESMTPS id 7F388431FB6\r
+       for <notmuch@notmuchmail.org>; Wed,  9 Nov 2011 00:46:08 -0800 (PST)\r
+Received: by vcbfk26 with SMTP id fk26so1473768vcb.26\r
+       for <notmuch@notmuchmail.org>; Wed, 09 Nov 2011 00:46:06 -0800 (PST)\r
+Received: by 10.52.26.9 with SMTP id h9mr2658719vdg.99.1320828366706;\r
+       Wed, 09 Nov 2011 00:46:06 -0800 (PST)\r
+Received: from localhost (nikula.org. [92.243.24.172])\r
+       by mx.google.com with ESMTPS id bl9sm6412627vdb.15.2011.11.09.00.46.03\r
+       (version=SSLv3 cipher=OTHER); Wed, 09 Nov 2011 00:46:04 -0800 (PST)\r
+From: Jani Nikula <jani@nikula.org>\r
+To: Austin Clements <amdragon@MIT.EDU>, notmuch@notmuchmail.org\r
+Subject: Re: [PATCH] tag: Automatically limit to messages whose tags will\r
+       actually change.\r
+In-Reply-To: <1320724523-23568-1-git-send-email-amdragon@mit.edu>\r
+References: <1320724523-23568-1-git-send-email-amdragon@mit.edu>\r
+User-Agent: Notmuch/0.5-232-g917e874 (http://notmuchmail.org) Emacs/23.1.1\r
+       (i686-pc-linux-gnu)\r
+Date: Wed, 09 Nov 2011 08:46:02 +0000\r
+Message-ID: <87ty6d1y5x.fsf@nikula.org>\r
+MIME-Version: 1.0\r
+Content-Type: text/plain; charset=us-ascii\r
+X-BeenThere: notmuch@notmuchmail.org\r
+X-Mailman-Version: 2.1.13\r
+Precedence: list\r
+List-Id: "Use and development of the notmuch mail system."\r
+       <notmuch.notmuchmail.org>\r
+List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
+       <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
+List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
+List-Post: <mailto:notmuch@notmuchmail.org>\r
+List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
+List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
+       <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
+X-List-Received-Date: Wed, 09 Nov 2011 08:46:10 -0000\r
+\r
+\r
+FWIW, I reviewed this and didn't find any obvious problems. A few\r
+nitpicks below, though.\r
+\r
+BR,\r
+Jani.\r
+\r
+On Mon,  7 Nov 2011 22:55:23 -0500, Austin Clements <amdragon@MIT.EDU> wrote:\r
+> This optimizes the user's tagging query to exclude messages that won't\r
+> be affected by the tagging operation, saving computation and IO for\r
+> redundant tagging operations.\r
+> \r
+> For example,\r
+>   notmuch tag +notmuch to:notmuch@notmuchmail.org\r
+> will now use the query\r
+>   ( to:notmuch@notmuchmail.org ) and (not tag:"notmuch")\r
+> \r
+> In the past, we've often suggested that people do this exact\r
+> transformation by hand for slow tagging operations.  This makes that\r
+> unnecessary.\r
+> ---\r
+> I was about to implement this optimization in my initial tagging\r
+> script, but then I figured, why not just do it in notmuch so we can\r
+> stop telling people to do this by hand?\r
+> \r
+>  NEWS          |    9 ++++++\r
+>  notmuch-tag.c |   76 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++\r
+>  2 files changed, 85 insertions(+), 0 deletions(-)\r
+> \r
+> diff --git a/NEWS b/NEWS\r
+> index e00452a..9ca5e0c 100644\r
+> --- a/NEWS\r
+> +++ b/NEWS\r
+> @@ -16,6 +16,15 @@ Add search terms to  "notmuch dump"\r
+>    search/show/tag. The output file argument of dump is deprecated in\r
+>    favour of using stdout.\r
+>  \r
+> +Optimizations\r
+> +-------------\r
+> +\r
+> +Automatic tag query optimization\r
+> +\r
+> +  "notmuch tag" now automatically optimizes the user's query to\r
+> +  exclude messages whose tags won't change.  In the past, we've\r
+> +  suggested that people do this by hand; this is no longer necessary.\r
+> +\r
+>  Notmuch 0.9 (2011-10-01)\r
+>  ========================\r
+>  \r
+> diff --git a/notmuch-tag.c b/notmuch-tag.c\r
+> index dded39e..62c4bf1 100644\r
+> --- a/notmuch-tag.c\r
+> +++ b/notmuch-tag.c\r
+> @@ -30,6 +30,76 @@ handle_sigint (unused (int sig))\r
+>      interrupted = 1;\r
+>  }\r
+>  \r
+> +static char *\r
+> +_escape_tag (char *buf, const char *tag)\r
+> +{\r
+> +    const char *in = tag;\r
+> +    char *out = buf;\r
+> +    /* Boolean terms surrounded by double quotes can contain any\r
+> +     * character.  Double quotes are quoted by doubling them. */\r
+> +    *(out++) = '"';\r
+> +    while (*in) {\r
+> +    if (*in == '"')\r
+> +        *(out++) = '"';\r
+> +    *(out++) = *(in++);\r
+> +    }\r
+> +    *(out++) = '"';\r
+\r
+The parenthesis are unnecessary for *p++.\r
+\r
+> +    *out = 0;\r
+> +    return buf;\r
+> +}\r
+> +\r
+> +static char *\r
+> +_optimize_tag_query (void *ctx, const char *orig_query_string, char *argv[],\r
+> +                 int *add_tags, int add_tags_count,\r
+> +                 int *remove_tags, int remove_tags_count)\r
+> +{\r
+> +    /* This is subtler than it looks.  Xapian ignores the '-' operator\r
+> +     * at the beginning both queries and parenthesized groups and,\r
+> +     * furthermore, the presence of a '-' operator at the beginning of\r
+> +     * a group can inhibit parsing of the previous operator.  Hence,\r
+> +     * the user-provided query MUST appear first, but it is safe to\r
+> +     * parenthesize and the exclusion part of the query must not use\r
+> +     * the '-' operator (though the NOT operator is fine). */\r
+> +\r
+> +    char *escaped, *query_string;\r
+> +    const char *join = "";\r
+> +    int i;\r
+> +    unsigned int max_tag_len = 0;\r
+> +\r
+> +    /* Allocate a buffer for escaping tags. */\r
+> +    for (i = 0; i < add_tags_count; i++)\r
+> +    if (strlen (argv[add_tags[i]] + 1) > max_tag_len)\r
+> +        max_tag_len = strlen (argv[add_tags[i]] + 1);\r
+> +    for (i = 0; i < remove_tags_count; i++)\r
+> +    if (strlen (argv[remove_tags[i]] + 1) > max_tag_len)\r
+> +        max_tag_len = strlen (argv[remove_tags[i]] + 1);\r
+> +    escaped = talloc_array(ctx, char, max_tag_len * 2 + 3);\r
+\r
+Perhaps a comment here or above _escape_tag() explaining the worst case\r
+memory consumption of strlen(tag) * 2 + 3 for a tag of "s would be in\r
+order.\r
+\r
+It's unrelated, but looking at the above also made me check something\r
+I've suspected before: notmuch allows you to have empty or zero length\r
+tags "", which is probably not intentional.\r
+\r
+There's no check for talloc failures here or below. But then there are\r
+few checks for that in the cli in general. *shrug*.\r
+\r
+> +\r
+> +    /* Build the new query string */\r
+> +    if (strcmp (orig_query_string, "*") == 0)\r
+> +    query_string = talloc_strdup (ctx, "(");\r
+> +    else\r
+> +    query_string = talloc_asprintf (ctx, "( %s ) and (", orig_query_string);\r
+> +\r
+> +    for (i = 0; i < add_tags_count; i++) {\r
+> +    query_string = talloc_asprintf_append_buffer (\r
+> +        query_string, "%snot tag:%s", join,\r
+> +        _escape_tag (escaped, argv[add_tags[i]] + 1));\r
+> +    join = " or ";\r
+> +    }\r
+> +    for (i = 0; i < remove_tags_count; i++) {\r
+> +    query_string = talloc_asprintf_append_buffer (\r
+> +        query_string, "%stag:%s", join,\r
+> +        _escape_tag (escaped, argv[remove_tags[i]] + 1));\r
+> +    join = " or ";\r
+> +    }\r
+> +\r
+> +    query_string = talloc_strdup_append_buffer (query_string, ")");\r
+> +\r
+> +    talloc_free (escaped);\r
+> +    return query_string;\r
+> +}\r
+> +\r
+>  int\r
+>  notmuch_tag_command (void *ctx, unused (int argc), unused (char *argv[]))\r
+>  {\r
+> @@ -93,6 +163,12 @@ notmuch_tag_command (void *ctx, unused (int argc), unused (char *argv[]))\r
+>      return 1;\r
+>      }\r
+>  \r
+> +    /* Optimize the query so it excludes messages that already have\r
+> +     * the specified set of tags. */\r
+> +    query_string = _optimize_tag_query (ctx, query_string, argv,\r
+> +                                    add_tags, add_tags_count,\r
+> +                                    remove_tags, remove_tags_count);\r
+> +\r
+>      config = notmuch_config_open (ctx, NULL, NULL);\r
+>      if (config == NULL)\r
+>      return 1;\r
+> -- \r
+> 1.7.7.1\r
+> \r
+> _______________________________________________\r
+> notmuch mailing list\r
+> notmuch@notmuchmail.org\r
+> http://notmuchmail.org/mailman/listinfo/notmuch\r