bug#23223: 25.0.92; Can xref-find-references be sped up?

[Dmitry Gutov <dgutov@yandex.ru>]

[-- Attachment #1: Type: text/plain, Size: 2745 bytes --]

Please try the attached patch. It cuts the time to search for 
'current_buffer' from 4.5s to 0.75s here.

You can comment out different parts of xref--collect-matches, to test if 
there are any easy bottlenecks left. The only one I see is 
find-buffer-visiting; replacing it with get-file-buffer yields some 
extra boost, but it would come with corresponding downsides in those 
exact cases for which find-buffer-visiting was designed (although I can 
add another local variable instead that would cache the previous file 
name and buffer used for it; but that's scraping the bottom).

To measure, open src/xdisp.c and evaluate (benchmark 1 
'(xref-collect-references "current_buffer" default-directory)).

On 04/09/2016 10:25 AM, Eli Zaretskii wrote:

> Would it help to only use the mode's syntax table, and avoid switching
> on the major mode as a whole?

Not for performance anymore (see above). You can't always determine the 
syntax table from the major mode name (there is a convention, but it's 
not iron-clad), it might be in an autoloaded file, etc... And like I 
mentioned before, we also need syntax-propertize-function, and 
potentially any buffer-local variables that it could use.

> That problem is relevant for IDutils as well (the scanner is
> determined by the file's extension only), so we already have a certain
> (hopefully, small) number of misses and false positives.

That affects xref-find-references (as long as you use id-utils, which is 
not mandatory) but not project-find-regexp. The currently discussed 
tuneup should improve both.

> I think this
> cannot be entirely avoided.  So maybe we shouldn't try so hard
> avoiding false positives.

It seems we can't avoid ignoring the mode specification at the bottom, 
but that's about it. And nothing's stopping id-utils (or other tools) 
from using a better language detection scheme in the future.

> E.g., the "M-x gid" command, which comes
> with IDutils and is a trivial wrapper around lid invocation, simply
> shows the output in a Grep-like buffer, through which you can step
> with next-error.

Do you actually want xref-find-regexp, and not xref-find-references?

> It is lightning-fast: what takes 13 sec with
> xref-find-references, takes less than 2 sec with "M-x gid".

What's the new time you get from the former?

> Perhaps use insert-file-contents-literally, as decoding could slow
> down things considerably.

No significant difference here, on the given example.

By the way, the "insert-file-contents + set-auto-mode" dance comes with 
a new minor downside: extra chatter from the major modes. E.g. try 
project-file-regexp with "should have received a copy". We can avoid 
saving it to the message log, but it appears in the echo area either way.

[-- Attachment #2: xref-with-temp-buffer.diff --]
[-- Type: text/x-patch, Size: 11988 bytes --]

diff --git a/lisp/cedet/semantic/symref/cscope.el b/lisp/cedet/semantic/symref/cscope.el
index 4890b5b..3abd8b3 100644
--- a/lisp/cedet/semantic/symref/cscope.el
+++ b/lisp/cedet/semantic/symref/cscope.el
@@ -60,6 +60,9 @@ semantic-symref-tool-cscope
     (semantic-symref-parse-tool-output tool b)
     ))
 
+(defconst semantic-symref-cscope--line-re
+  "^\\([^ ]+\\) [^ ]+ \\([0-9]+\\) ")
+
 (cl-defmethod semantic-symref-parse-tool-output-one-line ((tool semantic-symref-tool-cscope))
   "Parse one line of grep output, and return it as a match list.
 Moves cursor to end of the match."
@@ -78,8 +81,13 @@ semantic-symref-tool-cscope
 	       ;; We have to return something at this point.
 	       subtxt)))
 	 )
-	(t
-	 (when (re-search-forward "^\\([^ ]+\\) [^ ]+ \\([0-9]+\\) " nil t)
+        ((eq (oref tool :resulttype) 'line-and-text)
+         (when (re-search-forward semantic-symref-cscope--line-re nil t)
+           (list (string-to-number (match-string 2))
+                 (expand-file-name (match-string 1))
+                 (buffer-substring-no-properties (point) (line-end-position)))))
+	(t ; :resulttype is 'line
+	 (when (re-search-forward semantic-symref-cscope--line-re nil t)
 	   (cons (string-to-number (match-string 2))
 		 (expand-file-name (match-string 1)))
 	   ))))
diff --git a/lisp/cedet/semantic/symref/global.el b/lisp/cedet/semantic/symref/global.el
index e4c114e..a33427e 100644
--- a/lisp/cedet/semantic/symref/global.el
+++ b/lisp/cedet/semantic/symref/global.el
@@ -49,6 +49,9 @@ semantic-symref-tool-global
     (semantic-symref-parse-tool-output tool b)
     ))
 
