From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Dmitry Gutov Newsgroups: gmane.emacs.bugs Subject: bug#50777: Dropping EIEIO from xref (for performance) Date: Fri, 24 Sep 2021 16:28:38 +0300 Message-ID: <6b5b14d5-b2ca-8add-f4bf-a3405270c07a@yandex.ru> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------2D39B70D301BE44FBA52954B" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="13549"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.13.0 Cc: lars To: 50777@debbugs.gnu.org Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Fri Sep 24 15:30:33 2021 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1mTlHa-0003Im-MF for geb-bug-gnu-emacs@m.gmane-mx.org; Fri, 24 Sep 2021 15:30:30 +0200 Original-Received: from localhost ([::1]:59438 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mTlHY-0003dY-Pg for geb-bug-gnu-emacs@m.gmane-mx.org; Fri, 24 Sep 2021 09:30:28 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:58468) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mTlH9-0003dA-4q for bug-gnu-emacs@gnu.org; Fri, 24 Sep 2021 09:30:03 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:45410) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1mTlH8-0003XE-TN; Fri, 24 Sep 2021 09:30:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1mTlH8-0004wa-Pw; Fri, 24 Sep 2021 09:30:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Dmitry Gutov Original-Sender: "Debbugs-submit" Resent-CC: eliz@gnu.org, lars@debbugs.gnu.org, ingebrigtsen@debbugs.gnu.org, bug-gnu-emacs@gnu.org Resent-Date: Fri, 24 Sep 2021 13:30:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: report 50777 X-GNU-PR-Package: emacs X-Debbugs-Original-To: bug-gnu-emacs@gnu.org X-Debbugs-Original-Xcc: eli zaretskii , lars ingebrigtsen Original-Received: via spool by submit@debbugs.gnu.org id=B.163249014318870 (code B ref -1); Fri, 24 Sep 2021 13:30:02 +0000 Original-Received: (at submit) by debbugs.gnu.org; 24 Sep 2021 13:29:03 +0000 Original-Received: from localhost ([127.0.0.1]:56956 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mTlGA-0004uF-9x for submit@debbugs.gnu.org; Fri, 24 Sep 2021 09:29:03 -0400 Original-Received: from lists.gnu.org ([209.51.188.17]:34540) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mTlFz-0004tk-J3 for submit@debbugs.gnu.org; Fri, 24 Sep 2021 09:29:00 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:58382) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mTlFz-0003Vo-Bc for bug-gnu-emacs@gnu.org; Fri, 24 Sep 2021 09:28:51 -0400 Original-Received: from mail-wr1-x430.google.com ([2a00:1450:4864:20::430]:33343) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1mTlFw-0002gV-QG for bug-gnu-emacs@gnu.org; Fri, 24 Sep 2021 09:28:51 -0400 Original-Received: by mail-wr1-x430.google.com with SMTP id t18so27626549wrb.0 for ; Fri, 24 Sep 2021 06:28:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=sender:to:from:subject:message-id:date:user-agent:mime-version :content-language; bh=zoh37vkrJ8pG1pYof/jEIo640Ck0M4EzUOWrzN05gcY=; b=kpdZqxoSoCQczM2YzragcY5I6AVq/lX7qc9dywKefv/cTV1CZcpyRdGRJt2ug59lp3 fF+YaRbBpyUOJWpL+BjX1QYoQvH5YSxWQs8hBwADMjRjFPeiaCrD8naH4XFvghR62Ev8 fgJJ4NjdfNI/cDrcu4nqQXQB6Nod7PWmchici9KJkaFqJqC9qbxsM6B1NJLyZlMB0AXH M0ZlX8L6ygAieJz5HpnxnruQkm5aDT6/aKkkt/J3cgsbmWXkl6qWj54lTnvjXiYrH1V8 UXdq7msiDGnpr/kAUfXOfxpZtbt9o6jwazMEoAGeFxBGtnuq4q9xcaP33O/IQBeRUTJ7 weww== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:sender:to:from:subject:message-id:date :user-agent:mime-version:content-language; bh=zoh37vkrJ8pG1pYof/jEIo640Ck0M4EzUOWrzN05gcY=; b=KLDsg5yvpooEvXJdljR0DLoqcxdqZHl3SbA3sbT9OnMWGKJjVt8pYYDj/hkk5BWjrF Yhb5RkiJnD0tni6c6qYtKmDglocGPBRS5FG5jWov93QLDNV9FJ9w6RSngkdhvaT6ezkb 2y7mm5wMl/JAaqRU5aP+EGWPxYP00JopGi+GG0QP58G4ES5K0A0PpjNAO42OwRqPshEW A+p/uerR+9Fq86kLChjk1Jehwshj3xraisw3+rd2lrdoiJH8MYnaxqPeUJpJQZHs31OU AGKeZIZFbmpugra8yLmZDrTMlCkN5xFfs7qxBNP2uNJBMCAbQK0XfjbOXJoRU34oPvgI N48w== X-Gm-Message-State: AOAM531nRRVc5YvklFvy4S7optj2W1GPsf4in0RimSYvGD7WUOkVAmGz SHdGoJdvJ5dfhdl2PldAzMwFkT9TdmM= X-Google-Smtp-Source: ABdhPJymacV8YA7+4VIqZi6+JpsWw45qLGc+W8VvHuG0RUnXbuCLjxN4gmkZI469aH//Mi/Wq8vWqw== X-Received: by 2002:a05:600c:4106:: with SMTP id j6mr2204454wmi.99.1632490126838; Fri, 24 Sep 2021 06:28:46 -0700 (PDT) Original-Received: from [192.168.0.6] ([46.251.119.176]) by smtp.googlemail.com with ESMTPSA id 20sm13533562wme.46.2021.09.24.06.28.45 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 24 Sep 2021 06:28:46 -0700 (PDT) Content-Language: en-US Received-SPF: pass client-ip=2a00:1450:4864:20::430; envelope-from=raaahh@gmail.com; helo=mail-wr1-x430.google.com X-Spam_score_int: -14 X-Spam_score: -1.5 X-Spam_bar: - X-Spam_report: (-1.5 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_FORGED_FROMDOMAIN=0.249, FREEMAIL_FROM=0.001, HEADER_FROM_DIFFERENT_DOMAINS=0.248, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=no autolearn_force=no X-Spam_action: no action X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list 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-mx.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.io gmane.emacs.bugs:215294 Archived-At: This is a multi-part message in MIME format. --------------2D39B70D301BE44FBA52954B Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit X-Debbugs-CC: Eli Zaretskii , Lars Ingebrigtsen , Stefan Monnier , Daniel Martín , Mattias Engdegård With the recent discussion about our Lisp overhead when the search returns many matches, I went a couple more rounds to try to identify the hotspots. My main benchmark for that is (benchmark 1 '(project-find-regexp "list")) in the Emacs repo. The 3 recent commits have reduced that time from 8s to 6.5s on my machine (under best conditions: warm disk cache and fresh Emacs session). The machine is pretty fast, so I figure we can multiply that timing by 2x, to imagine the average user experience. Having exhausted the obvious steps, I have looked at the difference between defclass and cl-defstruct in this respect. Earlier quick tests indicated there would be little to no difference, but now that I've wrote the full patch, it's significant. The attached patch drops the timing of the above benchmark further down to 4.1s here. Both allocation ('make-instance' vs auto-generated 'xref-make-*' struct constructors) and accessing the fields show increased performance, with the former showing the most improvement: I guess EIEIO's type checking added some extra overhead. That creates some incompatibility (third-party packages can't inherit from 'xref-location' anymore, or use 'make-instance', 'oref' or 'with-slots' with our values), but OTOH a quick survey of existing packages that integrate with Xref shows they wisely don't do that already. Not the largest, popular ones I have reviewed, at least. The ones that do can be updated to rely on the recommended public interface: the xref-make-* constructors and the generic functions to access the data. This will keep the code compatible with both new and previous versions of Xref. I'd like to push it soon, so users of Emacs 28 can enjoy the speedup. What do people think? --------------2D39B70D301BE44FBA52954B Content-Type: text/x-patch; charset=UTF-8; name="xref-from-defclass-to-defstruct.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="xref-from-defclass-to-defstruct.diff" diff --git a/lisp/progmodes/etags.el b/lisp/progmodes/etags.el index e6af2b12c7..f53b09d9e8 100644 --- a/lisp/progmodes/etags.el +++ b/lisp/progmodes/etags.el @@ -2161,18 +2161,16 @@ etags--xref-apropos-additional (nreverse res)))) tags-apropos-additional-actions)) -(defclass xref-etags-location (xref-location) - ((tag-info :type list :initarg :tag-info) - (file :type string :initarg :file - :reader xref-location-group)) - :documentation "Location of an etags tag.") +(cl-defstruct (xref-etags-location + (:constructor xref-make-etags-location (tag-info file))) + "Location of an etags tag." + tag-info file) -(defun xref-make-etags-location (tag-info file) - (make-instance 'xref-etags-location :tag-info tag-info - :file (expand-file-name file))) +(cl-defmethod xref-location-group ((l xref-etags-location)) + (xref-etags-location-file l)) (cl-defmethod xref-location-marker ((l xref-etags-location)) - (with-slots (tag-info file) l + (pcase-let (((cl-struct xref-etags-location tag-info file) l)) (let ((buffer (find-file-noselect file))) (with-current-buffer buffer (save-excursion @@ -2182,25 +2180,20 @@ xref-location-marker (point-marker))))))) (cl-defmethod xref-location-line ((l xref-etags-location)) - (with-slots (tag-info) l + (pcase-let (((cl-struct xref-etags-location tag-info) l)) (nth 1 tag-info))) -(defclass xref-etags-apropos-location (xref-location) - ((symbol :type symbol :initarg :symbol) - (goto-fun :type function :initarg :goto-fun) - (group :type string :initarg :group - :reader xref-location-group)) - :documentation "Location of an additional apropos etags symbol.") +(cl-defstruct (xref-etags-apropos-location + (:constructor xref-make-etags-apropos-location (symbol goto-fun group))) + "Location of an additional apropos etags symbol." + symbol goto-fun group) -(defun xref-make-etags-apropos-location (symbol goto-fun group) - (make-instance 'xref-etags-apropos-location - :symbol symbol - :goto-fun goto-fun - :group group)) +(cl-defmethod xref-location-group ((l xref-etags-apropos-location)) + (xref-etags-apropos-location-group l)) (cl-defmethod xref-location-marker ((l xref-etags-apropos-location)) (save-window-excursion - (with-slots (goto-fun symbol) l + (pcase-let (((cl-struct xref-etags-apropos-location goto-fun symbol) l)) (funcall goto-fun symbol) (point-marker)))) diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el index fb8090cfb7..fa053bc987 100644 --- a/lisp/progmodes/xref.el +++ b/lisp/progmodes/xref.el @@ -78,9 +78,6 @@ xref ;;; Locations -(defclass xref-location () () - :documentation "A location represents a position in a file or buffer.") - (cl-defgeneric xref-location-marker (location) "Return the marker for LOCATION.") @@ -121,19 +118,20 @@ xref-file-name-display ;; FIXME: might be useful to have an optional "hint" i.e. a string to ;; search for in case the line number is slightly out of date. -(defclass xref-file-location (xref-location) - ((file :type string :initarg :file :reader xref-location-group) - (line :type fixnum :initarg :line :reader xref-location-line) - (column :type fixnum :initarg :column :reader xref-file-location-column)) - :documentation "A file location is a file/line/column triple. -Line numbers start from 1 and columns from 0.") +(cl-defstruct (xref-file-location + (:constructor xref-make-file-location (file line column))) + "A file location is a file/line/column triple. +Line numbers start from 1 and columns from 0." + file line column) + +(cl-defmethod xref-location-group ((l xref-file-location)) + (xref-file-location-file l)) -(defun xref-make-file-location (file line column) - "Create and return a new `xref-file-location'." - (make-instance 'xref-file-location :file file :line line :column column)) +(cl-defmethod xref-location-line ((l xref-file-location)) + (xref-file-location-line l)) (cl-defmethod xref-location-marker ((l xref-file-location)) - (with-slots (file line column) l + (pcase-let (((cl-struct xref-file-location file line column) l)) (with-current-buffer (or (get-file-buffer file) (let ((find-file-suppress-same-file-warnings t)) @@ -151,77 +149,45 @@ xref-location-marker (forward-char column)) (point-marker)))))) -(defclass xref-buffer-location (xref-location) - ((buffer :type buffer :initarg :buffer) - (position :type fixnum :initarg :position))) - -(defun xref-make-buffer-location (buffer position) - "Create and return a new `xref-buffer-location'." - (make-instance 'xref-buffer-location :buffer buffer :position position)) +(cl-defstruct (xref-buffer-location + (:constructor xref-make-buffer-location (buffer position))) + buffer position) (cl-defmethod xref-location-marker ((l xref-buffer-location)) - (with-slots (buffer position) l + (pcase-let (((cl-struct xref-buffer-location buffer position) l)) (let ((m (make-marker))) (move-marker m position buffer)))) (cl-defmethod xref-location-group ((l xref-buffer-location)) - (with-slots (buffer) l + (pcase-let (((cl-struct xref-buffer-location buffer) l)) (or (buffer-file-name buffer) (format "(buffer %s)" (buffer-name buffer))))) -(defclass xref-bogus-location (xref-location) - ((message :type string :initarg :message - :reader xref-bogus-location-message)) - :documentation "Bogus locations are sometimes useful to -indicate errors, e.g. when we know that a function exists but the -actual location is not known.") - -(defun xref-make-bogus-location (message) - "Create and return a new `xref-bogus-location'." - (make-instance 'xref-bogus-location :message message)) +(cl-defstruct (xref-bogus-location + (:constructor xref-make-bogus-location (message))) + "Bogus locations are sometimes useful to indicate errors, +e.g. when we know that a function exists but the actual location +is not known." + message) (cl-defmethod xref-location-marker ((l xref-bogus-location)) - (user-error "%s" (oref l message))) + (user-error "%s" (xref-bogus-location-message l))) (cl-defmethod xref-location-group ((_ xref-bogus-location)) "(No location)") ;;; Cross-reference -(defclass xref-item () - ((summary :type string :initarg :summary - :reader xref-item-summary - :documentation "One line which will be displayed for -this item in the output buffer.") - (location :initarg :location - :reader xref-item-location - :documentation "An object describing how to navigate -to the reference's target.")) - :comment "An xref item describes a reference to a location -somewhere.") - -(defun xref-make (summary location) - "Create and return a new `xref-item'. -SUMMARY is a short string to describe the xref. -LOCATION is an `xref-location'." - (make-instance 'xref-item :summary summary :location location)) - -(defclass xref-match-item () - ((summary :type string :initarg :summary - :reader xref-item-summary) - (location :initarg :location - :type xref-location - :reader xref-item-location) - (length :initarg :length :reader xref-match-length)) - :comment "A match xref item describes a search result.") - -(defun xref-make-match (summary location length) - "Create and return a new `xref-match-item'. -SUMMARY is a short string to describe the xref. -LOCATION is an `xref-location'. -LENGTH is the match length, in characters." - (make-instance 'xref-match-item :summary summary - :location location :length length)) +(cl-defstruct (xref-item + (:constructor xref-make (summary location))) + "An xref item describes a reference to a location somewhere." + summary location) + +(cl-defstruct (xref-match-item + (:include xref-item) + (:constructor xref-make-match (summary location length))) + "A match xref item describes a search result." + length) ;;; API @@ -970,7 +936,7 @@ xref--insert-xrefs for max-line-width = (cl-loop for xref in xrefs maximize (let ((line (xref-location-line - (oref xref location)))) + (xref-item-location xref)))) (and line (1+ (floor (log line 10)))))) for line-format = (and max-line-width (format "%%%dd: " max-line-width)) @@ -985,7 +951,7 @@ xref--insert-xrefs (xref--insert-propertized '(face xref-file-header xref-group t) group "\n") (cl-loop for xref in xrefs do - (with-slots (summary location) xref + (pcase-let (((cl-struct xref-item summary location) xref)) (let* ((line (xref-location-line location)) (prefix (cond @@ -1206,22 +1172,23 @@ xref-show-definitions-completing-read (cl-loop for ((group . xrefs) . more1) on xref-alist do (cl-loop for (xref . more2) on xrefs do - (with-slots (summary location) xref - (let* ((line (xref-location-line location)) - (line-fmt - (if line - (format #("%d:" 0 2 (face xref-line-number)) - line) - "")) - (group-prefix - (substring group group-prefix-length)) - (group-fmt - (propertize group-prefix - 'face 'xref-file-header - 'xref--group group-prefix)) - (candidate - (format "%s:%s%s" group-fmt line-fmt summary))) - (push (cons candidate xref) xref-alist-with-line-info))))) + (let* ((summary (xref-item-summary xref)) + (location (xref-item-location xref)) + (line (xref-location-line location)) + (line-fmt + (if line + (format #("%d:" 0 2 (face xref-line-number)) + line) + "")) + (group-prefix + (substring group group-prefix-length)) + (group-fmt + (propertize group-prefix + 'face 'xref-file-header + 'xref--group group-prefix)) + (candidate + (format "%s:%s%s" group-fmt line-fmt summary))) + (push (cons candidate xref) xref-alist-with-line-info)))) (setq xref (if (not (cdr xrefs)) (car xrefs) --------------2D39B70D301BE44FBA52954B--