From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Dmitry Gutov Newsgroups: gmane.emacs.bugs Subject: bug#23223: 25.0.92; Can xref-find-references be sped up? Date: Tue, 12 Apr 2016 02:25:54 +0300 Message-ID: <90b81ad1-c3ab-272b-724b-ba63f0451d5a@yandex.ru> References: <83pou4m6h7.fsf@gnu.org> <902ac022-3a76-c363-0c77-12b1cdb8d521@yandex.ru> <8360vumzk4.fsf@gnu.org> <83mvp5lauu.fsf@gnu.org> <4424e043-31c7-0da4-213a-ee8ac31d9265@yandex.ru> <83vb3ri6q0.fsf@gnu.org> <83zit0f8ah.fsf@gnu.org> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------D5E3E6A1CE927668B030A8E0" X-Trace: ger.gmane.org 1460417245 26061 80.91.229.3 (11 Apr 2016 23:27:25 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Mon, 11 Apr 2016 23:27:25 +0000 (UTC) Cc: 23223@debbugs.gnu.org To: Eli Zaretskii Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Tue Apr 12 01:27:14 2016 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1aplEe-0002nR-0R for geb-bug-gnu-emacs@m.gmane.org; Tue, 12 Apr 2016 01:27:12 +0200 Original-Received: from localhost ([::1]:36610 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aplEd-0003ak-B6 for geb-bug-gnu-emacs@m.gmane.org; Mon, 11 Apr 2016 19:27:11 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:59891) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aplEY-0003X9-5N for bug-gnu-emacs@gnu.org; Mon, 11 Apr 2016 19:27:08 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aplEU-0002Y6-Rj for bug-gnu-emacs@gnu.org; Mon, 11 Apr 2016 19:27:06 -0400 Original-Received: from debbugs.gnu.org ([208.118.235.43]:47500) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aplEU-0002Xu-Nc for bug-gnu-emacs@gnu.org; Mon, 11 Apr 2016 19:27:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1aplEU-0006xB-HN for bug-gnu-emacs@gnu.org; Mon, 11 Apr 2016 19:27:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Dmitry Gutov Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Mon, 11 Apr 2016 23:27:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 23223 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: Original-Received: via spool by 23223-submit@debbugs.gnu.org id=B23223.146041716526664 (code B ref 23223); Mon, 11 Apr 2016 23:27:02 +0000 Original-Received: (at 23223) by debbugs.gnu.org; 11 Apr 2016 23:26:05 +0000 Original-Received: from localhost ([127.0.0.1]:59837 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1aplDZ-0006vz-6f for submit@debbugs.gnu.org; Mon, 11 Apr 2016 19:26:05 -0400 Original-Received: from mail-wm0-f44.google.com ([74.125.82.44]:34555) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1aplDX-0006vV-2j for 23223@debbugs.gnu.org; Mon, 11 Apr 2016 19:26:03 -0400 Original-Received: by mail-wm0-f44.google.com with SMTP id l6so165696581wml.1 for <23223@debbugs.gnu.org>; Mon, 11 Apr 2016 16:26:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:subject:to:references:cc:from:message-id:date:user-agent :mime-version:in-reply-to; bh=zmxyY9CjyC6uDTV0c/CkqcCOs5kCzqZj+k86d/GJiV0=; b=q5semnjKveEsqU7R3m/1qyD9eO1B54gxrGn51sw90gmFTvjkTY/VoeZnI2wgufcCKl mQkwetBJfq1NsfZk3W8IQzqO93B9Y53YbXSlK7jQHzsekaBVK4nm8NPYktkhiXiBOtNF J1kF7PymvqvfoIKgFyOitWtKB3aLdDYJOBayy03PELvUwUBupoH2kxgbhabLpTCvVUzz MJuhrcVaPktPATVrTzmaBY2ahu84orKBB++exetn51JaF2JHRXFHKsxzA+PYAX7onfht FkHcq2sfPgk9JhMCTHIoFAdcU9CU6e55KGFC1hE8ZB/K8rJMkMzEYnqNPOND3sRHMkj8 rZ0A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:sender:subject:to:references:cc:from:message-id :date:user-agent:mime-version:in-reply-to; bh=zmxyY9CjyC6uDTV0c/CkqcCOs5kCzqZj+k86d/GJiV0=; b=OaeyG0Pp17F65R8MoJk35YeP5qpgYrZ2/VWT/Zgo0+FRFxdwMCQTr4/waVVEuXXBpa KMT4W+XcX9K7goxLbR1A02Ywah/6rXKZn6718J0DCFN4RjvoDuY2UOFn07kg3qzn9Zny WYwSZp7NJidTsRggTKXK3L/KsA11C+YTct7iM6FATldESGGrO29tGLD/x1Y8KUNqoo5G tWNjeOk5Smc3+58+gOkkWAYtJyyMS5LkXxBLW8WfTkMY2EuftF4Qm95F3D9o+0L+628R WgnhcDse6MDPjPxsrQEvnz2+mg7dL9ZisU4so4fm5VqnKDi0kyMLFMqBXuCqljwue2GX M3Xw== X-Gm-Message-State: AOPr4FWTOBQhxcIAJ+l7uDOvjEjCdFSx4pB8kgsNmpZl/WUrsixomwTG2FsNm8FGg9E4sA== X-Received: by 10.28.111.77 with SMTP id k74mr504651wmc.37.1460417157542; Mon, 11 Apr 2016 16:25:57 -0700 (PDT) Original-Received: from [192.168.1.2] ([185.105.175.24]) by smtp.googlemail.com with ESMTPSA id 202sm510541wmw.5.2016.04.11.16.25.55 (version=TLSv1/SSLv3 cipher=OTHER); Mon, 11 Apr 2016 16:25:56 -0700 (PDT) User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.0 In-Reply-To: <83zit0f8ah.fsf@gnu.org> X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 208.118.235.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.org gmane.emacs.bugs:116397 Archived-At: This is a multi-part message in MIME format. --------------D5E3E6A1CE927668B030A8E0 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 8bit 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. --------------D5E3E6A1CE927668B030A8E0 Content-Type: text/x-patch; name="xref-with-temp-buffer.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="xref-with-temp-buffer.diff" 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) --------------D5E3E6A1CE927668B030A8E0--