[PATCH v2 0/6] batch count for notmuch-hello speedup

[PATCH v2 0/6] batch count for notmuch-hello speedup

[Jani Nikula]

Rebased v2 of id:cover.1358273133.git.jani@nikula.org

Jani Nikula (5):
  cli: remove useless strdup
  cli: extract count printing to a separate function in notmuch count
  cli: add --batch option to notmuch count
  man: document notmuch count --batch and --input options
  test: notmuch count --batch and --input options

Mark Walters (1):
  emacs: hello: use batch count

 emacs/notmuch-hello.el   |   52 +++++++++++++---------
 man/man1/notmuch-count.1 |   20 +++++++++
 notmuch-count.c          |  111 +++++++++++++++++++++++++++++++++++-----------
 test/count               |   46 +++++++++++++++++++
 4 files changed, 182 insertions(+), 47 deletions(-)

-- 
1.7.10.4

[PATCH v2 1/6] cli: remove useless strdup

[Jani Nikula]

---
 notmuch-count.c |    4 ----
 1 file changed, 4 deletions(-)

diff --git a/notmuch-count.c b/notmuch-count.c
index 390794f..c2f1b7d 100644
--- a/notmuch-count.c
+++ b/notmuch-count.c
@@ -71,10 +71,6 @@ notmuch_count_command (notmuch_config_t *config, int argc, char *argv[])
 	return 1;
     }
 
