bug-gnu-emacs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

bug#54802: OClosure: Make `interactive-form` a generic function


From: Stefan Monnier
Subject: bug#54802: OClosure: Make `interactive-form` a generic function
Date: Thu, 14 Apr 2022 14:34:24 -0400
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/29.0.50 (gnu/linux)

> Calling `interactive-form' in a loop is also fairly common.

Do you have actual examples in some existing package?

> For example, I wrote some code a while back to list commands which
> operate on the region, which involves running this on each interned
> atom:
>
>   (defun region-command-p (command)
>     "Test if COMMAND, a symbol, is a command that accepts a region."
>     (and (commandp command)
>          (equal (cadr (interactive-form command)) "r")))

Interesting.  I just tested this in an `all-completions` loop and my
patch makes it run a factor of 2 slower (it went from ~30ms to ~60ms to
enumerate the 141 commands that matched).

There are some mitigating circumstances, tho:
- The first call to this code takes a lot of time (~6s) because
  it loads a crapload of packages (every package with a registered
  autoloaded command).
- It's very brittle since it won't find those commands that have
  interactive specs like "r\np" or where it's not a string (like
  `kill-region` and many others, actually there are regularly more, e.g.
  to make them obey `use-region-p`).
- That loop signaled an error because of an erroneous autoload in
  `gnus.el` (it's now fixed in `master`), so your code probably did
  not do that.
  [ Amusingly, I also tried the loop after removing the `commandp` call
    (which is arguably unnecessary and could even slow down the loop)
    but this bumped into even more errors because we then try to load
    even the non-interactive functions.  ]

> I'm sure a 3x slowdown would be noticeable, so why does this have to be
> a generic function?

Making a generic function is the "natural" choice in terms of the
semantics of the function (which is implemented as a sequence of tests
to dispatch to some implementation-specific alternative).

I could change `interactive-form` along the same lines as what I've done
with `function-documentation`, i.e. only delegate to a generic function
when it looks like an OClosure.  That would significantly reduce the
performance impact, probably to negligible levels, but semantically the
only difference between `interactive-form` and that new
`generic-interactive-form` is that one is generic and the other isn't,
so it's rather ugly.

I've never seen the kind of tight loop you suggest, which is why I have
the impression that we can afford to just make `interactive-form` into
a generic function, which is a simpler and cleaner API.

> Why can't we have `interactive-form' return some
> field of a given OClosure object instead?

One reason is that for the case of advice, I'd much rather compute the
interactive spec lazily (when the command is called) rather than when
the advice is built.

Another reason is that there is no dedicated "oclosure slot" for an
interactive-spec.  In theory we could use the byte-code object's slot
for that, but making it computable (as needed for bug#51695 and for
advice) would require significant changes to cconv.el and bytecomp.el
(and to make it not too inconvenient to use in `advice.el` it'd
additionally require extending the syntax of `interactive`).

We could add a dedicated "oclosure slot" for the interactive-spec, but
it'd likely be rather ugly, since that would need to be accessed from
the C in `cmds.c` but would require testing the type of the OClosure
first and that would have to be written in ELisp since it depends on how
OClosure types are represented which itself depends on `cl-defstruct`,
etc...

So if we want to go in this direction it'd be simpler and cleaner to
keep the C implementation of `interactive-form` and have it delegate to
a new `generic-interactive-form` when it finds an OClosure.


        Stefan






reply via email to

[Prev in Thread] Current Thread [Next in Thread]