Date: Sun, 25 Nov 2012 16:40:24 -0500
From: Austin Clements <amdragon@MIT.EDU>
To: david@tethera.net
Subject: Re: [Patch v4 2/2] test: initial performance testing infrastructure
Message-ID: <20121125214023.GO4562@mit.edu>
References: <1353855745-11697-1-git-send-email-david@tethera.net>
	<1353855745-11697-3-git-send-email-david@tethera.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1353855745-11697-3-git-send-email-david@tethera.net>
User-Agent: Mutt/1.5.21 (2010-09-15)
Cc: notmuch@notmuchmail.org, David Bremner <bremner@debian.org>
Precedence: list

Quoth david@tethera.net on Nov 25 at 11:02 am:
> From: David Bremner <bremner@debian.org>
> 
> This is not near as fancy as as the unit tests, on the theory that
> the code should typically be crashing when performance tuning.
> Nonetheless, there is plenty of room for improvement.  Several more of
> the pieces of the test infrastructure (e.g. the option parsing) could
> be factored out into test/test-lib-common.sh
> ---
>  Makefile                                           |    3 +-
>  performance-test/.gitignore                        |    1 +
>  performance-test/Makefile                          |    7 +++
>  performance-test/Makefile.local                    |   32 ++++++++++
>  performance-test/README                            |   50 +++++++++++++++
>  performance-test/basic                             |   15 +++++
>  performance-test/download/.gitignore               |    2 +
>  .../download/notmuch-email-corpus-0.2.tar.xz.asc   |    9 +++
>  performance-test/notmuch-perf-test                 |   25 ++++++++
>  performance-test/perf-test-lib.sh                  |   65 ++++++++++++++++++++
>  performance-test/version.sh                        |    3 +
>  11 files changed, 211 insertions(+), 1 deletion(-)
>  create mode 100644 performance-test/.gitignore
>  create mode 100644 performance-test/Makefile
>  create mode 100644 performance-test/Makefile.local
>  create mode 100644 performance-test/README
>  create mode 100755 performance-test/basic
>  create mode 100644 performance-test/download/.gitignore
>  create mode 100644 performance-test/download/notmuch-email-corpus-0.2.tar.xz.asc
>  create mode 100755 performance-test/notmuch-perf-test
>  create mode 100644 performance-test/perf-test-lib.sh
>  create mode 100644 performance-test/version.sh
> 
> diff --git a/Makefile b/Makefile
> index bb9c316..5decbea 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -3,7 +3,8 @@
>  all:
>  
>  # List all subdirectories here. Each contains its own Makefile.local
> -subdirs = compat completion emacs lib man parse-time-string util test
> +subdirs := compat completion emacs lib man parse-time-string
> +subdirs := $(subdirs) performance-test util test

+= ?

>  
>  # We make all targets depend on the Makefiles themselves.
>  global_deps = Makefile Makefile.config Makefile.local \
> diff --git a/performance-test/.gitignore b/performance-test/.gitignore
> new file mode 100644
> index 0000000..53f2697
> --- /dev/null
> +++ b/performance-test/.gitignore
> @@ -0,0 +1 @@
> +tmp.*/
> diff --git a/performance-test/Makefile b/performance-test/Makefile
> new file mode 100644
> index 0000000..de492a7
> --- /dev/null
> +++ b/performance-test/Makefile
> @@ -0,0 +1,7 @@
> +# See Makefile.local for the list of files to be compiled in this
> +# directory.
> +all:
> +	$(MAKE) -C .. all
> +
> +.DEFAULT:
> +	$(MAKE) -C .. $@
> diff --git a/performance-test/Makefile.local b/performance-test/Makefile.local
> new file mode 100644
> index 0000000..1114ec1
> --- /dev/null
> +++ b/performance-test/Makefile.local
> @@ -0,0 +1,32 @@
> +# -*- makefile -*-
> +
> +dir := performance-test
> +
> +include $(dir)/version.sh
> +
> +CORPUS_NAME := notmuch-email-corpus-$(PERFTEST_VERSION).tar.xz

Would it make sense to split out the different size corpora so a user
could, say, only download the small one?