+(defconst semantic-symref-global--line-re
+  "^\\([^ ]+\\) +\\([0-9]+\\) \\([^ ]+\\) ")
+
 (cl-defmethod semantic-symref-parse-tool-output-one-line ((tool semantic-symref-tool-global))
   "Parse one line of grep output, and return it as a match list.
 Moves cursor to end of the match."
@@ -57,8 +60,13 @@ semantic-symref-tool-global
 	 ;; Search for files
 	 (when (re-search-forward "^\\([^\n]+\\)$" nil t)
 	   (match-string 1)))
+        ((eq (oref tool :resulttype) 'line-and-text)
+         (when (re-search-forward semantic-symref-global--line-re nil t)
+           (list (string-to-number (match-string 2))
+                 (match-string 3)
+                 (buffer-substring-no-properties (point) (line-end-position)))))
 	(t
-	 (when (re-search-forward "^\\([^ ]+\\) +\\([0-9]+\\) \\([^ ]+\\) " nil t)
+	 (when (re-search-forward semantic-symref-global--line-re nil t)
 	   (cons (string-to-number (match-string 2))
 		 (match-string 3))
 	   ))))
diff --git a/lisp/cedet/semantic/symref/grep.el b/lisp/cedet/semantic/symref/grep.el
index 5d1fea8..868e6c3 100644
--- a/lisp/cedet/semantic/symref/grep.el
+++ b/lisp/cedet/semantic/symref/grep.el
@@ -188,6 +188,9 @@ semantic-symref-grep-shell
     ;; Return the answer
     ans))
 
+(defconst semantic-symref-grep--line-re
+  "^\\(\\(?:[a-zA-Z]:\\)?[^:\n]+\\):\\([0-9]+\\):")
+
 (cl-defmethod semantic-symref-parse-tool-output-one-line ((tool semantic-symref-tool-grep))
   "Parse one line of grep output, and return it as a match list.
 Moves cursor to end of the match."
@@ -195,8 +198,13 @@ semantic-symref-grep-shell
 	 ;; Search for files
 	 (when (re-search-forward "^\\([^\n]+\\)$" nil t)
 	   (match-string 1)))
+        ((eq (oref tool :resulttype) 'line-and-text)
+         (when (re-search-forward semantic-symref-grep--line-re nil t)
+           (list (string-to-number (match-string 2))
+                 (match-string 1)
+                 (buffer-substring-no-properties (point) (line-end-position)))))
 	(t
-	 (when (re-search-forward  "^\\(\\(?:[a-zA-Z]:\\)?[^:\n]+\\):\\([0-9]+\\):" nil t)
+	 (when (re-search-forward semantic-symref-grep--line-re nil t)
 	   (cons (string-to-number (match-string 2))
 		 (match-string 1))
 	   ))))
diff --git a/lisp/cedet/semantic/symref/idutils.el b/lisp/cedet/semantic/symref/idutils.el
index 4127d7a..db3e9a0 100644
--- a/lisp/cedet/semantic/symref/idutils.el
+++ b/lisp/cedet/semantic/symref/idutils.el
@@ -49,6 +49,9 @@ semantic-symref-tool-idutils
     (semantic-symref-parse-tool-output tool b)
     ))
 
