bug#47711: [PATCH VERSION 2] Add new `completion-filter-completions` API and deferred highlighting

[João Távora]

On Fri, Aug 13, 2021 at 12:21 PM Daniel Mendler <mail@daniel-mendler.de> wrote:

> No, this is not the case. There is no simple fix of the allocation issue
> on the frontend side.

I didn't claim that. At all. I claimed that the frontends that would be
affected by the (small)  backend patch are easy to adapt.  I think
you completely read past my idea.

> The existing API `completion-all-completions`
> necessarily has to allocate all the strings in order to attach
> highlighting and scoring.  The new API solves this in a clean way by
> both deferring highlighting and scoring.

I'm not sure you understand my alternative idea.  As far as I
understand (and have actually measured) the lines:

   ;; Don't modify the string itself.
   (setq str (copy-sequence str))

in minibuffer.el, in the function completion-pcm--hilit-commonality

Are the cause of the problem that _I am talking about_ and that
I have actually measured.  Again you may be referring to a
_different_ problem that I am unaware of.

If one removes these lines, the process becomes much faster, but there is a
problem with highlighting.  My idea is indeed to defer highlighting by not
setting the 'face property directly on that shared string, but some
other property
that is read later from the shared string by compliant frontents.

If you have understood this idea, can you comment on it?
(Preferably in terms of less adjectification regarding "cleanliness", but in
terms of actual drawbacks/advantages?)

The drawback that I can see in it is that frontends directly relying
on 'face are
broken by that patch. But according to Dmitry (and I tend to agree), it's
quite easy to address those frontends.  Most of them live in Emacs core or
GNU Elpa.

The advantage that I see is that those adaptations apart, it is a small
localized and effective change.

> I claim that my patch is easy to reason about and refactors the existing
> code to address the exact problem we are having. Please take some time
> in reviewing it.

I am already taking some time. I need your assistance in explaining the
problems first. I take into account your claims of cleanliness and elegance,
but in terms of their power of persuasion, they are much more limited
than hard material evidence.

> The main problem is that `completion-all-completions` allocates all the
> strings every time the completions are filtered.  This is the same
> performance issue you encountered in fido-mode/icomplete-mode.

OK. I encountered at least two different performance problems there, with
quite different causes. So let's stick to the string-allocation problem.  Post
a code snippet that demonstrates the problem the way you see it/experience it?

Some benchmark code would be very welcome.  You can probably grab my
benchmarking code from that other bug.

Then it becomes easy to study multiple solutions to that problem and
choose the best one!

> The second problem addressed by the new API
> `completion-filter-completions` is that `completion-all-completions` is
> limited in what it can return.  For example it cannot return the end
> position of the completion.

And why is this a problem? Can you post an example of something you'd
like to do, but can't?  Regardless, it does seem indeed like a "second" problem
(as you state) so perhaps something that can be addressed separately.

Is your particular solution to this second problem instrumental in solving
the "main problem"

> This is also solved by the new API.  The new API is a clean extensible way 
> forward.

I understand you've put time and effort into producing this work. We are
all indebted and I promise to read it. But every API writer in history of
programming has claimed those things and reality often shows otherwise.
So it's not that your work can't be those things you claim, maybe it is, but
generally the larger and broader the work the harder it is to reason about.

João