> +TXZFILE := ${dir}/download/${CORPUS_NAME}
> +SIGFILE := ${TXZFILE}.asc
> +TEST_SCRIPT := ${dir}/notmuch-perf-test
> +DEFAULT_URL :=  http://notmuchmail.org/releases/${CORPUS_NAME}
> +
> +perf-test: setup-perf-test all
> +	$(TEST_SCRIPT) $(OPTIONS)
> +
> +.PHONY: download-corpus setup-perf-test
> +
> +# Note that this intentionally does not depend on download-corpus.
> +setup-perf-test: $(TXZFILE)
> +	gpg --verify $(SIGFILE)
> +
> +$(TXZFILE):
> +	@printf "\nPlease download ${TXZFILE}.\n\n"

"\nPlease download ${TXZFILE} using\n\n"?

> +	@printf "\t%% make download-corpus\n\n"
> +	@echo or see http://notmuchmail.org/corpus for download locations
> +	@echo
> +	@false
> +
> +download-corpus:
> +	wget -O ${TXZFILE} ${DEFAULT_URL}
> +
> +CLEAN := $(CLEAN) $(dir)/tmp.*
> diff --git a/performance-test/README b/performance-test/README
> new file mode 100644
> index 0000000..239d2fb
> --- /dev/null
> +++ b/performance-test/README
> @@ -0,0 +1,50 @@
> +Pre-requisites
> +--------------
> +
> +In addition to having notmuch, you need:
> +
> +- gpg
> +- gnu tar
> +- gnu time
> +- xz. Some speedup can be gotten by installing "pixz", but this is
> +  probably only worthwhile if you are debugging the tests.
> +
> +Getting set up to run tests:
> +----------------------------
> +
> +First, you need to get the corpus.
> +
> +It should work to run
> +
> +   % make download-corpus
> +
> +In case that fails or is too slow, check
> +
> +   http://notmuchmail.org/corpus
> +
> +for a list of mirrors.
> +
> +Running tests
> +-------------
> +
> +The easiest way to run performance tests is to say "make perf-test", (or
> +simply run the notmuch-perf-test script). Either command will run all
> +available performance tests.
> +
> +Alternately, you can run a specific subset of tests by simply invoking
> +one of the executable scripts in this directory, (such as ./basic).
> +
> +
> +Writing tests
> +-------------
> +
> +Have a look at "basic" for an example.
> +
> +add_email_corpus takes arguments "--small" and "--medium" for when you
> +want smaller corpuses to check.

"corpora"?