+(defconst semantic-symref-idutils--line-re
+  "^\\(\\(?:[a-zA-Z]:\\)?[^:\n]+\\):\\([0-9]+\\):")
+
 (cl-defmethod semantic-symref-parse-tool-output-one-line ((tool semantic-symref-tool-idutils))
   "Parse one line of grep output, and return it as a match list.
 Moves cursor to end of the match."
@@ -59,8 +62,13 @@ semantic-symref-tool-idutils
 	((eq (oref tool :searchtype) 'tagcompletions)
 	 (when (re-search-forward "^\\([^ ]+\\) " nil t)
 	   (match-string 1)))
-	(t
-	 (when (re-search-forward "^\\(\\(?:[a-zA-Z]:\\)?[^:\n]+\\):\\([0-9]+\\):" nil t)
+        ((eq (oref tool :resulttype) 'line-and-text)
+         (when (re-search-forward semantic-symref-idutils--line-re nil t)
+	   (list (string-to-number (match-string 2))
+                 (expand-file-name (match-string 1) default-directory)
+                 (buffer-substring-no-properties (point) (line-end-position)))))
+	(t ; resulttype is line
+	 (when (re-search-forward semantic-symref-idutils--line-re nil t)
 	   (cons (string-to-number (match-string 2))
 		 (expand-file-name (match-string 1) default-directory))
 	   ))))
diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
index feed0fb..7354f8b 100644
--- a/lisp/progmodes/xref.el
+++ b/lisp/progmodes/xref.el
@@ -859,19 +859,25 @@ xref-collect-references
   ;; to force the backend to use `default-directory'.
   (let* ((ede-minor-mode nil)
          (default-directory dir)
+         ;; FIXME: Remove CScope and Global from the recognized tools?
+         ;; The current implementations interpret the symbol search as
+         ;; "find all calls to the given function", but not function
+         ;; definition. And they return nothing when passed a variable
+         ;; name, even a global one.
          (semantic-symref-tool 'detect)
          (case-fold-search nil)
-         (res (semantic-symref-find-references-by-name symbol 'subdirs))
-         (hits (and res (oref res hit-lines)))
-         (orig-buffers (buffer-list)))
+         (inst (semantic-symref-instantiate :searchfor symbol
+                                            :searchtype 'symbol
+                                            :searchscope 'subdirs
+                                            :resulttype 'line-and-text))
+         (hits (semantic-symref-perform-search inst))
+         (tmp-buffer (generate-new-buffer " *xref-temp*")))
     (unwind-protect
         (cl-mapcan (lambda (hit) (xref--collect-matches
-                             hit (format "\\_<%s\\_>" (regexp-quote symbol))))
+                             hit (format "\\_<%s\\_>" (regexp-quote symbol))
+                             tmp-buffer))
                    hits)
-      ;; TODO: Implement "lightweight" buffer visiting, so that we
-      ;; don't have to kill them.
-      (mapc #'kill-buffer
-            (cl-set-difference (buffer-list) orig-buffers)))))
+      (kill-buffer tmp-buffer))))
 
 ;;;###autoload
 (defun xref-collect-matches (regexp files dir ignores)
@@ -890,34 +896,24 @@ xref-collect-matches
                                        files
                                        (expand-file-name dir)
                                        ignores))
-         (orig-buffers (buffer-list))
          (buf (get-buffer-create " *xref-grep*"))
          (grep-re (caar grep-regexp-alist))
-         (counter 0)
-         reporter
-         hits)
+         hits
+         (tmp-buffer (generate-new-buffer " *xref-temp*")))
     (with-current-buffer buf
       (erase-buffer)
       (call-process-shell-command command nil t)
       (goto-char (point-min))
       (while (re-search-forward grep-re nil t)
-        (push (cons (string-to-number (match-string 2))
-                    (match-string 1))
+        (push (list (string-to-number (match-string 2))
+                    (match-string 1)
+                    (buffer-substring-no-properties (point) (line-end-position)))
               hits)))
-    (setq reporter (make-progress-reporter
-                    (format "Collecting search results...")
-                    0 (length hits)))
     (unwind-protect
         (cl-mapcan (lambda (hit)
-                     (prog1
-                         (progress-reporter-update reporter counter)
-                       (cl-incf counter))
-                     (xref--collect-matches hit regexp))
+                     (xref--collect-matches hit regexp tmp-buffer))
                    (nreverse hits))
-      (progress-reporter-done reporter)
-      ;; TODO: Same as above.
-      (mapc #'kill-buffer
-            (cl-set-difference (buffer-list) orig-buffers)))))
+      (kill-buffer tmp-buffer))))
 
 (defun xref--rgrep-command (regexp files dir ignores)
   (require 'find-dired)      ; for `find-name-arg'
@@ -980,30 +976,49 @@ xref--regexp-to-extended
                (match-string 1 str)))))
    str t t))
 
-(defun xref--collect-matches (hit regexp)
-  (pcase-let* ((`(,line . ,file) hit)
-               (buf (or (find-buffer-visiting file)
-                        (semantic-find-file-noselect file))))
-    (with-current-buffer buf
-      (save-excursion
+(defvar xref--temp-buffer-file-name nil)
+
+(defun xref--collect-matches (hit regexp tmp-buffer)
+  (pcase-let* ((`(,line ,file ,text) hit)
+               (buf (find-buffer-visiting file)))
+    (if buf
+        (with-current-buffer buf
+          (save-excursion
+            (goto-char (point-min))
+            (forward-line (1- line))
+            (xref--collect-matches-1 regexp file line
+                                     (line-beginning-position)
+                                     (line-end-position))))
+      (with-current-buffer tmp-buffer
+        (erase-buffer)
+        (unless (equal file xref--temp-buffer-file-name)
+          (insert-file-contents file nil 0 200)
+          (setq-local delay-mode-hooks t)
+          (set-auto-mode t)
+          (setq-local xref--temp-buffer-file-name file)
+          (setq-local inhibit-read-only t)
+          (erase-buffer))
+        (insert text)
         (goto-char (point-min))
-        (forward-line (1- line))
-        (let ((line-end (line-end-position))
-              (line-beg (line-beginning-position))
-              matches)
-          (syntax-propertize line-end)
-          ;; FIXME: This results in several lines with the same
-          ;; summary. Solve with composite pattern?
-          (while (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 'highlight
-                                      t summary)
-              (push (xref-make-match summary loc (- end-column beg-column))
-                    matches)))
-          (nreverse matches))))))
+        (xref--collect-matches-1 regexp file line
+                                 (point)
+                                 (point-max))))))
+
+(defun xref--collect-matches-1 (regexp file line line-beg line-end)
+  (let (matches)
+    (syntax-propertize line-end)
+    ;; FIXME: This results in several lines with the same
+    ;; summary. Solve with composite pattern?
+    (while (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 'highlight
+                                t summary)
+        (push (xref-make-match summary loc (- end-column beg-column))
+              matches)))
+    (nreverse matches)))
 
 (provide 'xref)