Re: noverlay branch

[Matt Armstrong <matt@rfc20.org>]

Stefan Kangas <stefankangas@gmail.com> writes:

> Matt Armstrong <matt@rfc20.org> writes:
>
>> TLDR: things are looking pretty stable and good on the feature/noverlay
>> branch.  If anyone here tested it a month ago and found it lacking, give
>> it another whirl.
>
> Would it be welcome to merge master into that feature branch?

I'm leaving the merge decision up to Stefan and other committers.  I'm
happy to accept commit privs and help out with this, but I'm still
pretty green.

There are two things I think should be done before a merge:

1) Relax some of the 'echeck' calls in src/itree.c.  They are too
expensive and actually make --enable-checking builds much slower than
they are on 'master' when there are non-trivial numbers of overlays.
Stefan has added FIXME comments for most of these.

2) Run some benchmarks.  My preliminary results seem to show that Emacs
redisplay can be marginally slower when the overlay count is low (with
what we'd consider "acceptable numbers" of overlays today), but much
faster when things get out of hand.

3) Anything Stefan has in his head.  :)

> BTW, I can see two warnings, with gcc 12.2.0:
>
> In function ‘interval_tree_remove_fix’,
>     inlined from ‘itree_remove’ at itree.c:1110:5:
> itree.c:923:15: warning: potential null pointer dereference [-Wnull-dereference]
>   923 |           if (null_safe_is_black (other->left) /* 2.a */
>       |               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> itree.c:960:15: warning: potential null pointer dereference [-Wnull-dereference]
>   960 |           if (null_safe_is_black (other->right) /* 2.b */
>       |               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Those warnings are false positives, though the invariant relies on
red-black in variants and is too complex for a compiler to realize.
Heck, I have to sit down and draw things out to re-prove it to myself
every time.  https://git.sr.ht/~matta/emacs/log/scratch/matta/for_stefan
has a patch for it (which uses 'eassume'), or we could just check for
NULL even though it should never happen.

> I'm now running feature/noverlay here (with master merged locally),
> and will report back if I run into any issues.

I'd welcome a merge from master on feature/noverlay.  I think it has
been one month.