From: Dmitry Gutov <dgutov@yandex.ru>
To: Eli Zaretskii <eliz@gnu.org>
Cc: 23223@debbugs.gnu.org
Subject: bug#23223: 25.0.92; Can xref-find-references be sped up?
Date: Tue, 12 Apr 2016 02:25:54 +0300 [thread overview]
Message-ID: <90b81ad1-c3ab-272b-724b-ba63f0451d5a@yandex.ru> (raw)
In-Reply-To: <83zit0f8ah.fsf@gnu.org>
[-- Attachment #1: Type: text/plain, Size: 2537 bytes --]
On 04/11/2016 06:56 PM, Eli Zaretskii wrote:
> Unfortunately, it seems to miss matches: out of 1127 matches of
> current_buffer with the original version, the new one only shows 963.
> It sounds like some conditions on what exactly is a symbol need
> adjustment,
Ooh, that's a great catch, thanks! Turns out, auto-mode-alist wasn't
getting applied because buffer-file-name wasn't set. Fixing that added
~25% performance hit, so I also added xref--find-buffer-visiting to
cache the expensive lookup mentioned previously. See the new patch attached.
That seems to about exhaust the optimization opportunity here.
> As an aside, if I invoke xref-find-references without an ID file,
> which AFAIU means Emacs will invoke find+grep, I get this error:
>
> semantic-symref-derive-find-filepatterns: Customize ‘semantic-symref-filepattern-alist’ for lisp-interaction-mode
>
> unless I invoke xref-find-references from a buffer in C mode.
...or the current major mode is one of the currently supported ones, via
the above variable.
> Curiously, this doesn't happen when there's an ID file and IDutils is
> invoked. Is this expected?
Yes. semantic-symref-filepattern-alist is defined in and used by
semantic/symref/grep.el. We can add (lisp-interaction-mode "*.el") to it.
> "gid" is just a short for "lid -R grep", so the
> contents I get is the same as xref-find-references does, it's just
> formatted differently.
Not exactly: you get more false positives because it doesn't apply the
language-aware filtering.
>> What's the new time you get from the former?
>
> 3 sec (in an unoptimized build, I'd expect this to become 1 sec in an
> optimized build). So we are OK speed-wise, we just need to fix the
> misses mentioned above.
Cool.
>> 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".
>
> I don't see this in xref-find-references. Should I?
Not at all. xref-find-references only searches in the files related to
the current one by type (or in a predefined database like id-utils's one).
The chatter I got was from e.g. sh-mode (about the indentation variable
being guessed), nxml-mode, and so on, with matches in ~4000 files.
>> We can avoid saving it to the message log, but it appears in the
>> echo area either way.
>
> Can't you bind inhibit-message to a non-nil value?
That works!
> Thanks again for working on this.
Thanks for the thoughtful bug report.
[-- Attachment #2: xref-with-temp-buffer.diff --]
[-- Type: text/x-patch, Size: 13517 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..68021c5 100644
--- a/lisp/progmodes/xref.el
+++ b/lisp/progmodes/xref.el
@@ -839,16 +839,18 @@ xref-etags-mode
(kill-local-variable 'xref-backend-functions))
(setq-local xref-backend-functions xref-etags-mode--saved)))
-(declare-function semantic-symref-find-references-by-name "semantic/symref")
-(declare-function semantic-find-file-noselect "semantic/fw")
+(declare-function semantic-symref-instantiate "semantic/symref")
+(declare-function semantic-symref-perform-search "semantic/symref")
(declare-function grep-expand-template "grep")
(defvar ede-minor-mode) ;; ede.el
+(defvar xref--last-visiting-buffer nil)
+
(defun xref-collect-references (symbol dir)
"Collect references to SYMBOL inside DIR.
This function uses the Semantic Symbol Reference API, see
-`semantic-symref-find-references-by-name' for details on which
-tools are used, and when."
+`semantic-symref-tool-alist' for details on which tools are used,
+and when."
(cl-assert (directory-name-p dir))
(require 'semantic/symref)
(defvar semantic-symref-tool)
@@ -859,19 +861,26 @@ 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))
+ xref--last-visiting-buffer
+ (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 +899,25 @@ 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
+ xref--last-visiting-buffer
+ (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 +980,60 @@ 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 (xref--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)
+ ;; Can't (setq-local delay-mode-hooks t) because of
+ ;; bug#23272, but the performance penalty seems minimal.
+ (let ((buffer-file-name file)
+ (inhibit-message t)
+ message-log-max)
+ (ignore-errors
+ (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)))
+
+(defun xref--find-buffer-visiting (file)
+ (unless (equal (car xref--last-visiting-buffer) file)
+ (setq xref--last-visiting-buffer
+ (cons file (find-buffer-visiting file))))
+ (cdr xref--last-visiting-buffer))
(provide 'xref)
next prev parent reply other threads:[~2016-04-11 23:25 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-04-05 15:16 bug#23223: 25.0.92; Can xref-find-references be sped up? Eli Zaretskii
2016-04-06 0:37 ` Dmitry Gutov
2016-04-06 15:09 ` Dmitry Gutov
2016-04-06 17:20 ` Eli Zaretskii
2016-04-06 23:11 ` Dmitry Gutov
2016-04-07 2:49 ` Eli Zaretskii
2016-04-06 17:12 ` Eli Zaretskii
2016-04-07 0:11 ` Dmitry Gutov
2016-04-07 15:03 ` Eli Zaretskii
2016-04-09 3:12 ` Dmitry Gutov
2016-04-09 7:25 ` Eli Zaretskii
2016-04-10 23:27 ` Dmitry Gutov
2016-04-11 15:56 ` Eli Zaretskii
2016-04-11 23:25 ` Dmitry Gutov [this message]
2016-04-12 15:50 ` Eli Zaretskii
2016-04-12 18:49 ` Dmitry Gutov
2016-04-12 19:16 ` Eli Zaretskii
2016-04-12 20:26 ` Dmitry Gutov
2016-04-13 2:44 ` Eli Zaretskii
2016-04-13 10:18 ` Dmitry Gutov
2016-04-13 15:16 ` Eli Zaretskii
2016-04-15 14:29 ` Eli Zaretskii
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=90b81ad1-c3ab-272b-724b-ba63f0451d5a@yandex.ru \
--to=dgutov@yandex.ru \
--cc=23223@debbugs.gnu.org \
--cc=eliz@gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.