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

["João Távora" <joaotavora@gmail.com>]

On Mon, Aug 16, 2021 at 10:09 AM Daniel Mendler <mail@daniel-mendler.de> wrote:

> There are serious drawbacks of attaching "private" string properties to
> arbitrary strings. For once it complicates debugging seriously if there
> are suddenly string properties attached to symbol names. It also leads
> to a potential memory leak.

Please, in the name of the sanity of this discussion, justify these two
statements with examples or follow them with a clause like "because...".

> 3. The `completion-filter-completions` API is the fastest possible API

Again that's quite a statement that I cannot evaluate the veracity of
without hard proof.

What I _can_ evaluate is the material you have put forward, and using
your patch and your Vertico completion software, version 0.14, the very
latest one. I try

   emacs -Q -f package-initialize benchmark.el

where benchmark.el is:

   (setq completion-styles '(flex))
   (defmacro heyhey ()
     `(progn
        ,@(cl-loop repeat 300000
           collect `(defun ,(intern (symbol-name (gensym "heyhey"))) () 42))))
   (heyhey)

I then turn on vertico-mode and press C-h f.  It takes about 4-5 seconds.
It's *faster* than if I do the same with fido-vertical-mode and the current
master, but is noticeably *slower* than if I do the same with the patch
provided earlier and available at scratch/icomplete-lazy-highlight-attempt-2.

Unfortunately, I cannot measure quantitatively here, because I don't
know how to tell Vertico to wait until it gets the correct result.
In other words, take this form:

    (completing-read "bla" obarray)<cursor here>

if you type C-u C-x C-e C-m veeery s-l-o-w-l-y in Vertico, if prints
, correctly, the character "%".  But if you evaluate it quickly wrapped
in a benchmark-run, it returns immediately and prints "".

In fido-mode, it always waits blockinly until it prints the correct result
and the time it took it to achieve that result.  Not questioning if this is
a bug in Vertico, but it would help if it could do the same, or be
configured to do the same, so that we can measure.

Without that, we can't evaluate the speed of Vertico where,
presumably, the fastest API in the world is being used right now.

> 4. If 'completion-all-completions' does indeed get slower thanks to my
> patch, it is a performance regression of my patch. I will fix this. And
> I thank you João for bringing this to my attention. However one should
> also consider that in the end, 'fido-mode' and 'icomplete-mode' should
> move to the new API 'completion-filter-completions' such that they
> benefit from the fast filtering and only copy and highlight the actually
> displayed strings.

Maybe they will, maybe they will.  But it's still quite early to decide if
we're going to move all frontends to that API.  There's no manual
documentation for it. Conceivably, if you appreciate your API so, you
could demonstrate in practice us how easy it is to use by providing
a separate patch that adapts icomplete-mode and fido-mode to use it.

Then, I or other fido-mode users could test it for a while, evaluating
its speed and correctness.  If it performs well and the completion
API architects have a good outlook for it, I see no reason why it
shouldn't be merged and eventually supersede the new one.

In the meantime, there is a patch with a measured and documented
performance boost where fido-mode and icomplete-mode move
opt-in to a new `completion-lazy-hilit` feature in the "old" API with
a total or four 1-line changes.  That patch lives in the branch
scratch/icomplete-lazy-highlight-attempt-2.

I think we should move to that, solving the bug#48841 while we
do the evaluation of all aspects of your contributions.

João Távora