> +
> +time_done does the cleanup; comment it out or define "$debug" to leave
> +the temporary files around.
> +
> +Currently there is no option processing (e.g. --debug) in the
> +performance tests.
> diff --git a/performance-test/basic b/performance-test/basic
> new file mode 100755
> index 0000000..9d015ee
> --- /dev/null
> +++ b/performance-test/basic
> @@ -0,0 +1,15 @@
> +#!/bin/bash
> +
> +. ./perf-test-lib.sh
> +
> +add_email_corpus
> +
> +print_header
> +
> +time_run 'initial notmuch new' 'notmuch new'
> +time_run 'second notmuch new' 'notmuch new'
> +time_run 'dump *' 'notmuch dump > tags.out'
> +time_run 'restore *' 'notmuch restore < tags.out'
> +time_run 'tag * +new_tag' "notmuch tag +new_tag '*'"
> +
> +time_done
> diff --git a/performance-test/download/.gitignore b/performance-test/download/.gitignore
> new file mode 100644
> index 0000000..7b09234
> --- /dev/null
> +++ b/performance-test/download/.gitignore
> @@ -0,0 +1,2 @@
> +*.tar.gz
> +*.tar.xz
> diff --git a/performance-test/download/notmuch-email-corpus-0.2.tar.xz.asc b/performance-test/download/notmuch-email-corpus-0.2.tar.xz.asc
> new file mode 100644
> index 0000000..c8b4b3d
> --- /dev/null
> +++ b/performance-test/download/notmuch-email-corpus-0.2.tar.xz.asc
> @@ -0,0 +1,9 @@
> +-----BEGIN PGP SIGNATURE-----
> +Version: GnuPG v1.4.12 (GNU/Linux)
> +
> +iJwEAAECAAYFAlCsvx0ACgkQTiiN/0Um85kZAwP9GgOQ22jK8mr5X4pT/mB8EjSH
> +QbndlxxbRrP0ChTqjBQoD3IsTHjNL7W572BfXb/MNo94R/iIQ7yTHCDVNuwBhvKd
> +7qgIuW2FUS1uTfJRP5KBNf8JPuin+6wqGe8/+y/iOs+XJSdiYg1ElS49Ntnpg0yl
> +btImgEcxTxQ2qfzDS1g=
> +=iuZR
> +-----END PGP SIGNATURE-----
> diff --git a/performance-test/notmuch-perf-test b/performance-test/notmuch-perf-test
> new file mode 100755
> index 0000000..1bea345
> --- /dev/null
> +++ b/performance-test/notmuch-perf-test
> @@ -0,0 +1,25 @@
> +#!/usr/bin/env bash
> +
> +# Run tests
> +#
> +# Copyright (c) 2005 Junio C Hamano
> +#
> +# Adapted from a Makefile to a shell script by Carl Worth (2010)
> +
> +if [ ${BASH_VERSINFO[0]} -lt 4 ]; then
> +    echo "Error: The notmuch test suite requires a bash version >= 4.0"
> +    echo "due to use of associative arrays within the test suite."
> +    echo "Please try again with a newer bash (or help us fix the"
> +    echo "test suite to be more portable). Thanks."
> +    exit 1
> +fi
> +
> +cd $(dirname "$0")
> +
> +TESTS="
> +  basic
> +"
> +
> +for test in $TESTS; do
> +    ./$test "$@"
> +done
> diff --git a/performance-test/perf-test-lib.sh b/performance-test/perf-test-lib.sh
> new file mode 100644
> index 0000000..11ccc77
> --- /dev/null
> +++ b/performance-test/perf-test-lib.sh
> @@ -0,0 +1,65 @@
> +. ./version.sh
> +
> +. ../test/test-lib-common.sh
> +
> +set -e
> +
> +if ! test -x ../notmuch
> +then
> +	echo >&2 'You do not seem to have built notmuch yet.'
> +	exit 1
> +fi
> +
> +add_email_corpus ()
> +{
> +    rm -rf ${MAIL_DIR}
> +
> +    arg=""
> +    case "$1" in
> +	--small)
> +	    arg="/enron/bailey-s"
> +	    ;;
> +	--medium)
> +	    arg="/notmuch-archive"
> +	    ;;
> +    esac
> +
> +    if command -v pixz > /dev/null; then
> +	XZ=pixz
> +    else
> +	XZ=xz
> +    fi
> +
> +    printf "Unpacking corpus\n"
> +    tar --checkpoint=.5000 --extract --strip-components=1 \
> +	--directory ${TMP_DIRECTORY} \
> +	--use-compress-program ${XZ} \
> +	--file ../download/notmuch-email-corpus-${PERFTEST_VERSION}.tar.xz \
> +	notmuch-email-corpus/mail"$arg"

I'm a bit confused by this.  What happens if you don't specify --small
or --medium?  Is the "large"/default corpus just the combined small
and medium corpora?  Would be worth a comment, at least.

This probably doesn't matter now, but I wonder if we want to unpack on
first use to somewhere not test-specific and then cp -rl the corpus
into the test directory.  I haven't tried unpacking the corpus yet,
but if you're running tests repeatedly to compare results, or running
more than one performance test, it seems like a full decompress and
unpack could get onerous.

> +
> +    printf "\n"
> +}
> +
> +print_header () {
> +    printf "                      Wall(s)\tUsr(s)\tSys(s)\tRes(K)\tIn\tOut\n"

Should the header include which corpus size is in use?  This would be
important when emailing around performance results.

> +}
> +
> +time_run () {
> +    printf "%-22s" "$1"
> +    if test "$verbose" != "t"; then exec 4>test.output 3>&4; fi
> +    if ! eval >&3 "/usr/bin/time -f '%e\t%U\t%S\t%M\t%I\t%O' $2" ; then
> +	test_failure=$(($test_failure + 1))
> +    fi
> +}
> +
> +time_done () {
> +    if [ "$test_failure" = "0" ]; then
> +	rm -rf "$remove_tmp"
> +	exit 0
> +    else
> +	exit 1
> +    fi
> +}
> +
> +cd -P "$test" || error "Cannot setup test environment"
> +test_failure=0
> diff --git a/performance-test/version.sh b/performance-test/version.sh
> new file mode 100644
> index 0000000..d9270b1
> --- /dev/null
> +++ b/performance-test/version.sh
> @@ -0,0 +1,3 @@
> +# this should be both a valid Makefile fragment and valid POSIX(ish) shell.
> +
> +PERFTEST_VERSION=0.2