-    if (*query_str == '\0') {
-	query_str = talloc_strdup (config, "");
-    }
-
     query = notmuch_query_create (notmuch, query_str);
     if (query == NULL) {
 	fprintf (stderr, "Out of memory\n");
-- 
1.7.10.4

[PATCH v2 2/6] cli: extract count printing to a separate function in notmuch count

[Jani Nikula]

Make count printing on a query string reusable. No functional changes.
---
 notmuch-count.c |   59 +++++++++++++++++++++++++++++++++----------------------
 1 file changed, 36 insertions(+), 23 deletions(-)

diff --git a/notmuch-count.c b/notmuch-count.c
index c2f1b7d..630f036 100644
--- a/notmuch-count.c
+++ b/notmuch-count.c
@@ -32,16 +32,47 @@ enum {
     EXCLUDE_FALSE,
 };
 
+static int
+print_count (notmuch_database_t *notmuch, const char *query_str,
+	     const char **exclude_tags, size_t exclude_tags_length, int output)
+{
+    notmuch_query_t *query;
+    size_t i;
+
+    query = notmuch_query_create (notmuch, query_str);
+    if (query == NULL) {
+	fprintf (stderr, "Out of memory\n");
+	return 1;
+    }
+
+    for (i = 0; i < exclude_tags_length; i++)
+	notmuch_query_add_tag_exclude (query, exclude_tags[i]);
+
+    switch (output) {
+    case OUTPUT_MESSAGES:
+	printf ("%u\n", notmuch_query_count_messages (query));
+	break;
+    case OUTPUT_THREADS:
+	printf ("%u\n", notmuch_query_count_threads (query));
+	break;
+    }
+
+    notmuch_query_destroy (query);
+
+    return 0;
+}
+
 int
 notmuch_count_command (notmuch_config_t *config, int argc, char *argv[])
 {
     notmuch_database_t *notmuch;
-    notmuch_query_t *query;
     char *query_str;
     int opt_index;
     int output = OUTPUT_MESSAGES;
     int exclude = EXCLUDE_TRUE;
-    unsigned int i;
+    const char **search_exclude_tags = NULL;
+    size_t search_exclude_tags_length = 0;
+    int ret;
 
     notmuch_opt_desc_t options[] = {
 	{ NOTMUCH_OPT_KEYWORD, &output, "output", 'o',
@@ -71,33 +102,15 @@ notmuch_count_command (notmuch_config_t *config, int argc, char *argv[])
 	return 1;
     }
 
-    query = notmuch_query_create (notmuch, query_str);
-    if (query == NULL) {
-	fprintf (stderr, "Out of memory\n");
-	return 1;
-    }
-
     if (exclude == EXCLUDE_TRUE) {
-	const char **search_exclude_tags;
-	size_t search_exclude_tags_length;
-
 	search_exclude_tags = notmuch_config_get_search_exclude_tags
 	    (config, &search_exclude_tags_length);
-	for (i = 0; i < search_exclude_tags_length; i++)
-	    notmuch_query_add_tag_exclude (query, search_exclude_tags[i]);
     }
 
-    switch (output) {
-    case OUTPUT_MESSAGES:
-	printf ("%u\n", notmuch_query_count_messages (query));
-	break;
-    case OUTPUT_THREADS:
-	printf ("%u\n", notmuch_query_count_threads (query));
-	break;
-    }
+    ret = print_count (notmuch, query_str, search_exclude_tags,
+		       search_exclude_tags_length, output);
 
-    notmuch_query_destroy (query);
     notmuch_database_destroy (notmuch);
 
-    return 0;
+    return ret;
 }
-- 
1.7.10.4

[PATCH v2 3/6] cli: add --batch option to notmuch count

[Jani Nikula]

Add support for reading queries from stdin, one per line, and writing
results to stdin, one per line.

This will bring considerable performance improvements when utilized in
Emacs notmuch-hello, especially so when running remote notmuch.
---
 notmuch-count.c |   52 ++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 50 insertions(+), 2 deletions(-)

diff --git a/notmuch-count.c b/notmuch-count.c
index 630f036..8772cff 100644
--- a/notmuch-count.c
+++ b/notmuch-count.c
@@ -62,6 +62,27 @@ print_count (notmuch_database_t *notmuch, const char *query_str,
     return 0;
 }
 
+static int
+count_file (notmuch_database_t *notmuch, FILE *input, const char **exclude_tags,
+	    size_t exclude_tags_length, int output)
+{
+    char *line = NULL;
+    ssize_t line_len;
+    size_t line_size;
+    int ret = 0;
+
+    while (!ret && (line_len = getline (&line, &line_size, input)) != -1) {
+	chomp_newline (line);
+	ret = print_count (notmuch, line, exclude_tags, exclude_tags_length,
+			   output);
+    }
+
+    if (line)
+	free (line);
+
+    return ret;
+}
+
 int
 notmuch_count_command (notmuch_config_t *config, int argc, char *argv[])
 {
@@ -72,6 +93,9 @@ notmuch_count_command (notmuch_config_t *config, int argc, char *argv[])
     int exclude = EXCLUDE_TRUE;
     const char **search_exclude_tags = NULL;
     size_t search_exclude_tags_length = 0;
+    notmuch_bool_t batch = FALSE;
+    FILE *input = stdin;
+    char *input_file_name = NULL;
     int ret;
 
     notmuch_opt_desc_t options[] = {
@@ -83,6 +107,8 @@ notmuch_count_command (notmuch_config_t *config, int argc, char *argv[])
 	  (notmuch_keyword_t []){ { "true", EXCLUDE_TRUE },
 				  { "false", EXCLUDE_FALSE },
 				  { 0, 0 } } },
+	{ NOTMUCH_OPT_BOOLEAN, &batch, "batch", 0, 0 },
+	{ NOTMUCH_OPT_STRING, &input_file_name, "input", 'i', 0 },
 	{ 0, 0, 0, 0, 0 }
     };
 
@@ -92,6 +118,21 @@ notmuch_count_command (notmuch_config_t *config, int argc, char *argv[])
 	return 1;
     }
 
+    if (input_file_name) {
+	batch = TRUE;
+	input = fopen (input_file_name, "r");
+	if (input == NULL) {
+	    fprintf (stderr, "Error opening %s for reading: %s\n",
+		     input_file_name, strerror (errno));
+	    return 1;
+	}
+    }
+
+    if (batch && opt_index != argc) {
+	fprintf (stderr, "--batch and query string are not compatible\n");
+	return 1;
+    }
+
     if (notmuch_database_open (notmuch_config_get_database_path (config),
 			       NOTMUCH_DATABASE_MODE_READ_ONLY, &notmuch))
 	return 1;
@@ -107,10 +148,17 @@ notmuch_count_command (notmuch_config_t *config, int argc, char *argv[])
 	    (config, &search_exclude_tags_length);
     }
 
-    ret = print_count (notmuch, query_str, search_exclude_tags,
-		       search_exclude_tags_length, output);
+    if (batch)
+	ret = count_file (notmuch, input, search_exclude_tags,
+			  search_exclude_tags_length, output);
+    else
+	ret = print_count (notmuch, query_str, search_exclude_tags,
+			   search_exclude_tags_length, output);
 
     notmuch_database_destroy (notmuch);
 
+    if (input != stdin)
+	fclose (input);
+
     return ret;
 }
-- 
1.7.10.4

Re: [PATCH v2 3/6] cli: add --batch option to notmuch count

[Jameson Graef Rollins]

