bug#53432: [PATCH] Avoid losing keyboard input when inotify is too busy [and 1 more messages]

[Ian Jackson <ijackson@chiark.greenend.org.uk>]

Po Lu writes ("Re: bug#53432: [PATCH] Avoid losing keyboard input when inotify is too busy [and 1 more messages]"):
> It can't end up with the user unknowingly editing the wrong version of a
> file, because Emacs will ask him:
> 
>   foo.h changed on disk; really edit the buffer? (y, n, r or C-h)

Good point.  That means that losing the file notifications isn't
terrible.  It's still not great, though.  In fact now that you point
this out, I think this (lost file notifications resulting in this
prompt) has happened to me on more than one occasion.  It made me
uneasy; I thought "why does it sometimes do this, is there something I
am doing wrong, or did I miskey?"


I suggest the following approach for stable branches:

 1. Have the inotify code drop notifications if kbd_on_hold_p().
    This will mean that keystrokes will never be lost.

 2. At least double the size of the keyboard buffer from 4k to 8k.
    I think this is necessary because in a stable branch we don't want
    to make anyone's experience worse.  Doing (1) will cause emacs to
    start to lose file notifications when the buffer is half full,
    rather than full.  Doubling the buffer (at trivial memory cost) is
    an easy way to fix this.

 3. Consider a significantly larger bump to the keyboard buffer to
    further reduce the incidence of this unreliability.  It's a single
    static buffer; we could easily change it from 4k to 40k (say) with
    negligible cost.

If desired, the increased keyboard buffer size could be made conditional on
inotify support so that systems which don't use the keyboard buffer
for file notifications don't pay a price.


For master, I still think we need to make global-auto-revert-mode
reliable.  We don't want to stop using file notifications for it
(where they are available), because we don't want emacs to be polling
- that is poor for battery life on laptops.

It is true that the kernel's inotify system may sometimes drop events
because the buffer in the kernel filled up - but when that happens, it
sends a IN_Q_OVERFLOW event, with -1 for the associated fd.
I.e. rather than completely losing events, it collapses them into a
single "you missed some stuff" event.

If we handled that notification, we could continue to use inotify and
fall back to a single scan through all the files, when we're told that
inotify was overloaded.  I haven't looked at the auto-revert code,
but, is there a way we could invoke a "one-off" poll ?

IF we did this then the inotify code could use the same mechanism to
handle the case where it dropped events due to the kbd_on_hold_p, and
everything would work correctly.  Complicated buffer management in the
inotify code wouldn't be needed.

We would still probably want to have a significantly larger keyboard
buffer, at leat when inotify is enabled.  This is because we can
reasonably expect a much higher rate of file notifications than
keyboard events.  Even if we have made emacs reliable when the buffer
fills up, we still don't *want* the buffer to fill up because the
non-file-notification based auto revert is a lot less efficient,
especially in a large emacs visiting many files: it will often be the
case that only a handful of files have changed, perhaps very many
times.

I definitely think we want to get (back) to the point where choosing
the keyboard buffer size can be done purely with respect to
performance considerations rather than worrying about lossage.


> > If it is OK for file notifications to get lost, because the need for a
> > buffer revert will be picked up another way somehow, then your
> > suggestion makes perfect sense to me.  But in that case, I don't
> > understand why this code doesn't use a buffer of fixed size (or, at
> > least, limited size).
> 
> Which code?  The keyboard buffer is the "buffer of fixed size" in the
> inotify code.

The inotify code has its own buffer in inotify_callback.  On each
pass, it calls FIONREAD and passes the return value to SAFE_ALLOCA.
I think this is quite pathological.

If we can have some proper way of handling lost inotify events
(including IN_Q_OVERFLOW) then the inotify code could have a fixed
size buffer or a fixed limit on the amount it owuld read.  For
performance reasons (see above) this ought to be reasonably large
(tens of kbytes I guess).

> All file notification systems are "best effort".  At the very least,
> dropping events when the keyboard buffer is full would make the behavior
> consistent with GLib file notifications, which drops notifications
> whenever its own internal buffer fills up.

I'm not familiar with glib.  Presuambly glib either exposes something
with the semantics of IN_Q_OVERFLOW, or internally converts that to a
notification to every watcher.  Otherwise programs using glib's API
would have to poll, which seems unlikely - I think that the glib
authors will be well aware of the desire to avoid polling for battery
life reasons.


Po Lu writes ("Re: bug#53432: [PATCH] Avoid losing keyboard input when inotify is too busy [and 1 more messages]"):
> The underlying objection is that it's IMHO wrong to allocate anything on
> the heap to cater to "abuse" of a feature, in this case inotify.

I really regret (ab)using the word "abuse" in my original message.

I experienced actual lost keystrokes with emacs 26 in in my usual
configuration.  I don't think I was doing anything unreasonable.  The
build system of the project I was working on does do quite a lot of
file writing in a single directory but that is hardly so unusual.

In emacs 27 and 28 it was necessary to be more aggressive to create a
situation where the bug would reproduce.  This is in the nature of
this kind of bug.


Thanks,
Ian.

-- 
Ian Jackson <ijackson@chiark.greenend.org.uk>   These opinions are my own.  

Pronouns: they/he.  If I emailed you from @fyvzl.net or @evade.org.uk,
that is a private address which bypasses my fierce spamfilter.