0f/4efaeb0fb9d0c9335c3b39cb77113b3e4fc439

   1 Return-Path: <amdragon@mit.edu>\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 394C3431FC3\r
   6         for <notmuch@notmuchmail.org>; Sun, 16 Dec 2012 11:11:30 -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 Rju8Y7CW83jj for <notmuch@notmuchmail.org>;\r
  16         Sun, 16 Dec 2012 11:11:29 -0800 (PST)\r
  17 Received: from dmz-mailsec-scanner-6.mit.edu (DMZ-MAILSEC-SCANNER-6.MIT.EDU\r
  18         [18.7.68.35])\r
  19         by olra.theworths.org (Postfix) with ESMTP id 0A360431FAE\r
  20         for <notmuch@notmuchmail.org>; Sun, 16 Dec 2012 11:11:28 -0800 (PST)\r
  21 X-AuditID: 12074423-b7fcb6d000000927-40-50ce1cdf359b\r
  22 Received: from mailhub-auth-4.mit.edu ( [18.7.62.39])\r
  23         by dmz-mailsec-scanner-6.mit.edu (Symantec Messaging Gateway) with SMTP\r
  24         id 87.6D.02343.FDC1EC05; Sun, 16 Dec 2012 14:11:27 -0500 (EST)\r
  25 Received: from outgoing.mit.edu (OUTGOING-AUTH.MIT.EDU [18.7.22.103])\r
  26         by mailhub-auth-4.mit.edu (8.13.8/8.9.2) with ESMTP id qBGJBQZb000674; \r
  27         Sun, 16 Dec 2012 14:11:26 -0500\r
  28 Received: from awakening.csail.mit.edu (awakening.csail.mit.edu [18.26.4.91])\r
  29         (authenticated bits=0)\r
  30         (User authenticated as amdragon@ATHENA.MIT.EDU)\r
  31         by outgoing.mit.edu (8.13.6/8.12.4) with ESMTP id qBGJBMxX024218\r
  32         (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NOT);\r
  33         Sun, 16 Dec 2012 14:11:23 -0500 (EST)\r
  34 Received: from amthrax by awakening.csail.mit.edu with local (Exim 4.80)\r
  35         (envelope-from <amdragon@mit.edu>)\r
  36         id 1TkJcQ-0001oQ-0X; Sun, 16 Dec 2012 14:11:22 -0500\r
  37 Date: Sun, 16 Dec 2012 14:11:21 -0500\r
  38 From: Austin Clements <amdragon@MIT.EDU>\r
  39 To: david@tethera.net\r
  40 Subject: Re: [PATCH 3/4] perf-test: initial version of memory test\r
  41         infrastructure.\r
  42 Message-ID: <20121216191121.GH6187@mit.edu>\r
  43 References: <1355682208-13729-1-git-send-email-david@tethera.net>\r
  44         <1355682208-13729-4-git-send-email-david@tethera.net>\r
  45 MIME-Version: 1.0\r
  46 Content-Type: text/plain; charset=us-ascii\r
  47 Content-Disposition: inline\r
  48 In-Reply-To: <1355682208-13729-4-git-send-email-david@tethera.net>\r
  49 User-Agent: Mutt/1.5.21 (2010-09-15)\r
  50 X-Brightmail-Tracker:\r
  51  H4sIAAAAAAAAA+NgFprJKsWRmVeSWpSXmKPExsUixG6nrntf5lyAQfcVBYuNy34yW9xo7Wa0\r
  52         uH5zJrMDs8evtrnMHs9W3WL22HLoPXMAcxSXTUpqTmZZapG+XQJXxue1L1kLJgVXzDz8gqWB\r
  53         sdupi5GTQ0LAROL6vF9MELaYxIV769m6GLk4hAT2MUpsvrqHGcLZwCgxYd0SZpAqIYGLTBKd\r
  54         S1MgEksYJZ7eWswGkmARUJV4eGMpO4jNJqAhsW3/ckYQW0RAVOL0pkmsIDazgL3EotmTwGqE\r
  55         BUIk5ndcBKvhFdCW6J+8iRFiQaVE5/dTbBBxQYmTM5+wQPRqSdz49xLoVA4gW1pi+T8OkDCn\r
  56         gJNE9+UNYONFBVQkppzcxjaBUWgWku5ZSLpnIXQvYGRexSibklulm5uYmVOcmqxbnJyYl5da\r
  57         pGuml5tZopeaUrqJERzqLso7GP8cVDrEKMDBqMTDa/H1TIAQa2JZcWXuIUZJDiYlUd4rUucC\r
  58         hPiS8lMqMxKLM+KLSnNSiw8xSnAwK4nwXhUHyvGmJFZWpRblw6SkOViUxHmvpdz0FxJITyxJ\r
  59         zU5NLUgtgsnKcHAoSfC+kgZqFCxKTU+tSMvMKUFIM3FwggznARp+DqSGt7ggMbc4Mx0if4pR\r
  60         l2PW2TtPGYVY8vLzUqXEed+CFAmAFGWU5sHNgaWoV4ziQG8J8z4GqeIBpje4Sa+AljABLVlu\r
  61         cwZkSUkiQkqqgXHDA4G1t5Pn69/4tzzXZsKvmoXrfa5u7TxhqsLbx7gvpmvpw87urIdnOnIq\r
  62         npkuUAtaVyinuMPZu+bM/WfnN7RpTBYJOH2pfe/WD6z39txhKT387N3t7Rcu2WzPNxWp/fHe\r
  63         L89YbPN0reakrGyDr/O2N+/mTRB9diJH71DqBbF1vd7MD3TuK99UYinOSDTUYi4qTgQAiOE/\r
  64         hywDAAA=\r
  65 Cc: notmuch@notmuchmail.org, David Bremner <bremner@debian.org>\r
  66 X-BeenThere: notmuch@notmuchmail.org\r
  67 X-Mailman-Version: 2.1.13\r
  68 Precedence: list\r
  69 List-Id: "Use and development of the notmuch mail system."\r
  70         <notmuch.notmuchmail.org>\r
  71 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  72         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  73 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  74 List-Post: <mailto:notmuch@notmuchmail.org>\r
  75 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  76 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  77         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  78 X-List-Received-Date: Sun, 16 Dec 2012 19:11:30 -0000\r
  79 \r
  80 Quoth david@tethera.net on Dec 16 at  2:23 pm:\r
  81 > From: David Bremner <bremner@debian.org>\r
  82 > \r
  83 > The idea is run some code under valgrind --leak-check=full and report\r
  84 > a summary, leaving the user to peruse the log file if they want.\r
  85 > \r
  86 > We go to some lengths to preserve the log files from accidental\r
  87 > overwriting; the full corpus takes about 3 hours to run under valgrind\r
  88 > on my machine.\r
  89 > \r
  90 > The naming of the log directories is probably overkill; I find it nice\r
  91 > to have them sequenced by time. Arguably the mktemp is then overkill,\r
  92 > but I know people will be nervous if it looks like timestamps are\r
  93 > being used for uniqueness.\r
  94 > \r
  95 > One new test is included, to check notmuch new for memory leaks.\r
  96 > ---\r
  97 >  performance-test/.gitignore       |    1 +\r
  98 >  performance-test/M00-new          |   14 +++++++++\r
  99 >  performance-test/Makefile.local   |   17 +++++++++--\r
 100 >  performance-test/README           |   57 +++++++++++++++++++++++++------------\r
 101 >  performance-test/perf-test-lib.sh |   55 ++++++++++++++++++++++++++++-------\r
 102 >  5 files changed, 113 insertions(+), 31 deletions(-)\r
 103 >  create mode 100755 performance-test/M00-new\r
 104 > \r
 105 > diff --git a/performance-test/.gitignore b/performance-test/.gitignore\r
 106 > index 6421a9a..f3f9be4 100644\r
 107 > --- a/performance-test/.gitignore\r
 108 > +++ b/performance-test/.gitignore\r
 109 > @@ -1,3 +1,4 @@\r
 110 >  tmp.*/\r
 111 > +log.*/\r
 112 >  corpus/\r
 113 >  notmuch.cache.*/\r
 114 > diff --git a/performance-test/M00-new b/performance-test/M00-new\r
 115 > new file mode 100755\r
 116 > index 0000000..733e9b0\r
 117 > --- /dev/null\r
 118 > +++ b/performance-test/M00-new\r
 119 > @@ -0,0 +1,14 @@\r
 120 > +#!/bin/bash\r
 121 > +\r
 122 > +test_description='notmuch new'\r
 123 > +\r
 124 > +. ./perf-test-lib.sh\r
 125 > +\r
 126 > +# ensure initial 'notmuch new' is run by memory_start\r
 127 > +uncache_database\r
 128 > +\r
 129 > +memory_start\r
 130 > +\r
 131 > +memory_run "notmuch new" "notmuch new"\r
 132 \r
 133 This will run notmuch new twice.  Is that intentional?  If so, it\r
 134 might be worth a comment.\r
 135 \r
 136 > +\r
 137 > +memory_done\r
 138 > diff --git a/performance-test/Makefile.local b/performance-test/Makefile.local\r
 139 > index 57beb44..357d800 100644\r
 140 > --- a/performance-test/Makefile.local\r
 141 > +++ b/performance-test/Makefile.local\r
 142 > @@ -4,14 +4,25 @@ dir := performance-test\r
 143 >  \r
 144 >  include $(dir)/version.sh\r
 145 >  \r
 146 > +# these two are just make sure dir is expanded at the right time.\r
 147 > +TIME_TEST_SCRIPT := ${dir}/notmuch-time-test\r
 148 > +MEMORY_TEST_SCRIPT := ${dir}/notmuch-memory-test\r
 149 \r
 150 This is obviously fine, but I don't understand the comment.  There are\r
 151 all sorts of places where we have to := assign to get ${dir} right\r
 152 (including TXZFILE file below).  Is there something different here?\r
 153 \r
 154 > +\r
 155 >  CORPUS_NAME := notmuch-email-corpus-$(PERFTEST_VERSION).tar.xz\r
 156 >  TXZFILE := ${dir}/download/${CORPUS_NAME}\r
 157 >  SIGFILE := ${TXZFILE}.asc\r
 158 > -TEST_SCRIPT := ${dir}/notmuch-perf-test\r
 159 >  DEFAULT_URL :=  http://notmuchmail.org/releases/${CORPUS_NAME}\r
 160 >  \r
 161 > +perf-test: time-test memory-test\r
 162 > +\r
 163 >  time-test: setup-perf-test all\r
 164 > -     $(TEST_SCRIPT) $(OPTIONS)\r
 165 > +     @echo\r
 166 > +     $(TIME_TEST_SCRIPT) $(TEST_OPTIONS)\r
 167 \r
 168 Why not use the same OPTIONS variable name as the correctness tests?\r
 169 \r
 170 > +\r
 171 > +memory-test: setup-perf-test all\r
 172 > +     @echo\r
 173 > +     $(MEMORY_TEST_SCRIPT) $(TEST_OPTIONS)\r
 174 > +\r
 175 >  \r
 176 >  .PHONY: download-corpus setup-perf-test\r
 177 >  \r
 178 > @@ -29,4 +40,4 @@ $(TXZFILE):\r
 179 >  download-corpus:\r
 180 >       wget -O ${TXZFILE} ${DEFAULT_URL}\r
 181 >  \r
 182 > -CLEAN := $(CLEAN) $(dir)/tmp.* $(dir)/corpus $(dir)/notmuch.cache.*\r
 183 > +CLEAN := $(CLEAN) $(dir)/tmp.* $(dir)/log.* $(dir)/corpus $(dir)/notmuch.cache.*\r
 184 > diff --git a/performance-test/README b/performance-test/README\r
 185 > index d1fb6de..7eaf5f7 100644\r
 186 > --- a/performance-test/README\r
 187 > +++ b/performance-test/README\r
 188 > @@ -1,3 +1,10 @@\r
 189 > +Performance Tests\r
 190 > +-----------------\r
 191 > +\r
 192 > +This directory contains two kinds of performance tests, time tests,\r
 193 \r
 194 s/performance tests,/performance tests:/\r
 195 \r
 196 > +and memory tests. The former use gnu time, and the latter use\r
 197 > +valgrind.\r
 198 > +\r
 199 >  Pre-requisites\r
 200 >  --------------\r
 201 >  \r
 202 > @@ -5,9 +12,10 @@ In addition to having notmuch, you need:\r
 203 >  \r
 204 >  - gpg\r
 205 >  - gnu tar\r
 206 > -- gnu time\r
 207 > +- gnu time (for the time tests).\r
 208 \r
 209 No period?\r
 210 \r
 211 >  - xz. Some speedup can be gotten by installing "pixz", but this is\r
 212 >    probably only worthwhile if you are debugging the tests.\r
 213 > +- valgrind (for the memory tests)\r
 214 >  \r
 215 >  Getting set up to run tests:\r
 216 >  ----------------------------\r
 217 > @@ -36,34 +44,47 @@ for a list of mirrors.\r
 218 >  Running tests\r
 219 >  -------------\r
 220 >  \r
 221 > -The easiest way to run performance tests is to say "make time-test", (or\r
 222 > -simply run the notmuch-time-test script). Either command will run all\r
 223 > -available performance tests.\r
 224 > -\r
 225 > -Alternately, you can run a specific subset of tests by simply invoking\r
 226 > -one of the executable scripts in this directory, (such as ./basic).\r
 227 > -Each test script supports the following arguments\r
 228 > +The easiest way to run performance tests is to say "make perf-test".\r
 229 > +This will run all time and memory tests.  Be aware that the memory\r
 230 > +tests are quite time consuming when run on the full corpus, and that\r
 231 > +depending on your interests it may be more sensible to run "make\r
 232 > +time-test" or "make memory-test".  You can also invoke one of the\r
 233 > +scripts notmuch-time-test or notmuch-memory-test or run a more\r
 234 > +specific subset of tests by simply invoking one of the executable\r
 235 > +scripts in this directory, (such as ./T00-new).  Each test script\r
 236 > +supports the following arguments\r
 237 >  \r
 238 >  --small / --medium / --large Choose corpus size.\r
 239 >  --debug                              Enable debugging. In particular don't delete\r
 240 >                               temporary directories.\r
 241 >  \r
 242 > +When using the make targets, you can pass arguments to all test\r
 243 > +scripts by defining the make variable TEST_OPTIONS.\r
 244 > +\r
 245 >  Writing tests\r
 246 >  -------------\r
 247 >  \r
 248 > -Have a look at "T01-dump-restore" for an example. Sourcing\r
 249 > -"perf-test-lib.sh" is mandatory.  Utility functions include\r
 250 > +Have a look at "T01-dump-restore" for an example time test and and\r
 251 \r
 252 s/and and/and/\r
 253 \r
 254 > +"M00-new" for an example memory tests. In both cases sourcing\r
 255 > +"perf-test-lib.sh" is mandatory.\r
 256 >  \r
 257 > -- 'add_email_corpus' unpacks a set of messages and adds them to the database.\r
 258 > -- 'cache_database': makes a snapshot of the current database\r
 259 > -- 'uncache_database': forces the next 'add_email_corpus' to rebuild the\r
 260 > -  database.\r
 261 > -- 'time_start' unpacks the mail corpus and calls notmuch new if it\r
 262 > +Basics:\r
 263 > +\r
 264 > +- '(time|memory)_start' unpacks the mail corpus and calls notmuch new if it\r
 265 >     cannot find a cache of the appropriate corpus.\r
 266 > -- 'time_done' does the cleanup; comment it out or pass --debug to the\r
 267 > +- '(time|memory)_run' runs the command under time or valgrind. Currently\r
 268 > +  "memory_run" does not support i/o redirection in the command.\r
 269 > +- '(time|memory)_done' does the cleanup; comment it out or pass --debug to the\r
 270 >    script to leave the temporary files around.\r
 271 >  \r
 272 > +Utility functions include\r
 273 > +\r
 274 > +- 'add_email_corpus' unpacks a set of messages and tags\r
 275 > +- 'cache_database': makes a snapshot of the current database\r
 276 > +- 'uncache_database': forces the next '(time|memory)_start' to rebuild the\r
 277 > +  database.\r
 278 > +\r
 279 >  Scripts are run in the order specified in notmuch-perf-test. In the\r
 280 >  future this order might be chosen automatically so please follow the\r
 281 > -convention of starting the name with 'T' followed by two digits to\r
 282 > -specify the order.\r
 283 > +convention of starting the name with 'T' or 'M' followed by two digits\r
 284 > +to specify the order.\r
 285 > diff --git a/performance-test/perf-test-lib.sh b/performance-test/perf-test-lib.sh\r
 286 > index 3a7ef2b..79eb2c5 100644\r
 287 > --- a/performance-test/perf-test-lib.sh\r
 288 > +++ b/performance-test/perf-test-lib.sh\r
 289 > @@ -92,20 +92,52 @@ add_email_corpus ()\r
 290 >  \r
 291 >  }\r
 292 >  \r
 293 > -time_start () {\r
 294 > -\r
 295 > -    add_email_corpus\r
 296 > -\r
 297 > -    print_header\r
 298 > +notmuch_new_with_cache () {\r
 299 >  \r
 300 \r
 301 Out of curiosity, why the blank lines at the beginning of functions?\r
 302 \r
 303 >      if [ -d $DB_CACHE_DIR ]; then\r
 304 >       cp -r $DB_CACHE_DIR ${MAIL_DIR}/.notmuch\r
 305 >      else\r
 306 > -     time_run 'Initial notmuch new' "notmuch new"\r
 307 > +     "$1" 'Initial notmuch new' "notmuch new"\r
 308 >       cache_database\r
 309 >      fi\r
 310 >  }\r
 311 >  \r
 312 > +time_start () {\r
 313 > +\r
 314 > +    add_email_corpus\r
 315 > +\r
 316 > +    print_header\r
 317 > +\r
 318 > +    notmuch_new_with_cache time_run\r
 319 > +}\r
 320 > +\r
 321 > +memory_start () {\r
 322 > +\r
 323 > +    add_email_corpus\r
 324 > +\r
 325 > +    _timestamp=$(printf "%x" $(date +"%s"))\r
 326 \r
 327 You could use 'local' here.\r
 328 \r
 329 > +    log_dir=$(mktemp -d "${TEST_DIRECTORY}/log.$(basename $0)-$corpus_size-${_timestamp}-XXXXXX")\r
 330 \r
 331 This really does seem like overkill.  I'd say just use timestamps for\r
 332 uniqueness.  These tests take a long time, and if you do manage to run\r
 333 the same test twice in less than a second, does it really matter that\r
 334 you have to run it again if you want the first log?\r
 335 \r
 336 How about using ISO 8601 timestamps?  They're both compact and more\r
 337 user-friendly than hexadecimal epoch time.\r
 338 \r
 339 local timestamp=$(date +%Y%m%dT%H%M%S)\r
 340 log_dir="${TEST_DIRECTORY}/log.$(basename $0)-$corpus_size-${timestamp}"\r
 341 \r
 342 > +\r
 343 > +    notmuch_new_with_cache memory_run\r
 344 > +}\r
 345 > +\r
 346 > +memory_run () {\r
 347 > +    test_count=$(($test_count+1))\r
 348 > +\r
 349 > +    log_file=$log_dir/$test_count.log\r
 350 > +\r
 351 > +    printf "[ %d ]\t%s\n" $test_count "$1"\r
 352 > +\r
 353 > +    valgrind --leak-check=full --log-file="$log_file" $2\r
 354 > +\r
 355 > +    awk '/LEAK SUMMARY/,/suppressed/ { sub(/^==[0-9]*==/," "); print }' "$log_file"\r
 356 > +    echo\r
 357 > +}\r
 358 > +\r
 359 > +memory_done () {\r
 360 > +    time_done\r
 361 > +}\r
 362 > +\r
 363 >  cache_database () {\r
 364 >      if [ -d $MAIL_DIR/.notmuch ]; then\r
 365 >       cp -r $MAIL_DIR/.notmuch $DB_CACHE_DIR\r
 366 > @@ -119,12 +151,13 @@ uncache_database () {\r
 367 >  }\r
 368 >  \r
 369 >  print_header () {\r
 370 > -    printf "[v%4s %6s]          Wall(s)\tUsr(s)\tSys(s)\tRes(K)\tIn/Out(512B)\n" \\r
 371 > -        ${PERFTEST_VERSION} ${corpus_size}\r
 372 > +    printf "\t\t\tWall(s)\tUsr(s)\tSys(s)\tRes(K)\tIn/Out(512B)\n"\r
 373 > +\r
 374 >  }\r
 375 >  \r
 376 >  time_run () {\r
 377 >      printf "  %-22s" "$1"\r
 378 > +    test_count=$(($test_count+1))\r
 379 >      if test "$verbose" != "t"; then exec 4>test.output 3>&4; fi\r
 380 >      if ! eval >&3 "/usr/bin/time -f '%e\t%U\t%S\t%M\t%I/%O' $2" ; then\r
 381 >       test_failure=$(($test_failure + 1))\r
 382 > @@ -144,6 +177,8 @@ time_done () {\r
 383 >  \r
 384 >  cd -P "$test" || error "Cannot setup test environment"\r
 385 >  test_failure=0\r
 386 > +test_count=0\r
 387 >  \r
 388 > -echo\r
 389 > -echo $(basename "$0"): "Testing ${test_description:-notmuch performance}"\r
 390 > +printf "\n%-55s [%s %s]\n"  \\r
 391 > +    "$(basename "$0"): Testing ${test_description:-notmuch performance}" \\r
 392 > +    "${PERFTEST_VERSION}"  "${corpus_size}"\r