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

["João Távora" <joaotavora@gmail.com>]

Philip Kaludercic <philipk@posteo.net> writes:

> João Távora <joaotavora@gmail.com> writes:
>
>> Philip Kaludercic <philipk@posteo.net> writes:
>>
>>>> Yes, do that, but use byte-compile instead, not eval.
>>> I have tried both, and it doesn't appear to be a particular advantage
>>> one way or another.  That being said, this approach is *a lot* faster,
>>> to the point that I first assumed it was broken:
>>
>> Yes, this approach is always going to be much faster than the "naive"
>> approach.  Now I've taken your code as a starting point, simplified it,
>> and I get a reasonable/typical 3.5x speedup when I use a
>> byte-compilation strategy, so one of us isn't measuring
>>
>> ```elisp
>> (defun translate-buffer-condition-1 (condition)
>>   (pcase-exhaustive condition
>>     ((or 't 'nil)
>>      condition)
>>     ((pred stringp)
>>      `(string-match-p ,condition (buffer-name buffer)))
>>     ((pred functionp)
>>      `(,condition buffer))
>
> This would break the current behaviour, because `buffer-match-p'
> requires the function be called with an additional argument if possible.
>
> This is fundamental for `display-buffer-alist' to work.

I couldn't figure out where this argument arise or who should provides
it (the condition?).  It wasn't clear.  At any rate, if you understand
this you can probably re-add it and I'm sure it won't make any
difference to the time.

>>     (`(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 ,(translate-buffer-condition-1 cond)))
>>     (`(or . ,conds)
>>      `(or ,@(mapcar #'translate-buffer-condition-1 conds)))
>>     (`(and . ,conds)
>>      `(and ,@(mapcar #'translate-buffer-condition-1 conds)))))
>>
>> (defun translate-buffer-condition (condition)
>>   `(lambda (buffer) ,(translate-buffer-condition-1 condition)))
>>
>> (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 . "\\` ")))
>>
>> (defvar form (translate-buffer-condition sample-condition))
>> (defvar compiled (byte-compile form))
>>
>> (benchmark-run 100000 (funcall (eval form) (current-buffer))) ;; (0.397404883 3 0.18942550900000032)
>> (benchmark-run 100000 (funcall compiled (current-buffer))) ;; (0.113651836 0 0.0)
>> ```
>>
>> I couldn't understand the need for a hash table or special symbol vars
>> or what that "arg" was, so I took it out, but it shouldn't make a
>> difference.
>
> The hash table makes a significant different, try evaluating
>
>    (benchmark-run 100000 (funcall (byte-compile compiled) (current-buffer))) ;; (0.113651836 0 0.0)
>

You seem to be suggesting to byte-compile a compiled object which is
odd.  Did you mean (byte-compile form)?  More importantly, you're
forcing the byte-compilation process to run every one of those 100000
repetitions, which is not something we want to measure: the point of any
code compilation is to do it once and then reuse the results of
compilation many times.

(and I'm still confused by the purpose of the hash table usage)

> The fresh symbols are used to keep the code clean and avoid possible
> naming conflicts.

Can you explain what naming conflict would arise or how the code I
provided is somehow unclean?