bug#58950: [PATCH] * lisp/subr.el (buffer-match-p): Optimise performance

bug#58950: [PATCH] * lisp/subr.el (buffer-match-p): Optimise performance

[Philip Kaludercic]

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

Tags: patch


The below patch is based on a tangent discussion in bug#58839, the below
patch was written in collaboration with João Távora.  It involves an
optimisation to `buffer-match-p' that dramatically speeds the execution
of the function.  This is important for the very least as
`buffer-match-p' is used for displaying buffer.

Running (benchmark-run 1000 (match-buffers "\\*.+\\*")) I previously got
(22.822269875 178 15.524474267999977), and with the patch applied
(0.27100275 2 0.1730835160000197).

There are a few points that can be discussed:

1. Style.  I wrap the defun in a let (or rather letrec) block to avoid
   littering the global namespace.  It isn't necessary, and one could
   argue it makes debugging more difficult.

2. Caching policy.  Caching is critical to this optimisation.  Just
   using byte-compilation would cause the above test to slow down to
   (76.323692627 656 57.088315405).  The question is if the hash map
   will collect too much garbage over time, and if there is a better
   approach that could be taken?

In GNU Emacs 29.0.50 (build 3, x86_64-pc-linux-gnu, GTK+ Version
 3.24.30, cairo version 1.16.0) of 2022-10-31 built on heron
Repository revision: 462a66e79edcc34ecbeef7cc1604765adfdc038e
Repository branch: feature/package+vc
System Description: Guix System

Configured using:
 'configure --with-pgtk --with-imagemagick
 PKG_CONFIG_PATH=/gnu/store/ssg343s6ldqdwh30136pnawhbgd0cb6i-profile/lib/pkgconfig:/gnu/store/ssg343s6ldqdwh30136pnawhbgd0cb6i-profile/share/pkgconfig'


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0002-lisp-subr.el-buffer-match-p-Accelerate-using-byte-co.patch --]
[-- Type: text/patch, Size: 4214 bytes --]

From 0a9ddbcc6958fa7ed94456722a3eee65582a56b2 Mon Sep 17 00:00:00 2001
From: Philip Kaludercic <philipk@posteo.net>
Date: Tue, 1 Nov 2022 19:57:49 +0100
Subject: [PATCH] * lisp/subr.el (buffer-match-p): Optimise performance

---
 lisp/subr.el | 75 +++++++++++++++++++++++++++-------------------------
 1 file changed, 39 insertions(+), 36 deletions(-)

diff --git a/lisp/subr.el b/lisp/subr.el
index 83e2e75c41..0dd7a814d9 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -7002,8 +7002,38 @@ string-lines
             (setq start (length string)))))
       (nreverse lines))))
 
