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

[João Távora]

Dmitry Gutov <dgutov@yandex.ru> writes:

> On 16.08.2021 16:21, Eli Zaretskii wrote:
>
>>>> FWIW, I also don't understand how adding properties could cause a
>>>> memory leak.  When a string is GCed, its properties get GCed as well,
>>>> all of them.  Am I missing something?
>>>
>>> If you add string properties to all symbol names this memory will stay
>>> alive for much longer than necessary.
>> That's a very extreme example, something that I wouldn't expect a
>> Lisp
>> program to do, without removing the properties shortly thereafter.
>
> And that *will* happen with Joao's approach, as soon as some package
> implements a Lisp completion backend in a certain (legal) fashion.

There is no leak, not in the strong or weak sense. There is a constant
usage memory footprint proportional to the size of obarray, yes, but the
factor isn't huge.  From the top of my head, I think it's about two
conses and a fp number for each sym. does anyone know how much that is
or can teach me how to measure?  Thanks.

Anyway the current situation is constant copies of strings that put
pressure on the GC, as the benchmarks show.

Anyhoo, in the interest of placating this string property bogeyman, I've
prepared a version of my patch that is based on weak-keyed hash tables.
Slightly slower, but not much. Here are the usual benchmarks:

   (defmacro heyhey ()
     `(progn
        ,@(cl-loop repeat 300000
                  collect `(defun ,(intern (symbol-name (gensym "heyhey"))) () 
42))))
   (setq completion-styles '(flex))
   (heyhey)
   (when nil
     ;; Press C-u C-x C-e C-m quickly to produce a sample
     (benchmark-run (completing-read "" obarray))
    
     ;; my patch with private string properties
     (3.596972924 4 1.125298095999998)
     (3.584963294 4 1.1266740010000014)
     (3.4622216089999998 4 1.0924070069999985)
     (3.565632813 4 1.1066678320000012)
     (3.456130054 4 1.099950519)
     (3.49538751 4 1.1085273779999998)
     (3.4882531059999997 4 1.0928655200000001)
     (3.526581152 4 1.0909935229999999)
     (3.710919876 4 1.13883417)
     (3.576690379 4 1.09514685)
    
     ;; my patch with an no string properties (global weak hts)
     ;; Probably the extra gc sweeps are paranoid cleaning up of the
     ;; hash tables.
     (3.981110008 7 1.6466288340000013)
     (3.819598429 7 1.5200578379999996)
     (3.823931386 7 1.5175787589999992)
     (3.9161236720000003 7 1.6184865899999998)
     (3.835148066 7 1.5686207249999988)
     (3.791906221 7 1.5481051090000015)
     (3.798378812 7 1.5164137029999996)
     (4.049880173 7 1.7670989089999996)
     (3.716469474 6 1.3442434509999996)
     (3.422806969 6 1.3272816180000002)
    
     ;; current master
     (5.405534502 12 2.8778620629999994)
     (5.038353216999999 12 2.553688440000002)
     (4.94358915 12 2.4917956500000003)
     (4.950710861 12 2.4638737510000013)
     (5.0242796929999995 12 2.5226992029999984)
     (5.020964648 12 2.495171900999999)
     (4.914088866 12 2.4218276250000024)
     (5.003779622 12 2.502260272000001)
     (4.969347707 12 2.4814790469999988)
     (5.376038238 11 2.565757513000001)
    
     ;; didn't bother with daniel's patch as I've already shown it to be
     ;; about 10% slower than current master.
     
     )

The patch lives in the branch
scratch/icomplete-lazy-highlight-no-string-props.  It's a bit more
complicated to follow, but not much if you understand hash tables.  The
interface to icomplete.el is completely unchanged.

All in all, still a very good improvement over the current situation,
and I think I can make it faster.

(Though really do consider Eli's arguments the fastest approach)

>> And even that isn't a leak.
>> Note that we already put all kind of properties (although not text
>> properties) on symbols.
>
> Those do not, generally, change over time.

Neither does this one! At least in size, which is the thing that
matters.  So in terms of "negative" consequences it's exactly
equivalent. Read the patch it will be obvious, I think.

João