unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Dmitry Gutov <dgutov@yandex.ru>
To: Stefan Monnier <monnier@iro.umontreal.ca>
Cc: "Mattias Engdegård" <mattiase@acm.org>,
	50777@debbugs.gnu.org, "Daniel Martín" <mardani29@yahoo.es>
Subject: bug#50777: Dropping EIEIO from xref (for performance)
Date: Fri, 24 Sep 2021 18:32:39 +0300	[thread overview]
Message-ID: <c0af84cd-ca85-6a9c-5c82-7d6409e241f1@yandex.ru> (raw)
In-Reply-To: <jwv5yuq3uq9.fsf-monnier+emacs@gnu.org>

On 24.09.2021 17:49, Stefan Monnier wrote:
>> 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),
> 
> About that: it should be fairly easy to get `make-instance`, `oref`, and
> `with-slots` to work with cl-defstruct objects/classes.
> 
> This should be easy enough that we *could* potentially consider doing
> those changes "right away" (i.e. for Emacs-28).

Interesting point.

Not sure we really want to: I imagine the removal of the use of 
'make-instance' contributed to the overall performance improvement. If 
I'm right, discouraging of its use (for this particular purpose) in 
third-party code seems like a good idea.

> As for the inheritance, they can still inherit from `xref-location` but:
> - not with `defclass` (the subclass needs to be defined with `cl-defstruct`
>    instead).
> - they can't inherit from both `xref-location` and something else any more.

There is no 'xref-location' structure, it's only defined by generic 
functions. I think that's better.

'xref-location' has always been a class with empty definition.

One area of bigger concern is whether code compiled against the new 
version of Xref would work with the old one without recompiling (and 
vice versa). I guess for that to work we need to disable inlining on 
xref-item's accessors, at least. Maybe there's something else I'm missing.





  reply	other threads:[~2021-09-24 15:32 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-24 13:28 bug#50777: Dropping EIEIO from xref (for performance) Dmitry Gutov
2021-09-24 13:35 ` Dmitry Gutov
2021-09-24 13:37 ` Dmitry Gutov
2021-09-24 14:49   ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-09-24 15:32     ` Dmitry Gutov [this message]
2021-09-26  1:15       ` Dmitry Gutov
2021-09-26  8:51         ` Mattias Engdegård
2021-09-26 18:34           ` Dmitry Gutov
2021-09-24 15:12   ` Arthur Miller
2021-09-25  1:42   ` Lars Ingebrigtsen
2021-09-25  1:52     ` Dmitry Gutov
2021-09-25 14:07   ` Daniel Martín via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-09-26  2:16     ` Dmitry Gutov
2021-09-30 21:10 ` Dmitry Gutov

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

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c0af84cd-ca85-6a9c-5c82-7d6409e241f1@yandex.ru \
    --to=dgutov@yandex.ru \
    --cc=50777@debbugs.gnu.org \
    --cc=mardani29@yahoo.es \
    --cc=mattiase@acm.org \
    --cc=monnier@iro.umontreal.ca \
    /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 public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).