-(defun buffer-match-p (condition buffer-or-name &optional arg)
-  "Return non-nil if BUFFER-OR-NAME matches CONDITION.
+(letrec ((buffer-sym (make-symbol "buffer"))
+         (arg-sym (make-symbol "arg"))
+         (translate
+          (lambda (condition)
+            "Compile a CONDITION into a predicate function."
+            (pcase-exhaustive condition
+              ((or 't 'nil)
+               condition)
+              ((pred stringp)
+               `(string-match-p ,condition (buffer-name ,buffer-sym)))
+              ((pred functionp)
+               (if (eq 1 (cdr (func-arity condition)))
+                   `(condition ,buffer-sym)
+                 `(condition
+                   ,buffer-sym
+                   ,arg-sym)))
+              (`(major-mode . ,mode)
+               `(eq (buffer-local-value 'major-mode ,buffer-sym)
+                    ',mode))
+              (`(derived-mode . ,mode)
+               `(provided-mode-derived-p
+                 (buffer-local-value 'major-mode ,buffer-sym)
+                 ',mode))
+              (`(not . ,cond)
+               `(not ,(funcall translate cond)))
+              (`(or . ,conds)
+               `(or ,@(mapcar translate conds)))
+              (`(and . ,conds)
+               `(and ,@(mapcar translate conds))))))
+         (cond-cache (make-hash-table :test 'eq)))
+  (defun buffer-match-p (condition buffer-or-name &optional arg)
+    "Return non-nil if BUFFER-OR-NAME matches CONDITION.
 CONDITION is either:
 - the symbol t, to always match,
 - the symbol nil, which never matches,
@@ -7022,40 +7052,13 @@ buffer-match-p
     to be met.
   * `or': the cdr is a list of recursive condition, of which at
     least one has to be met."
-  (letrec
-      ((buffer (get-buffer buffer-or-name))
-       (match
-        (lambda (conditions)
-          (catch 'match
-            (dolist (condition conditions)
-              (when (pcase condition
-                      ('t t)
-                      ((pred stringp)
-                       (string-match-p condition (buffer-name buffer)))
-                      ((pred functionp)
-                       (if (eq 1 (cdr (func-arity condition)))
-                           (funcall condition buffer)
-                         (funcall condition buffer arg)))
-                      (`(major-mode . ,mode)
-                       (eq
-                        (buffer-local-value 'major-mode buffer)
-                        mode))
-                      (`(derived-mode . ,mode)
-                       (provided-mode-derived-p
-                        (buffer-local-value 'major-mode buffer)
-                        mode))
-                      (`(not . ,cond)
-                       (not (funcall match cond)))
-                      (`(or . ,args)
-                       (funcall match args))
-                      (`(and . ,args)
-                       (catch 'fail
-                         (dolist (c args)
-                           (unless (funcall match (list c))
-                             (throw 'fail nil)))
-                         t)))
-                (throw 'match t)))))))
-    (funcall match (list condition))))
+    (funcall (or (gethash condition cond-cache)
+                 (puthash condition
+                          (byte-compile
+                           `(lambda (,buffer-sym ,arg-sym)
+                              ,(funcall translate condition)))
+                          cond-cache))
+             (get-buffer buffer-or-name) arg)))
 
 (defun match-buffers (condition &optional buffers arg)
   "Return a list of buffers that match CONDITION.
-- 
2.38.0

bug#58950: [PATCH] * lisp/subr.el (buffer-match-p): Optimise performance

[Philip Kaludercic]

Philip Kaludercic <philipk@posteo.net> writes:

> Tags: patch
>
>
> The below patch is based on a tangent discussion in bug#58839, the below
> patch was written in collaboration with João Távora.  It involves an
> optimisation to `buffer-match-p' that dramatically speeds the execution
> of the function.  This is important for the very least as
> `buffer-match-p' is used for displaying buffer.
>
> Running (benchmark-run 1000 (match-buffers "\\*.+\\*")) I previously got
> (22.822269875 178 15.524474267999977), and with the patch applied
> (0.27100275 2 0.1730835160000197).
>
> There are a few points that can be discussed:
>
> 1. Style.  I wrap the defun in a let (or rather letrec) block to avoid
>    littering the global namespace.  It isn't necessary, and one could
>    argue it makes debugging more difficult.
>
> 2. Caching policy.  Caching is critical to this optimisation.  Just
>    using byte-compilation would cause the above test to slow down to
>    (76.323692627 656 57.088315405).  The question is if the hash map
>    will collect too much garbage over time, and if there is a better
>    approach that could be taken?
>
> In GNU Emacs 29.0.50 (build 3, x86_64-pc-linux-gnu, GTK+ Version
>  3.24.30, cairo version 1.16.0) of 2022-10-31 built on heron
> Repository revision: 462a66e79edcc34ecbeef7cc1604765adfdc038e
> Repository branch: feature/package+vc
> System Description: Guix System
>
> Configured using:
>  'configure --with-pgtk --with-imagemagick
>  PKG_CONFIG_PATH=/gnu/store/ssg343s6ldqdwh30136pnawhbgd0cb6i-profile/lib/pkgconfig:/gnu/store/ssg343s6ldqdwh30136pnawhbgd0cb6i-profile/share/pkgconfig'

Ping?

bug#58950: [PATCH] * lisp/subr.el (buffer-match-p): Optimise performance

[Dmitry Gutov]

On 01.11.2022 21:11, Philip Kaludercic wrote:
> 1. Style.  I wrap the defun in a let (or rather letrec) block to avoid
>     littering the global namespace.  It isn't necessary, and one could
>     argue it makes debugging more difficult.
> 
> 2. Caching policy.  Caching is critical to this optimisation.  Just
>     using byte-compilation would cause the above test to slow down to
>     (76.323692627 656 57.088315405).  The question is if the hash map
>     will collect too much garbage over time, and if there is a better
>     approach that could be taken?

I'd like to let our language-level specialists to take the deeper look.

This approach looks the most straightforward, but there could be others, 
just like "compiling" the form inside defcustom setter (for 
project-kill-buffer-conditions, and every similar option), doing 
precompilation closer to where the rules are used (similar to 
font-lock-compile-keywords), or not doing any of that. All depending on 
how long a typical compilation takes, and how many buffers the user has 
to have, to see any noticeable benefit.

On the last note, I'm curious how many buffers would it take to see a 
50ms improvement in match-buffers' runtime when using the current 
project-kill-buffer-conditions's value, for example.

bug#58950: [PATCH] * lisp/subr.el (buffer-match-p): Optimise performance

[Philip Kaludercic]

Dmitry Gutov <dgutov@yandex.ru> writes:

> On 01.11.2022 21:11, Philip Kaludercic wrote:
>> 1. Style.  I wrap the defun in a let (or rather letrec) block to avoid
>>     littering the global namespace.  It isn't necessary, and one could
>>     argue it makes debugging more difficult.
>> 2. Caching policy.  Caching is critical to this optimisation.  Just
>>     using byte-compilation would cause the above test to slow down to
>>     (76.323692627 656 57.088315405).  The question is if the hash map
>>     will collect too much garbage over time, and if there is a better
>>     approach that could be taken?
>
> I'd like to let our language-level specialists to take the deeper look.
>
> This approach looks the most straightforward, but there could be
> others, just like "compiling" the form inside defcustom setter (for
> project-kill-buffer-conditions, and every similar option), doing
> precompilation closer to where the rules are used (similar to
> font-lock-compile-keywords), or not doing any of that. All depending
> on how long a typical compilation takes, and how many buffers the user
> has to have, to see any noticeable benefit.
>
> On the last note, I'm curious how many buffers would it take to see a
> 50ms improvement in match-buffers' runtime when using the current
> project-kill-buffer-conditions's value, for example.

Ping?  If this change is too controversial, I'd like to backport the
changes from bug#58951 and apply them since they are fixing an actual bug.

bug#58950: [PATCH] * lisp/subr.el (buffer-match-p): Optimise performance

[Dmitry Gutov]

On 31/12/2022 15:56, Philip Kaludercic wrote:
> Dmitry Gutov<dgutov@yandex.ru>  writes:
> 
>> On 01.11.2022 21:11, Philip Kaludercic wrote:
>>> 1. Style.  I wrap the defun in a let (or rather letrec) block to avoid
>>>      littering the global namespace.  It isn't necessary, and one could
>>>      argue it makes debugging more difficult.
>>> 2. Caching policy.  Caching is critical to this optimisation.  Just
>>>      using byte-compilation would cause the above test to slow down to
>>>      (76.323692627 656 57.088315405).  The question is if the hash map
>>>      will collect too much garbage over time, and if there is a better
>>>      approach that could be taken?
>> I'd like to let our language-level specialists to take the deeper look.
>>
>> This approach looks the most straightforward, but there could be
>> others, just like "compiling" the form inside defcustom setter (for
>> project-kill-buffer-conditions, and every similar option), doing
>> precompilation closer to where the rules are used (similar to
>> font-lock-compile-keywords), or not doing any of that. All depending
>> on how long a typical compilation takes, and how many buffers the user
>> has to have, to see any noticeable benefit.
>>
>> On the last note, I'm curious how many buffers would it take to see a
>> 50ms improvement in match-buffers' runtime when using the current
>> project-kill-buffer-conditions's value, for example.
> Ping?  If this change is too controversial, I'd like to backport the
> changes from bug#58951 and apply them since they are fixing an actual bug.

It does look too ambitious for the release branch.

On the merits of the change itself, maybe Mattias or Stefan have some 
thoughts?

bug#58950: [PATCH] * lisp/subr.el (buffer-match-p): Optimise performance

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>>>> 2. Caching policy.  Caching is critical to this optimisation.  Just
>>>>      using byte-compilation would cause the above test to slow down to
>>>>      (76.323692627 656 57.088315405).  The question is if the hash map
>>>>      will collect too much garbage over time, and if there is a better
>>>>      approach that could be taken?

You could make the hash table key-weak (since the test is `eq` it will
have no detrimental effect and will avoid most risks of leaks).

>>> I'd like to let our language-level specialists to take the deeper look.

Do we have any reason to believe that the performance of
`buffer-match-p` is a problem in `display-buffer-alist`?

The benchmark you quote seems to be fairly different from what
`display-buffer` does.  I'm not surprised your optimization improves
this benchmark, but I'm wondering whether this use-case corresponds to
a real life situation (and if so which).

>>> On the last note, I'm curious how many buffers would it take to see a
>>> 50ms improvement in match-buffers' runtime when using the current
>>> project-kill-buffer-conditions's value, for example.

Also, where is `match-buffers` used?  I only see it used in
`lisp/net/rcirc.el` in a way that can trivially be replaced with
something much more efficient.

To be clear: I don't much like the kind of mini-language we invented for
`buffer-match-p`.  I'd prefer we just used plain old ELisp for that.
It's a bit more verbose for that particular application, but:
- we have a much more efficient interpreter at hand.
- it's useful for many more things, so it's much wore valuable to
  learn it.
- it's a lot more powerful/general.


        Stefan

bug#58950: [PATCH] * lisp/subr.el (buffer-match-p): Optimise performance

[Mattias Engdegård]

5 jan. 2023 kl. 05.31 skrev Stefan Monnier <monnier@iro.umontreal.ca>:

> I don't much like the kind of mini-language we invented for
> `buffer-match-p`.  I'd prefer we just used plain old ELisp for that.

Yes, I sort of wondered whether we were going full Greenspun here. We haven't added many embedded languages into our embedded language lately.

The usual DSL worthiness criteria:

- more expressive than plain code for the domain
- potential for significantly better performance
- better error-checking, statically or dynamically
- structured editing

etc, don't really seem to be met here but I'm not deeply familiar with the problem and perhaps the author could make a better case for it.

bug#58950: [PATCH] * lisp/subr.el (buffer-match-p): Optimise performance

[Dmitry Gutov]

On 05/01/2023 12:31, Mattias Engdegård wrote:
> 5 jan. 2023 kl. 05.31 skrev Stefan Monnier<monnier@iro.umontreal.ca>:
> 
>> I don't much like the kind of mini-language we invented for
>> `buffer-match-p`.  I'd prefer we just used plain old ELisp for that.
> Yes, I sort of wondered whether we were going full Greenspun here. We haven't added many embedded languages into our embedded language lately.
> 
> The usual DSL worthiness criteria:
> 
> - more expressive than plain code for the domain
> - potential for significantly better performance
> - better error-checking, statically or dynamically
> - structured editing
> 
> etc, don't really seem to be met here but I'm not deeply familiar with the problem and perhaps the author could make a better case for it.

Note that we already had ah-hoc and informally-specified pieces of such 
DSL before, in font-lock-global-modes and display-buffer-alist. The new 
use in show-paren-predicate seems fitting too.

I'm not sure how we'd reach the same goals with plain old Elisp 
(structured editing in particular -- in Customize).

bug#58950: [PATCH] * lisp/subr.el (buffer-match-p): Optimise performance

[Mattias Engdegård]

5 jan. 2023 kl. 13.55 skrev Dmitry Gutov <dgutov@yandex.ru>:

> I'm not sure how we'd reach the same goals with plain old Elisp (structured editing in particular -- in Customize).

No enemy of little DSLs in principle but is that structural editing the main rationale now?

(I wish we had (byte-)compiled elisp functions carrying their own source, either as s-exp, string of formatted source text, or source file reference -- that would allow for sensible editing in Customise without performance penalty. But Santa gave me a wool jumper instead, that's nice too.)

Regarding buffer-match-p, the fact that `not` actually means `nor` is a bit odd (we don't do that elsewhere), as well as arbitrary (why not `nand` etc) and undocumented. And like all the rest of the machinery, untested.

bug#58950: [PATCH] * lisp/subr.el (buffer-match-p): Optimise performance

[Dmitry Gutov]

On 06/01/2023 13:17, Mattias Engdegård wrote:
> 5 jan. 2023 kl. 13.55 skrev Dmitry Gutov <dgutov@yandex.ru>:
> 
>> I'm not sure how we'd reach the same goals with plain old Elisp (structured editing in particular -- in Customize).
> 
> No enemy of little DSLs in principle but is that structural editing the main rationale now?

I think so? And the fact that it's more limited than Elisp means the 
values have to be uniform-ish. As a result they're easier to quickly grasp.

> (I wish we had (byte-)compiled elisp functions carrying their own source, either as s-exp, string of formatted source text, or source file reference -- that would allow for sensible editing in Customise without performance penalty. But Santa gave me a wool jumper instead, that's nice too.)

We kind of have that already, if we just made the type for be 'sexp', or 
a Lisp form. With all the freedom associated with it, just lower 
performance compared to a compiled function.

Would that look like a good choice for e.g. font-lock-global-modes? I 
don't think the performance hit would be a problem for that use.

> Regarding buffer-match-p, the fact that `not` actually means `nor` is a bit odd (we don't do that elsewhere), as well as arbitrary (why not `nand` etc) and undocumented.

Yeah, it's a wrinkle. I'm on the fence regarding changing it, though, 
for compatibility and ergonomical reasons (it's easier for the user to 
avoid typing a dot).

But I'm not married to it either.

bug#58950: [PATCH] * lisp/subr.el (buffer-match-p): Optimise performance

[Mattias Engdegård]

6 jan. 2023 kl. 22.41 skrev Dmitry Gutov <dgutov@yandex.ru>:

>> (I wish we had (byte-)compiled elisp functions carrying their own source, either as s-exp, string of formatted source text, or source file reference -- that would allow for sensible editing in Customise without performance penalty. But Santa gave me a wool jumper instead, that's nice too.)
> 
> We kind of have that already, if we just made the type for be 'sexp', or a Lisp form. With all the freedom associated with it, just lower performance compared to a compiled function.

That's short in two respects: performance, and non-retention of formatting and comments. We could store source refs in:

- a hash table weakly keyed on the code object
- some back corner of byte-code objects
- OClosures

We have a similar problem with regexps, which are only retained and edited in the traditional syntax.

We are veering off-topic. Sorry about that.

> I'm on the fence regarding changing it, though, for compatibility and ergonomical reasons (it's easier for the user to avoid typing a dot).

Is compatibility a serious concern given that buffer-match-p is new in 29?

bug#58950: [PATCH] * lisp/subr.el (buffer-match-p): Optimise performance

[Dmitry Gutov]

On 07/01/2023 14:57, Mattias Engdegård wrote:
> 6 jan. 2023 kl. 22.41 skrev Dmitry Gutov <dgutov@yandex.ru>:
> 
>>> (I wish we had (byte-)compiled elisp functions carrying their own source, either as s-exp, string of formatted source text, or source file reference -- that would allow for sensible editing in Customise without performance penalty. But Santa gave me a wool jumper instead, that's nice too.)
>>
>> We kind of have that already, if we just made the type for be 'sexp', or a Lisp form. With all the freedom associated with it, just lower performance compared to a compiled function.
> 
> That's short in two respects: performance, and non-retention of formatting and comments. We could store source refs in:
> 
> - a hash table weakly keyed on the code object
> - some back corner of byte-code objects
> - OClosures

That's still seems like an overkill for font-lock-global-modes. And it's 
hard to draw a line between defcustoms which should have their own 
mini-languages, and those that do not.

Or to look at it the other way -- perhaps the line has been drawn now, 
since we haven't received any further requests for extending the syntax.

> We have a similar problem with regexps, which are only retained and edited in the traditional syntax.
> 
> We are veering off-topic. Sorry about that.
> 
>> I'm on the fence regarding changing it, though, for compatibility and ergonomical reasons (it's easier for the user to avoid typing a dot).
> 
> Is compatibility a serious concern given that buffer-match-p is new in 29?

This close to a release, it most likely is.

Another compatibility goal (though probably not a hard requirement) is 
being able to replace project--buffer-check with the standard code, 
while keeping all existing user customizations in 
project-kill-buffer-conditions and project-ignore-buffer-conditions.

The alternative would be to have a prolonged migration procedure.

bug#58950: [PATCH] * lisp/subr.el (buffer-match-p): Optimise performance

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

BTW, while this is an interesting discussion, I'm not sure it matters
very much.  The core question is still the same: does this performance
issue matter?  For `display-buffer`, AFAICT we only consider a single
buffer, so I can't imagine `buffer-match-p` being an important factor to
the overall performance.


        Stefan

bug#58950: [PATCH] * lisp/subr.el (buffer-match-p): Optimise performance

[Dmitry Gutov]

On 05/01/2023 06:31, Stefan Monnier via Bug reports for GNU Emacs, the 
Swiss army knife of text editors wrote:
>>>> I'd like to let our language-level specialists to take the deeper look.
> Do we have any reason to believe that the performance of
> `buffer-match-p` is a problem in `display-buffer-alist`?
> 
> The benchmark you quote seems to be fairly different from what
> `display-buffer` does.  I'm not surprised your optimization improves
> this benchmark, but I'm wondering whether this use-case corresponds to
> a real life situation (and if so which).

I was also wondering that.

>>>> On the last note, I'm curious how many buffers would it take to see a
>>>> 50ms improvement in match-buffers' runtime when using the current
>>>> project-kill-buffer-conditions's value, for example.
> Also, where is `match-buffers` used?  I only see it used in
> `lisp/net/rcirc.el` in a way that can trivially be replaced with
> something much more efficient.

I suppose it could replace the use of dolist+project--buffer-check 
inside project--buffers-to-kill. But the main target of the patch under 
discussion is buffer-match-p, of course.

bug#58950: [PATCH] * lisp/subr.el (buffer-match-p): Optimise performance

[Dmitry Gutov]

On 31/12/2022 15:56, Philip Kaludercic wrote:
> I'd like to backport the
> changes from bug#58951 and apply them since they are fixing an actual bug.

Speaking of backporting, though, could you address Eli's last message in 
bug#58951? Also, he seems to have reverted the change in the code, but 
not in the docstring.

Could you also clarify this for me: which of the semantics for 'not' is 
going to make 'buffer-match-p' a compatible replacement for 
project--buffer-check?

I assume we do want to drop the custom interpreter inside project.el 
someday.

bug#58950: [PATCH] * lisp/subr.el (buffer-match-p): Optimise performance

[Eli Zaretskii]

> Cc: 58950@debbugs.gnu.org
> Date: Thu, 5 Jan 2023 02:02:06 +0200
> From: Dmitry Gutov <dgutov@yandex.ru>
> 
> On 31/12/2022 15:56, Philip Kaludercic wrote:
> > I'd like to backport the
> > changes from bug#58951 and apply them since they are fixing an actual bug.
> 
> Speaking of backporting, though, could you address Eli's last message in 
> bug#58951? Also, he seems to have reverted the change in the code, but 
> not in the docstring.

I hoped the problem would be fixed very soon after the revert.  If
this is not going to happen, please adjust the doc string, or tell me
how to do that.

bug#58950: [PATCH] * lisp/subr.el (buffer-match-p): Optimise performance

[Dmitry Gutov]

On 05/01/2023 08:32, Eli Zaretskii wrote:
>> Cc:58950@debbugs.gnu.org
>> Date: Thu, 5 Jan 2023 02:02:06 +0200
>> From: Dmitry Gutov<dgutov@yandex.ru>
>>
>> On 31/12/2022 15:56, Philip Kaludercic wrote:
>>> I'd like to backport the
>>> changes from bug#58951 and apply them since they are fixing an actual bug.
>> Speaking of backporting, though, could you address Eli's last message in
>> bug#58951? Also, he seems to have reverted the change in the code, but
>> not in the docstring.
> I hoped the problem would be fixed very soon after the revert.  If
> this is not going to happen, please adjust the doc string, or tell me
> how to do that.

Looking more into it, it actually seems like the current code and 
doscstring work okay together.

The original description of bug#58951 seems a little confused, given 
that show-paren-predicate was already written in the style

   (not CONDITION)

and buffer-match-p was apparently working fine with it already. The 
revert that you did brought the previous (working) code back. The 
dosctring was problematic because it didn't quite match the behavior, 
and Philip's change fixed that.

The dosctring can say (as it does now) that

     * `not': the cadr is interpreted as a negation of a condition.

or it could more accurately say

     * `or': the cdr is a list of recursive conditions, of which at
     least one has to be met

as project-kill-buffer-conditions does. These two descriptions are 
compatible, however, as long as one only puts 1 item in said list. So we 
can sweep the difference under the rug as "implementation details".

And maybe there's nothing more to do in there.

Philip's patch in this bug, however, (the "Optimise performance" one) 
changes the semantics of 'not' in a way that would require us to change 
show-paren-predicate to '(not . (derived-mode . special-mode)).

I'm not sure if it's intended or not. Let's see what he says on that.