Subject: bug#53749: 29.0.50; [PATCH] Xref backend for TeX buffers

From: Dmitry Gutov <[email-masked]>
Date: Wed, 15 May 2024 00:24:24 +0300

On 13/05/2024 23:54, Stefan Monnier via Bug reports for GNU Emacs, the 
Swiss army knife of text editors wrote:
>> For now, I've pushed a fix in 86187d43e2d which seems to handle David's
>> scenario and address your review comment as well.
> 
> The let-binding is done outside of `with-current-buffer`, so it relies
> on the fact (itself problematic) that `inhibit-modification-hooks` is
> not buffer-local.

Good point.

> Would it be OK to use a patch like the one below?

Sure, thank you. Pushed.

> IIUC, in the `syntax-needed` case, the let-binding of
> `inhibit-modification-hooks` is just not useful very (4-7% is not worth
> the trouble), so its purpose is to speed up the other case.

4-10% is the improvement for both cases (the "syntax needed" and not).

I could be on the fence whether it's "not useful" - on the one hand 4% 
might not sound like much - on the other we already have this bit of 
improvement which has no known bugs. And when you combine a few of such 
performance hacks, the difference gets more noticeable.

Also, I'm eyeing another performance improvement (simplifying file type 
detection) - the call to set-auto-mode is not fast. Simply commenting 
this call out improves the performance by 4x or so - but we'll need a 
simpler version of it instead, of course.

And with the above change (commenting out the set-auto-mode call), the 
difference that the inhibit-modification-hooks hack makes is amplified: 
it can get up to 20%. Ultimately it'll be somewhere in between, but this 
sounds better, right?

> Also, what about the other two bindings of `inhibit-modification-hooks`?

The other two are used while the contents of the Xref buffer are printed 
(or re-printed), so there's none of the syntax-ppss complications there. 
The performance difference is 8.5% in my last measurement.

> 
>          Stefan
> 
> 
> diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
> index f9faec1b474..214e9cb6c09 100644
> --- a/lisp/progmodes/xref.el
> +++ b/lisp/progmodes/xref.el
> @@ -1282,7 +1282,7 @@ xref--show-common-initialize
>       (erase-buffer)
>       (setq overlay-arrow-position nil)
>       (xref--insert-xrefs xref-alist)
> -    (add-hook 'post-command-hook 'xref--apply-truncation nil t)
> +    (add-hook 'post-command-hook #'xref--apply-truncation nil t)
>       (goto-char (point-min))
>       (setq xref--original-window (assoc-default 'window alist)
>             xref--original-window-intent (assoc-default 'display-action alist))
> @@ -2112,10 +2112,7 @@ xref--convert-hits
>   (defun xref--collect-matches (hit regexp tmp-buffer syntax-needed)
>     (pcase-let* ((`(,line ,file ,text) hit)
>                  (file (and file (concat xref--hits-remote-id file)))
> -               (buf (xref--find-file-buffer file))
> -               ;; This is fairly dangerouns, but improves performance
> -               ;; for large lists, see https://debbugs.gnu.org/53749#227
> -               (inhibit-modification-hooks t))
> +               (buf (xref--find-file-buffer file)))
>       (if buf
>           (with-current-buffer buf
>             (save-excursion
> @@ -2130,6 +2130,9 @@
>         ;; Using the temporary buffer is both a performance and a buffer
>         ;; management optimization.
>         (with-current-buffer tmp-buffer
> +        ;; This let is fairly dangerouns, but improves performance
> +        ;; for large lists, see https://debbugs.gnu.org/53749#227
> +        (let ((inhibit-modification-hooks t))
>           (erase-buffer)
>           (when (and syntax-needed
>                      (not (equal file xref--temp-buffer-file-name)))
> @@ -2144,7 +2147,7 @@
>             (setq-local xref--temp-buffer-file-name file)
>             (setq-local inhibit-read-only t)
>             (erase-buffer))
> -        (insert text)
> +          (insert text))
>           (goto-char (point-min))
>           (when syntax-needed
>             (syntax-ppss-flush-cache (point)))
> 
> 
> 
>