8b/ddae8b0f5549f32a6b1921955973f3506d6c3a

   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 83C6A431FBD\r
   6         for <notmuch@notmuchmail.org>; Mon, 10 Mar 2014 19:07:08 -0700 (PDT)\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 cuG6QWbMUVIK for <notmuch@notmuchmail.org>;\r
  16         Mon, 10 Mar 2014 19:07:04 -0700 (PDT)\r
  17 Received: from dmz-mailsec-scanner-4.mit.edu (dmz-mailsec-scanner-4.mit.edu\r
  18         [18.9.25.15])\r
  19         (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits))\r
  20         (No client certificate requested)\r
  21         by olra.theworths.org (Postfix) with ESMTPS id 25D0B431FBC\r
  22         for <notmuch@notmuchmail.org>; Mon, 10 Mar 2014 19:07:04 -0700 (PDT)\r
  23 X-AuditID: 1209190f-f790b6d000000c3a-93-531e6fc74afc\r
  24 Received: from mailhub-auth-3.mit.edu ( [18.9.21.43])\r
  25         (using TLS with cipher AES256-SHA (256/256 bits))\r
  26         (Client did not present a certificate)\r
  27         by dmz-mailsec-scanner-4.mit.edu (Symantec Messaging Gateway) with SMTP\r
  28         id F2.BA.03130.7CF6E135; Mon, 10 Mar 2014 22:07:03 -0400 (EDT)\r
  29 Received: from outgoing.mit.edu (outgoing-auth-1.mit.edu [18.9.28.11])\r
  30         by mailhub-auth-3.mit.edu (8.13.8/8.9.2) with ESMTP id s2B271aO002954; \r
  31         Mon, 10 Mar 2014 22:07:02 -0400\r
  32 Received: from awakening.csail.mit.edu (awakening.csail.mit.edu [18.26.4.91])\r
  33         (authenticated bits=0)\r
  34         (User authenticated as amdragon@ATHENA.MIT.EDU)\r
  35         by outgoing.mit.edu (8.13.8/8.12.4) with ESMTP id s2B26xxj008042\r
  36         (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NOT);\r
  37         Mon, 10 Mar 2014 22:07:00 -0400\r
  38 Received: from amthrax by awakening.csail.mit.edu with local (Exim 4.80)\r
  39         (envelope-from <amdragon@mit.edu>)\r
  40         id 1WNC5r-0008IW-Gs; Mon, 10 Mar 2014 22:06:59 -0400\r
  41 From: Austin Clements <amdragon@MIT.EDU>\r
  42 To: Mark Walters <markwalters1009@gmail.com>, notmuch@notmuchmail.org\r
  43 Subject: Re: [PATCH v2 1/7] Make keys of notmuch-tag-formats regexps and use\r
  44         caching\r
  45 In-Reply-To: <1392841212-8494-2-git-send-email-markwalters1009@gmail.com>\r
  46 References: <1392841212-8494-1-git-send-email-markwalters1009@gmail.com>\r
  47         <1392841212-8494-2-git-send-email-markwalters1009@gmail.com>\r
  48 User-Agent: Notmuch/0.17~rc2+14~g06f47e0 (http://notmuchmail.org) Emacs/23.4.1\r
  49         (i486-pc-linux-gnu)\r
  50 Date: Mon, 10 Mar 2014 22:06:59 -0400\r
  51 Message-ID: <87eh2932v0.fsf@awakening.csail.mit.edu>\r
  52 MIME-Version: 1.0\r
  53 Content-Type: text/plain; charset=us-ascii\r
  54 X-Brightmail-Tracker:\r
  55  H4sIAAAAAAAAA+NgFnrEIsWRmVeSWpSXmKPExsUixCmqrXs8Xy7YYPt8NovVc3ksrt+cyezA\r
  56         5LFz1l12j2erbjEHMEVx2aSk5mSWpRbp2yVwZdx++out4KVXxdFLm9gbGE9bdTFyckgImEgs\r
  57         OTKdCcIWk7hwbz1bFyMXh5DAbCaJHyc/skA4Gxklzs08AZU5zSQx5ck3ZghnCaPEzeXP2UH6\r
  58         2QQ0JLbtX84IYosIuEo8/faZGcQWFgiReHZ5DVgNp4CnxNmpfYwQza2MEheWXmEBSYgKJEmc\r
  59         nvqbFcRmEVCV+HhhLlicF+jAXZP2M0HYghInZz4BizMLaEnc+PeSaQKjwCwkqVlIUgsYmVYx\r
  60         yqbkVunmJmbmFKcm6xYnJ+blpRbpmujlZpbopaaUbmIEB6Uk/w7GbweVDjEKcDAq8fAG+MoF\r
  61         C7EmlhVX5h5ilORgUhLlXZcJFOJLyk+pzEgszogvKs1JLT7EKMHBrCTCe9oCKMebklhZlVqU\r
  62         D5OS5mBREuftOysRLCSQnliSmp2aWpBaBJOV4eBQkuB9kQfUKFiUmp5akZaZU4KQZuLgBBnO\r
  63         AzScA6SGt7ggMbc4Mx0if4pRUUqcVx4kIQCSyCjNg+uFJY1XjOJArwjzXgCp4gEmHLjuV0CD\r
  64         mYAGNx+XAhlckoiQkmpgjHl77Sdj94TfAcoLVsfXGrb5lQsV/5sb3KEqci3fK27z1PW/lilE\r
  65         tGyvUTbwrD9x9r9Vj+qW9/s1XXZMTLubefynw944wydhy9XyDoodKZRl1LBQPcsULfmBVXxO\r
  66         iPRil/Tf9Z/7dgv73H47+Z983QTvKR7ROUW/F56Szzn/JvrlozuWnx6dVmIpzkg01GIuKk4E\r
  67         ADs20GL1AgAA\r
  68 X-BeenThere: notmuch@notmuchmail.org\r
  69 X-Mailman-Version: 2.1.13\r
  70 Precedence: list\r
  71 List-Id: "Use and development of the notmuch mail system."\r
  72         <notmuch.notmuchmail.org>\r
  73 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  74         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  75 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  76 List-Post: <mailto:notmuch@notmuchmail.org>\r
  77 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  78 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  79         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  80 X-List-Received-Date: Tue, 11 Mar 2014 02:07:08 -0000\r
  81 \r
  82 On Wed, 19 Feb 2014, Mark Walters <markwalters1009@gmail.com> wrote:\r
  83 > From: Austin Clements <amdragon@MIT.EDU>\r
  84 >\r
  85 > This patch switches notmuch-tag-formats to use regexps with caching\r
  86 > for performance.\r
  87 >\r
  88 > We have to clear the cache somehow on changes to notmuch-tag-formats.\r
  89 > This version takes the simplest approach: search/show/tree all clear\r
  90 > the cache whenever they start loading.\r
  91 >\r
  92 > We cannot use assoc-default since there's no way to distinguish a\r
  93 > missing key from a present key with a null cdr: thus, we use assoc*\r
  94 > from cl instead.\r
  95 >\r
  96 > Performance-wise, the caching of regexp lookup makes this at least as\r
  97 > fast as the previous code using assoc (see\r
  98 > id:1392226351-31440-1-git-send-email-amdragon@mit.edu for timing\r
  99 > details).\r
 100 \r
 101 How about this for a commit message?\r
 102 \r
 103 - 8< -\r
 104 \r
 105 This modifies `notmuch-tag-format-tag' to treat the keys of\r
 106 `notmuch-tag-formats' as (anchored) regexps, rather than literal\r
 107 strings.  This is clearly more flexible, as it allows for prefix\r
 108 matching, defining a fallback format, etc.  This may cause compatibility\r
 109 problems if people have customized `notmuch-tag-formats' to match tags\r
 110 that contain regexp specials, but this seems unlikely.\r
 111 \r
 112 Regular expression matching has quite a performance hit over string\r
 113 lookup, so this also introduces a simple cache from exact tags to\r
 114 formatted strings.  The number of unique tags is likely to be quite\r
 115 small, so this cache should have a high hit rate.  In addition to\r
 116 eliminating the regexp lookup in the common case, this cache stores\r
 117 fully formatted tags, eliminating the repeated evaluation of potentially\r
 118 expensive, user-specified formatting code.  This makes regexp lookup at\r
 119 least as fast as assoc for unformatted tags (e.g., inbox) and *faster*\r
 120 than the current code for formatted tags (e.g., unread):\r
 121 \r
 122                     inbox (usec)   unread (usec)\r
 123     assoc:              0.4            2.8\r
 124     regexp:             3.2            7.2\r
 125     regexp+caching:     0.4            0.4\r
 126 \r
 127 (Though even at 7.2 usec, tag formatting is not our top bottleneck.)\r
 128 \r
 129 This cache must be explicitly cleared to keep it coherent, so this adds\r
 130 the appropriate clearing calls.\r
 131 \r
 132 - >8 -\r
 133 \r
 134 > ---\r
 135 >  emacs/notmuch-show.el |    1 +\r
 136 >  emacs/notmuch-tag.el  |   70 +++++++++++++++++++++++++++++++++---------------\r
 137 >  emacs/notmuch-tree.el |    1 +\r
 138 >  emacs/notmuch.el      |    1 +\r
 139 >  4 files changed, 51 insertions(+), 22 deletions(-)\r
 140 >\r
 141 > diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el\r
 142 > index 1ac80ca..4bddf6c 100644\r
 143 > --- a/emacs/notmuch-show.el\r
 144 > +++ b/emacs/notmuch-show.el\r
 145 > @@ -1145,6 +1145,7 @@ function is used."\r
 146 >      ;; Don't track undo information for this buffer\r
 147 >      (set 'buffer-undo-list t)\r
 148 >  \r
 149 > +    (notmuch-tag-clear-cache)\r
 150 >      (erase-buffer)\r
 151 >      (goto-char (point-min))\r
 152 >      (save-excursion\r
 153 > diff --git a/emacs/notmuch-tag.el b/emacs/notmuch-tag.el\r
 154 > index 908e7ad..47e0205 100644\r
 155 > --- a/emacs/notmuch-tag.el\r
 156 > +++ b/emacs/notmuch-tag.el\r
 157 > @@ -28,23 +28,39 @@\r
 158 >  (require 'crm)\r
 159 >  (require 'notmuch-lib)\r
 160 >  \r
 161 > +;; (notmuch-tag-clear-cache will be called by the defcustom\r
 162 > +;; notmuch-tag-formats, so it has to be defined first.)\r
 163 \r
 164 This is no longer true, so this comment can be removed and the following\r
 165 two defs can be moved to a more natural location further down in the\r
 166 file.\r
 167 \r
 168 > +\r
 169 > +(defvar notmuch-tag--format-cache (make-hash-table :test 'equal)\r
 170 > +  "Cache of tag format lookup.  Internal to `notmuch-tag-format-tag'.")\r
 171 > +\r
 172 > +(defun notmuch-tag-clear-cache ()\r
 173 > +  "Clear the internal cache of tag formats.\r
 174 > +\r
 175 > +This must be called after changes to `notmuch-tag-formats'."\r
 176 \r
 177 Likewise, this sentence is out of date.  I would just omit it and add\r
 178 the doc I suggest below.\r
 179 \r
 180 > +  (clrhash notmuch-tag--format-cache))\r
 181 > +\r
 182 >  (defcustom notmuch-tag-formats\r
 183 >    '(("unread" (propertize tag 'face '(:foreground "red")))\r
 184 >      ("flagged" (propertize tag 'face '(:foreground "blue"))\r
 185 >       (notmuch-tag-format-image-data tag (notmuch-tag-star-icon))))\r
 186 >    "Custom formats for individual tags.\r
 187 >  \r
 188 > -This gives a list that maps from tag names to lists of formatting\r
 189 > -expressions.  The car of each element gives a tag name and the\r
 190 > -cdr gives a list of Elisp expressions that modify the tag.  If\r
 191 > -the list is empty, the tag will simply be hidden.  Otherwise,\r
 192 > -each expression will be evaluated in order: for the first\r
 193 > -expression, the variable `tag' will be bound to the tag name; for\r
 194 > -each later expression, the variable `tag' will be bound to the\r
 195 > -result of the previous expression.  In this way, each expression\r
 196 > -can build on the formatting performed by the previous expression.\r
 197 > -The result of the last expression will displayed in place of the\r
 198 > -tag.\r
 199 > +This is an association list that maps from tag name regexps to\r
 200 > +lists of formatting expressions.  The first entry whose car\r
 201 > +regexp-matches a tag will be used to format that tag.  The regexp\r
 202 > +is implicitly anchored, so to match a literal tag name, just use\r
 203 > +that tag name (if it contains special regexp characters like\r
 204 > +\".\" or \"*\", these have to be escaped).  The cdr of the\r
 205 > +matching entry gives a list of Elisp expressions that modify the\r
 206 > +tag.  If the list is empty, the tag will simply be hidden.\r
 207 > +Otherwise, each expression will be evaluated in order: for the\r
 208 > +first expression, the variable `tag' will be bound to the tag\r
 209 > +name; for each later expression, the variable `tag' will be bound\r
 210 > +to the result of the previous expression.  In this way, each\r
 211 > +expression can build on the formatting performed by the previous\r
 212 > +expression.  The result of the last expression will displayed in\r
 213 > +place of the tag.\r
 214 >  \r
 215 >  For example, to replace a tag with another string, simply use\r
 216 >  that string as a formatting expression.  To change the foreground\r
 217 > @@ -56,7 +72,7 @@ with images."\r
 218 >  \r
 219 >    :group 'notmuch-search\r
 220 >    :group 'notmuch-show\r
 221 > -  :type '(alist :key-type (string :tag "Tag")\r
 222 > +  :type '(alist :key-type (regexp :tag "Tag")\r
 223 >               :extra-offset -3\r
 224 >               :value-type\r
 225 >               (radio :format "%v"\r
 226 > @@ -137,16 +153,26 @@ This can be used with `notmuch-tag-format-image-data'."\r
 227 >  \r
 228 >  (defun notmuch-tag-format-tag (tag)\r
 229 >    "Format TAG by looking into `notmuch-tag-formats'."\r
 230 \r
 231 This would be a great place to mention that modes need to call\r
 232 `notmuch-tag-clear-cache' if they intent to use formatted tags.\r
 233 \r
 234   "Format TAG according to `notmuch-tag-formats'.\r
 235 \r
 236 Callers must ensure that the tag format cache has been recently cleared\r
 237 via `notmuch-tag-clear-cache' before using this function.  For example,\r
 238 it would be appropriate to clear the cache just prior to filling a\r
 239 buffer that uses formatted tags."\r
 240 \r
 241 > -  (let ((formats (assoc tag notmuch-tag-formats)))\r
 242 > -    (cond\r
 243 > -     ((null formats)         ;; - Tag not in `notmuch-tag-formats',\r
 244 > -      tag)                   ;;   the format is the tag itself.\r
 245 > -     ((null (cdr formats))   ;; - Tag was deliberately hidden,\r
 246 > -      nil)                   ;;   no format must be returned\r
 247 > -     (t                              ;; - Tag was found and has formats,\r
 248 > -      (let ((tag tag))               ;;   we must apply all the formats.\r
 249 > -     (dolist (format (cdr formats) tag)\r
 250 > -       (setq tag (eval format))))))))\r
 251 > +  (let ((formatted (gethash tag notmuch-tag--format-cache 'missing)))\r
 252 > +    (when (eq formatted 'missing)\r
 253 > +      (let* ((formats\r
 254 > +           (save-match-data\r
 255 \r
 256 This is a better place for the comment about assoc* that's currently in\r
 257 the commit message.\r
 258 \r
 259 ;; Don't use assoc-default since there's no way to distinguish a missing\r
 260 ;; key from a present key with a null cdr:.\r
 261 \r
 262 > +             (assoc* tag notmuch-tag-formats\r
 263 > +                     :test (lambda (tag key)\r
 264 > +                             (and (eq (string-match key tag) 0)\r
 265 > +                                  (= (match-end 0) (length tag))))))))\r
 266 > +     (setq formatted\r
 267 > +           (cond\r
 268 > +            ((null formats)          ;; - Tag not in `notmuch-tag-formats',\r
 269 > +             tag)                    ;;   the format is the tag itself.\r
 270 > +            ((null (cdr formats))    ;; - Tag was deliberately hidden,\r
 271 > +             nil)                    ;;   no format must be returned\r
 272 > +            (t                       ;; - Tag was found and has formats,\r
 273 > +             (let ((tag tag))        ;;   we must apply all the formats.\r
 274 > +               (dolist (format (cdr formats) tag)\r
 275 > +                 (setq tag (eval format)))))))\r
 276 > +     (puthash tag formatted notmuch-tag--format-cache)))\r
 277 > +    formatted))\r
 278 >  \r
 279 >  (defun notmuch-tag-format-tags (tags &optional face)\r
 280 >    "Return a string representing formatted TAGS."\r
 281 > diff --git a/emacs/notmuch-tree.el b/emacs/notmuch-tree.el\r
 282 > index 4f2ac02..a106e09 100644\r
 283 > --- a/emacs/notmuch-tree.el\r
 284 > +++ b/emacs/notmuch-tree.el\r
 285 > @@ -881,6 +881,7 @@ the same as for the function notmuch-tree."\r
 286 >        (message-arg "--entire-thread"))\r
 287 >      (if (equal (car (process-lines notmuch-command "count" search-args)) "0")\r
 288 >       (setq search-args basic-query))\r
 289 > +    (notmuch-tag-clear-cache)\r
 290 >      (let ((proc (notmuch-start-notmuch\r
 291 >                "notmuch-tree" (current-buffer) #'notmuch-tree-process-sentinel\r
 292 >                "show" "--body=false" "--format=sexp"\r
 293 > diff --git a/emacs/notmuch.el b/emacs/notmuch.el\r
 294 > index 0471750..0c767f7 100644\r
 295 > --- a/emacs/notmuch.el\r
 296 > +++ b/emacs/notmuch.el\r
 297 > @@ -888,6 +888,7 @@ the configured default sort order."\r
 298 >      (set 'notmuch-search-oldest-first oldest-first)\r
 299 >      (set 'notmuch-search-target-thread target-thread)\r
 300 >      (set 'notmuch-search-target-line target-line)\r
 301 > +    (notmuch-tag-clear-cache)\r
 302 >      (let ((proc (get-buffer-process (current-buffer)))\r
 303 >         (inhibit-read-only t))\r
 304 >        (if proc\r
 305 > -- \r
 306 > 1.7.9.1\r