GNU bug report logs - #50777
Dropping EIEIO from xref (for performance)

Previous Next

Package: emacs;

Reported by: Dmitry Gutov <dgutov <at> yandex.ru>

Date: Fri, 24 Sep 2021 13:30:02 UTC

Severity: normal

Done: Dmitry Gutov <dgutov <at> yandex.ru>

Bug is archived. No further changes may be made.

Full log


View this message in rfc822 format

From: help-debbugs <at> gnu.org (GNU bug Tracking System)
To: Dmitry Gutov <dgutov <at> yandex.ru>
Subject: bug#50777: closed (Re: bug#50777: Dropping EIEIO from xref (for
 performance))
Date: Thu, 30 Sep 2021 21:11:02 +0000
[Message part 1 (text/plain, inline)]
Your bug report

#50777: Dropping EIEIO from xref (for performance)

which was filed against the emacs package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to 50777 <at> debbugs.gnu.org.

-- 
50777: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=50777
GNU Bug Tracking System
Contact help-debbugs <at> gnu.org with problems
[Message part 2 (message/rfc822, inline)]
From: Dmitry Gutov <dgutov <at> yandex.ru>
To: 50777-done <at> debbugs.gnu.org
Subject: Re: bug#50777: Dropping EIEIO from xref (for performance)
Date: Fri, 1 Oct 2021 00:10:07 +0300
In the absence of strong objections, pushed to Emacs 28.

Sorry about jumping into a departing train: I had a flight this morning 
and had very little time the night before.

In addition to other considerations, removing the unjustified 
restriction of ':type xref-location' on xref-match-item's location 
should also have a good effect on the ecosystem.

Thanks all.

[Message part 3 (message/rfc822, inline)]
From: Dmitry Gutov <dgutov <at> yandex.ru>
To: bug-gnu-emacs <at> gnu.org
Subject: Dropping EIEIO from xref (for performance)
Date: Fri, 24 Sep 2021 16:28:38 +0300
[Message part 4 (text/plain, inline)]
X-Debbugs-CC: Eli Zaretskii <eliz <at> gnu.org>, Lars Ingebrigtsen 
<larsi <at> gnus.org>, Stefan Monnier <monnier <at> iro.umontreal.ca>, Daniel 
Martín <mardani29 <at> yahoo.es>, Mattias Engdegård <mattiase <at> acm.org>

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?
[xref-from-defclass-to-defstruct.diff (text/x-patch, attachment)]

This bug report was last modified 3 years and 327 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.