[-- Attachment #1: Type: text/plain, Size: 215 bytes --]

On Sat, Mar 09 2013, Jani Nikula <jani@nikula.org> wrote:
> Add support for reading queries from stdin, one per line, and writing
> results to stdin, one per line.

I assume this is meant to be "to stdout".

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: [PATCH v2 3/6] cli: add --batch option to notmuch count

[Jani Nikula]

On Thu, 21 Mar 2013, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> On Sat, Mar 09 2013, Jani Nikula <jani@nikula.org> wrote:
>> Add support for reading queries from stdin, one per line, and writing
>> results to stdin, one per line.
>
> I assume this is meant to be "to stdout".

Oops, I got that comment the last time too. :(

BR,
Jani.

Re: [PATCH v2 3/6] cli: add --batch option to notmuch count

[Jani Nikula]

On Tue, 26 Mar 2013, Jani Nikula <jani@nikula.org> wrote:
> On Thu, 21 Mar 2013, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
>> On Sat, Mar 09 2013, Jani Nikula <jani@nikula.org> wrote:
>>> Add support for reading queries from stdin, one per line, and writing
>>> results to stdin, one per line.
>>
>> I assume this is meant to be "to stdout".
>
> Oops, I got that comment the last time too. :(

BTW just to be clear, I'm not sending another version of this patch
(series) just to fix this mistake; I'll wait for more review
comments. (And if all looks good and David is in a good mood, perhaps he
can tweak the commit message. ;)

BR,
Jani.

[PATCH v2 4/6] man: document notmuch count --batch and --input options

[Jani Nikula]

---
 man/man1/notmuch-count.1 |   20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/man/man1/notmuch-count.1 b/man/man1/notmuch-count.1
index 86a67fe..7fc4378 100644
--- a/man/man1/notmuch-count.1
+++ b/man/man1/notmuch-count.1
@@ -46,6 +46,26 @@ Output the number of matching threads.
 Specify whether to omit messages matching search.tag_exclude from the
 count (the default) or not.
 .RE
+
+.RS 4
+.TP 4
+.BR \-\-batch
+
+Read queries from a file (stdin by default), one per line, and output
+the number of matching messages (or threads) to stdout, one per
+line. On an empty input line the count of all messages (or threads) in
+the database will be output. This option is not compatible with
+specifying search terms on the command line.
+.RE
+
+.RS 4
+.TP 4
+.BR "\-\-input=" <filename>
+
+Read input from given file, instead of from stdin. Implies
+.BR --batch .
+.RE
+
 .RE
 .RE
 
-- 
1.7.10.4

[PATCH v2 5/6] test: notmuch count --batch and --input options

[Jani Nikula]

---
 test/count |   46 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/test/count b/test/count
index 879b114..05713fd 100755
--- a/test/count
+++ b/test/count
@@ -38,4 +38,50 @@ test_expect_equal \
     "0" \
     "`notmuch count --output=threads from:cworth and not from:cworth`"
 
+test_begin_subtest "message count is the default for batch count"
+notmuch count --batch >OUTPUT <<EOF
+
+from:cworth
+EOF
+notmuch count --output=messages >EXPECTED
+notmuch count --output=messages from:cworth >>EXPECTED
+test_expect_equal_file EXPECTED OUTPUT
+
+test_begin_subtest "batch message count"
+notmuch count --batch --output=messages >OUTPUT <<EOF
+from:cworth
+
+tag:inbox
+EOF
+notmuch count --output=messages from:cworth >EXPECTED
+notmuch count --output=messages >>EXPECTED
+notmuch count --output=messages tag:inbox >>EXPECTED
+test_expect_equal_file EXPECTED OUTPUT
+
+test_begin_subtest "batch thread count"
+notmuch count --batch --output=threads >OUTPUT <<EOF
+
+from:cworth
+from:cworth and not from:cworth
+foo
+EOF
+notmuch count --output=threads >EXPECTED
+notmuch count --output=threads from:cworth >>EXPECTED
+notmuch count --output=threads from:cworth and not from:cworth >>EXPECTED
+notmuch count --output=threads foo >>EXPECTED
+test_expect_equal_file EXPECTED OUTPUT
+
+test_begin_subtest "batch message count with input file"
+cat >INPUT <<EOF
+from:cworth
+
+tag:inbox
+EOF
+notmuch count --input=INPUT --output=messages >OUTPUT
+notmuch count --output=messages from:cworth >EXPECTED
+notmuch count --output=messages >>EXPECTED
+notmuch count --output=messages tag:inbox >>EXPECTED
+test_expect_equal_file EXPECTED OUTPUT
+
+
 test_done
-- 
1.7.10.4

[PATCH v2 6/6] emacs: hello: use batch count

[Jani Nikula]

From: Mark Walters <markwalters1009@gmail.com>

This modifies notmuch hello to use the new count --batch
functionality. It should give exactly the same results as before but
under many conditions it should be much faster. In particular it is
much faster for remote use.

The code is a little ugly as it has to do some working out of the
query when asking the query and some when dealing with the result.
However, the code path is exactly the same in both local and remote
use.
---
 emacs/notmuch-hello.el |   52 +++++++++++++++++++++++++++++-------------------
 1 file changed, 32 insertions(+), 20 deletions(-)

diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el
index 00b78e1..cda79f1 100644
--- a/emacs/notmuch-hello.el
+++ b/emacs/notmuch-hello.el
@@ -381,26 +381,38 @@ The result is the list of elements of the form (NAME QUERY COUNT).
 The values :show-empty-searches, :filter and :filter-count from
 options will be handled as specified for
 `notmuch-hello-insert-searches'."
-  (notmuch-remove-if-not
-   #'identity
-   (mapcar
-    (lambda (elem)
-      (let* ((name (car elem))
-	     (query-and-count (if (consp (cdr elem))
-				  ;; do we have a different query for the message count?
-				  (cons (second elem) (third elem))
-				(cons (cdr elem) (cdr elem))))
-	     (message-count
-	      (string-to-number
-	       (notmuch-saved-search-count
-		(notmuch-hello-filtered-query (cdr query-and-count)
-					      (or (plist-get options :filter-count)
-						 (plist-get options :filter)))))))
-	(and (or (plist-get options :show-empty-searches) (> message-count 0))
-	     (list name (notmuch-hello-filtered-query
-			 (car query-and-count) (plist-get options :filter))
-		   message-count))))
-    query-alist)))
+  (with-temp-buffer
+    (dolist (elem query-alist nil)
+      (let ((count-query (if (consp (cdr elem))
+			     ;; do we have a different query for the message count?
+			     (third elem)
+			   (cdr elem))))
+	(insert
+	 (notmuch-hello-filtered-query count-query
+				       (or (plist-get options :filter-count)
+					   (plist-get options :filter)))
+	 "\n")))
+
+    (call-process-region (point-min) (point-max) notmuch-command
+			 t t nil "count" "--batch")
+    (goto-char (point-min))
+
+    (notmuch-remove-if-not
+     #'identity
+     (mapcar
+      (lambda (elem)
+	(let ((name (car elem))
+	      (search-query (if (consp (cdr elem))
+				 ;; do we have a different query for the message count?
+				 (second elem)
+			      (cdr elem)))
+	      (message-count (prog1 (read (current-buffer))
+				(forward-line 1))))
+	  (and (or (plist-get options :show-empty-searches) (> message-count 0))
+	       (list name (notmuch-hello-filtered-query
+			   search-query (plist-get options :filter))
+		     message-count))))
+      query-alist))))
 
 (defun notmuch-hello-insert-buttons (searches)
   "Insert buttons for SEARCHES.
-- 
1.7.10.4

Re: [PATCH v2 0/6] batch count for notmuch-hello speedup

[Jameson Graef Rollins]

[-- Attachment #1: Type: text/plain, Size: 635 bytes --]

On Sat, Mar 09 2013, Jani Nikula <jani@nikula.org> wrote:
> Rebased v2 of id:cover.1358273133.git.jani@nikula.org
>
> Jani Nikula (5):
>   cli: remove useless strdup
>   cli: extract count printing to a separate function in notmuch count
>   cli: add --batch option to notmuch count
>   man: document notmuch count --batch and --input options
>   test: notmuch count --batch and --input options

This series LGTM.  The first commit could have used a little bit more
explanation, and there's the one typo I mentioned, but otherwise it's
fine.

> Mark Walters (1):
>   emacs: hello: use batch count

I did not review this patch.

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: [PATCH v2 0/6] batch count for notmuch-hello speedup

[Tomi Ollila]

On Sat, Mar 09 2013, Jani Nikula <jani@nikula.org> wrote:

> Rebased v2 of id:cover.1358273133.git.jani@nikula.org
>
> Jani Nikula (5):
>   cli: remove useless strdup
>   cli: extract count printing to a separate function in notmuch count
>   cli: add --batch option to notmuch count
>   man: document notmuch count --batch and --input options
>   test: notmuch count --batch and --input options
>
> Mark Walters (1):
>   emacs: hello: use batch count

LGTM. comments in id:m2a9s0hvgy.fsf@guru.guru-group.fi and
id:m27gn4htrg.fsf@guru.guru-group.fi apply.

Tomi

>
>  emacs/notmuch-hello.el   |   52 +++++++++++++---------
>  man/man1/notmuch-count.1 |   20 +++++++++
>  notmuch-count.c          |  111 +++++++++++++++++++++++++++++++++++-----------
>  test/count               |   46 +++++++++++++++++++
>  4 files changed, 182 insertions(+), 47 deletions(-)
>
> -- 
> 1.7.10.4