bug#46859: 28.0.50; [PATCH]: Add option to truncate long lines in xref.el

[Theodor Thornhill]

Hi again!
Dmitry Gutov <dgutov@yandex.ru> writes:

> On 03.03.2021 00:14, Theodor Thornhill via Bug reports for GNU Emacs, 
> the Swiss army knife of text editors wrote:
>> I'm interested in seeing if I could gain some more
>> performance by short circuiting after the first iteration of a match on
>> the same line.  In my test scenario there are a lot of matches on the
>> same huge line.  What do you think?

I couldn't really find any approaches that yielded better results with
short-circuiting in mind, so I dropped that idea.

> You probably mean to short-circuit as soon as you reach the target 

>
> ...of course, ideally we would keep all contents of the line somewhere 
> in memory and truncate with (setq truncate-line t). But IIRC Juri said 
> this didn't give us as much of a speedup as we'd want.
>
> Another question: how many hits do you usually have in that huge 
> one-line file? If it's more than 2-3, it might be that our current 
> algorithm which creates "match objects" will duplicate this string 
> unnecessarily N times (which is the number of hits), in 
> xref--collect-matches-1, to then cut it up and merge into one line again 
> when printing the buffer. In which case the patch above should also show 
> a healthy improvement, but we could figure out something better instead.
>

This long line has 25 matches, so yeah, that takes some time. With your
hint here I tried another approach which yielded some nice results.

Ok, some benchmarks:

;; With nothing
(benchmark-run 10 (project-find-regexp "UrlChange")) ;; (11.748253 14 
0.23526199999999997)

;; With -M 500
(benchmark-run 10 (project-find-regexp "UrlChange")) ;; (0.293626 0 0.0)

;; My first patch
(benchmark-run 10 (project-find-regexp "UrlChange")) ;; (1.230833 8 
0.13783999999999996)

;; Dmitrys patch
(benchmark-run 10 (project-find-regexp "UrlChange")) ;; (1.007787 0 0.0)

;; Latest diff (attached at the bottom)
(benchmark-run 10 (project-find-regexp "UrlChange")) ;; (1.0351299999999999 0 
0.0)


So there are some interesting findings here:

- There are some improvements to gain
- None so far kills "-M 500"
- Pretty close between Dmitrys and my last patch

However, only my patch actually renders the long file as a match in the
output buffer. All the others seem to drop it altogether. IMO that is
one point in favour of my approaches. In addition, we could add another
defcustom for the xref--collect-matches-1 function,
"xref--collect-all-matches-p" or something like that. Retrofitting the
current variable seems a little off. That means you could customize xref
to render the whole long line if you want, while not bothering about
multiple matches. Not sure if that has a great benefit, though.

What do you think? Are any of these approaches worth pursuing further?

--
Theo


diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
index 18fdd963fb..fb422dcffa 100644
--- a/lisp/progmodes/xref.el
+++ b/lisp/progmodes/xref.el
@@ -872,6 +872,18 @@ beginning of the line."
       (xref--search-property 'xref-item))
   (xref-show-location-at-point))
 
+(defcustom xref-truncate-line-to 500
+  "Max number of columns to display in xref buffer."
+  :type '(choice
+          (fixnum :tag "Number of lines")
+          (null :tag "Don't truncate"))
+  :version "28.1"
+  :package-version '(xref . "1.0.5"))
+
+(defun xref--truncate-long-lines-p (summary)
+  (and (numberp xref-truncate-line-to)
+       (> (length summary) xref-truncate-line-to)))
+
 (defun xref--insert-xrefs (xref-alist)
   "Insert XREF-ALIST in the current-buffer.
 XREF-ALIST is of the form ((GROUP . (XREF ...)) ...), where
@@ -902,14 +914,22 @@ GROUP is a string for decoration purposes and XREF is an
                                 "  ")))
                         ;; Render multiple matches on the same line, together.
                         (when (and line (equal prev-line-key line-key))
-                          (when-let ((column (xref-location-column location)))
-                            (delete-region
-                             (save-excursion
-                               (forward-line -1)
-                               (move-to-column (+ (length prefix) column))
+                          (if (xref--truncate-long-lines-p summary)
+                              (delete-region
+                               (save-excursion (forward-line -1) (point))
+                               (point))
+                            (when-let ((column (xref-location-column 
location)))
+                              (delete-region
+                               (save-excursion
+                                 (forward-line -1)
+                                 (move-to-column (+ (length prefix) column))
+                                 (point))
                                (point))
-                             (point))
-                            (setq new-summary (substring summary column) 
prefix "")))
+                              (setq new-summary (substring summary column) 
prefix ""))))
+                        (when (xref--truncate-long-lines-p new-summary)
+                          (setq new-summary
+                                (concat (substring new-summary 0 
xref-truncate-line-to)
+                                        " (...truncated)")))
                         (xref--insert-propertized
                          (list 'xref-item xref
                                'mouse-face 'highlight
@@ -1678,7 +1698,7 @@ Such as the current syntax table and the applied syntax 
properties."
                                  syntax-needed)))))
 
 (defun xref--collect-matches-1 (regexp file line line-beg line-end 
syntax-needed)
-  (let (matches)
+  (let (matches prev-line)
     (when syntax-needed
       (syntax-propertize line-end))
     ;; FIXME: This results in several lines with the same
@@ -1688,14 +1708,18 @@ Such as the current syntax table and the applied syntax 
properties."
             (or (null matches)
                 (> (point) line-beg))
             (re-search-forward regexp line-end t))
-      (let* ((beg-column (- (match-beginning 0) line-beg))
-             (end-column (- (match-end 0) line-beg))
-             (loc (xref-make-file-location file line beg-column))
-             (summary (buffer-substring line-beg line-end)))
-        (add-face-text-property beg-column end-column 'xref-match
-                                t summary)
-        (push (xref-make-match summary loc (- end-column beg-column))
-              matches)))
+
+      (unless (and (eq prev-line line)
+                   (numberp xref-truncate-line-to))
+        (let* ((beg-column (- (match-beginning 0) line-beg))
+               (end-column (- (match-end 0) line-beg))
+               (loc (xref-make-file-location file line beg-column))
+               (summary (buffer-substring line-beg line-end)))
+          (add-face-text-property beg-column end-column 'xref-match
+                                  t summary)
+          (push (xref-make-match summary loc (- end-column beg-column))
+                matches)))
+      (setq prev-line line))
     (nreverse matches)))
 
 (defun xref--find-file-buffer (file)