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

[Philip Kaludercic]

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

> 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.

See `display-buffer-assq-regexp' in window.el.

>>>     (`(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)?  

Yes, that was a copy-o on my end.

>                                          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.

Exactly, but if the byte-compilation would take place in buffer-match-p,
then the measurement is relevant.

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

The rationale is the same as for regular expressions in the core.  They
are also compiled and stored, to avoid the need for them to be
interpreted over and over again.

This should all be explained in the bug report I pointed you to, and
where this discussion should continue.

>> 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?

This is currently not the case, but if the language extended in the
future, there is the possibility that naming conflicts could arise.  I
am just following the same principle used when writing macros that
avoids name capturing.