bug#58839: [Patch] Re: bug#58839: 29.0.50; project-kill-buffer fails when Eglot is running

[João Távora]

I haven't studied your code in depth, but it seems like you're giving

`match-buffers/compiled` benchmark 10 times the work you're giving to

the other function, which would explain why it's 10x slower.

The byte-compiler (or the native compiler) can't really optimize the

mini-language more magically.  It can only optimize elisp.

My idea of using the byte-compiler to do this is different: it entails

translating the mini-language to elisp first and then byte-compiling

that.  But it is a technique that I think your code isn't applying

or at least not correctly (though I haven't read all of it: I will soon).

You can see eglot's "glob matching" section for the application of

such a technique the "glob" minilanguage that is required by LSP (iow

it wasn't "invented by me" ;-) )

João

On Tue, Nov 1, 2022 at 1:03 PM Philip Kaludercic <philipk@posteo.net> wrote:

João Távora <joaotavora@gmail.com> writes:

> On Tue, Nov 1, 2022 at 11:27 AM Philip Kaludercic <philipk@posteo.net>
> wrote:
>
>
>> E.g. `display-buffer-alist' makes use of it to associate display-buffer
>> rules with buffers.  Now you can add
>>
>>       ((major-mode . help-mode) display-buffer-in-side-window)
>>
>> instead of trying to match being a regular _expression_ to catch all
>> *Help* buffer names of a function along the lines of
>>
>>       (lambda (buf _alist)
>>         (with-current-buffer buf
>>           (derived-mode-p 'help-mode)))
>>
>
> If you really want to save up on this typing, it's better to define
> a reusable helper function, or even a higher order function.
>
>   (defun buffer-mode-matcher (mode)
>     (lambda (b _alist)
>       (with-current-buffer b (derived-mode-p 'help-mode))))
>
> You can add buffer-mode-matcher to the library if it becomes
> useful enough.  Then you add:
>
>   `(,(buffer-mode-matcher 'help-mode) display-buffer-in-side-window)
>
> to display-buffer-alist.
>
> But if you really want a new language your language, then I suggest
> a simple adapter buffer-matcher utility that merges the two.  That way one
> doesn't couple existing utilities to the new mini-language and
> simultaneously
> the new mini-language become useful in a much wider setting for those who
> appreciate such things.
>
>   (defun buffer-matcher (condition)
>      "Return unary predicate of a buffer matching the CONDITION
> mini-language."
>     (lambda (buf &rest _whatever) ; make it even more lax
>        (buffer-match-p condition)))
>
> Later on, you might even pass an (... &optional compiled) so that the
> return value
> is syntax checked and optimized in some way at compile time.
>
> IOW, (E)Lisp already gives you the tools for these composition without
> needing to invent new languages with the drawbacks I listed.

I was curious to try this out, and implemented something along the lines
of your suggestion.  The bad news is that it is at least 10 times slower
than the current implementation, that isn't even really optimised.
Perhaps I did something native and didn't see what is wrong, but here
are my notes:

--8<---------------cut here---------------start------------->8---
(defun translate-buffer-condition (condition)
  "Compile a CONDITION into a predicate function."
  (pcase-exhaustive condition
    ((or 't 'nil)
     (lambda (_buffer _arg)
       condition))
    ((pred stringp)
     (lambda (buffer _arg)
       (string-match-p condition (buffer-name buffer))))
    ((pred functionp)
     (if (eq 1 (cdr (func-arity condition)))
         (lambda (buffer _arg)
           (funcall condition buffer))
       condition))
    (`(major-mode . ,mode)
     (lambda (buffer _arg)
       (eq
        (buffer-local-value 'major-mode buffer)
        mode)))
    (`(derived-mode . ,mode)
     (lambda (buffer _arg)
       (provided-mode-derived-p
        (buffer-local-value 'major-mode buffer)
        mode)))
    (`(not . ,cond)
     (lambda (buffer arg)
       (not (funcall (translate-buffer-condition cond) buffer arg))))
    (`(or . ,conds)
     (lambda (buffer arg)
       (catch 'match
         (dolist (cond conds)
           (when (funcall (translate-buffer-condition cond) buffer arg)
             (throw 'match t))))))
    (`(and . ,conds)
     (lambda (buffer arg)
       (catch 'match
         (dolist (cond conds t)
           (when (funcall (translate-buffer-condition cond) buffer arg)
             (throw 'match nil))))))))

(defvar buffer-match-p-cache (make-hash-table :test 'eq))

(defun buffer-match-p/compiled (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,
- a regular _expression_, to match a buffer name,
- a predicate function that takes a buffer object and ARG as
  arguments, and returns non-nil if the buffer matches,
- a cons-cell, where the car describes how to interpret the cdr.
  The car can be one of the following:
  * `derived-mode': the buffer matches if the buffer's major mode
    is derived from the major mode in the cons-cell's cdr.
  * `major-mode': the buffer matches if the buffer's major mode
    is eq to the cons-cell's cdr.  Prefer using `derived-mode'
    instead when both can work.
  * `not': the cdr is interpreted as a negation of a condition.
  * `and': the cdr is a list of recursive conditions, that all have
    to be met.
  * `or': the cdr is a list of recursive condition, of which at
    least one has to be met."
  (funcall (or (gethash condition buffer-match-p-cache)
               (puthash condition
                        (byte-compile (translate-buffer-condition condition))
                        buffer-match-p-cache))
           (get-buffer buffer-or-name)
           arg))

(defun match-buffers/compiled (condition &optional buffers arg)
  "Return a list of buffers that match CONDITION.
See `buffer-match' for details on CONDITION.  By default all
buffers are checked, this can be restricted by passing an
optional argument BUFFERS, set to a list of buffers to check.
ARG is passed to `buffer-match', for predicate conditions in
CONDITION."
  (let (bufs)
    (dolist (buf (or buffers (buffer-list)))
      (when (buffer-match-p/compiled condition (get-buffer buf) arg)
        (push buf bufs)))
    bufs))

;; Here we will test a moderately complicated condition and time how
;; long it takes with the current implementation and with the proposed
;; alternative.

(defvar sample-condition
  '(and (or buffer-file-name
            (derived-mode . compilation-mode)
            (derived-mode . dired-mode)
            (derived-mode . diff-mode)
            (derived-mode . comint-mode)
            (derived-mode . eshell-mode)
            (derived-mode . change-log-mode))
        "\\*.+\\*"
        (not . "\\` ")))

(benchmark-run 100
  (match-buffers sample-condition pr))
;; => (1.7045469830000002 20 1.1418286690000023)


(benchmark-run 1000
  (match-buffers/compiled project-buffer-conditions pr))
;; => (17.646938126000002 219 12.428946030999999)
--8<---------------cut here---------------end--------------->8---

I guess this just goes to show that one shouldn't underestimate the cost
of a function call...

    LISP programmers know the value of everything and the cost of nothing.
         --  Alan Perlis

--

João Távora