[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#53242: [PATCH] unify reads from local_var_alist
From: |
Sergey Vinokurov |
Subject: |
bug#53242: [PATCH] unify reads from local_var_alist |
Date: |
Sat, 15 Jan 2022 17:54:02 +0000 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.4.1 |
On 15/01/2022 16:02, Corwin Brust wrote:
I tried to follow this conversation but it wasn't clear to me what out
motive is for this change.
The motive is that prior to change the alist with buffer-local variables
was handled inconsistently. Sometimes with Fassoc, other times with
Fassq and even assq_no_quit (the one that doesn't allow interrupts).
Since the keys of alist are symbols (variable names), it doesn't make
sense to use Fassoc which compares them with Fequal - an Fassq which
does the comparison which simpler Feq would suffice.
I had understood we typically make (especially in the c sources) our
changes to achieve specific, tangible improvement. Is that the case
here? is the particularly oppressive 'tech debt'? In the latter case,
does history reflect consideration wrt the original selections in each
of the various cases we hereby change?
I don't know whether this is an oppressive tech debt, but from my
perspective I have taken a look over handling of buffer-local variables
during hacking some elisp code and saw the inconsistency. My patch is
just an effort to reduce it and try to make Emacs a little bit better
than it was before.
I don't know what the improvement will be, probably in will be pretty small.
My main consideration for selecting which function to use it to look at
the types, notice that this in an associative list with symbols as keys
and select the most appropriate function that would handle lookups in
the list.
Also (and especially if we must 'clean for the sake of cleanliness'),
could we prefer the (seeming more conservative of UX) interruptable
varient in this case? (Is that very costly? How costly and how have we
measured that?)
Some parts before the change were already using uninterruptible variant.
The Fassq does more work than assq_no_quit because it's not only
interruptible but also checks for circular lists whereas assq_no_quit
does not handle them correctly and would just loop forever. It is safe
to use assq_no_quit for buffer-local variables because this in Emacs
internal structure not visible to the user, Emacs fully maintains it and
does not make it into a circular list.
> Please consider the case of a package developer who may be abusing
buffer-local vars during experiments. It seems this will cause much
more ’oops, time to kill Emacs/grab a coffee'.
I think it's unrealistic to introduce, even accidentally, enough
buffer-local variables that lack of interruptibility in these particular
functions will start to show.
This is based on the following benchmark, which I encourage everyone to
try out. It creates a list of length n and does one lookup into it. This
corresponds to a buffer having n local variables and the lookup is the
operation we're arguing about (Fassq vs assq_no_quit). The assq_no_quit
is not exposed in elisp as it's not safe so the benchmark uses Fassq but
assq_no_quit will be pretty close as it does roughly the same amount of
work.
(defun mk-list (n)
(let ((res nil))
(dotimes (i n)
(push (cons i nil) res))
res))
(byte-compile #'mk-list)
(let* ((n 100000)
(xs (mk-list n)))
(benchmark-call
(lambda ()
(assq 'foo xs))
1))
It takes pretty large n to get the lookup take significant amount of
time (please note that list creation time is not included in the
calculation as it has nothing to do with Fassq vs assq_no_quit, so look
at what benchmark-call returns and not on how long it all subjectively
takes).
On my machine I need 10 000 000 elements for lookup to take 110 ms,
which is a noticeable amount of time (probably still bearable to work
with). For Emacs to "freeze" over, say 10 second, the number of
variables introduced has to be even higher.
Is having millions (or hundreds of millions...) of buffer-local
variables a reasonable scenario to consider? Please keep in mind that
even if this occurs, interruptibility will not make lookups finish
faster (even probably slower because of additional work that Fassq does
compared to assq_no_quit). Yes, user would be able to C-g out of an
operation if we use Fassq. But the operation will still be as much slow
the next time it's performed. User will do the 'oops, time to kill
Emacs/grab a coffee' sequence anyway in this case since any commands
looking at local variables will be affected. AND this is the case with
Emacs prior to my patch. Please do a benchmark and define 100 000 000
local variables and see whether Emacs works appropriately well in this case.
I argue that if we're really concerned what would happen when someone
defines a few hundred million local variables then we should not be
asking a question whether reads of said variables are interruptible but
should be asking a question whether linked list is the appropriate data
structure to store local variables in the first place (spoiler alert:
it's not).
- bug#53242: [PATCH] unify reads from local_var_alist, Sergey Vinokurov, 2022/01/13
- bug#53242: [PATCH] unify reads from local_var_alist, Lars Ingebrigtsen, 2022/01/14
- bug#53242: [PATCH] unify reads from local_var_alist, Eli Zaretskii, 2022/01/14
- bug#53242: [PATCH] unify reads from local_var_alist, Lars Ingebrigtsen, 2022/01/14
- bug#53242: [PATCH] unify reads from local_var_alist, Sergey Vinokurov, 2022/01/14
- bug#53242: [PATCH] unify reads from local_var_alist, Eli Zaretskii, 2022/01/14
- bug#53242: [PATCH] unify reads from local_var_alist, Sergey Vinokurov, 2022/01/14
- bug#53242: [PATCH] unify reads from local_var_alist, Eli Zaretskii, 2022/01/15
- bug#53242: [PATCH] unify reads from local_var_alist, Sergey Vinokurov, 2022/01/15
- bug#53242: [PATCH] unify reads from local_var_alist, Corwin Brust, 2022/01/15
- bug#53242: [PATCH] unify reads from local_var_alist,
Sergey Vinokurov <=