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

[Po Lu]

Ian Jackson <ijackson@chiark.greenend.org.uk> writes:

> 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.

It's too late to do anything of this sort on the release branch.

>  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.

Why is that?  I'm not convinced that's necessary, since you're the only
person to complain so far, and you needed to abuse the feature in order
to get keys to be dropped.

>  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.

40k is unacceptable.

> 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.

We have many other file notification systems, essentially one for each
platform we support.  The one that people actually build Emacs with is
in gfilenotify.c, and you can't get it to experience such problems
since it will silently drop file notifications once its own internal
buffer fills up.  GLib does not tell you when that happens, because it
happens rarely enough to be worth taking into account.

> 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.

Someone has to demonstrate that running `stat' every five seconds
actually results in the visible decrease of battery life before we take
that into account.

> 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 don't want to drop inotify events when keyboard input is paused, we
only want to drop them when `kbd_buffer_nr_stored' is equal to
KBD_BUFFER_SIZE.

> We would still probably want to have a significantly larger keyboard
> buffer, at leat when inotify is enabled.

The keyboard buffer is read very rapidly in most cases, so that
shouldn't be necessary at all.

> 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.

That sounds like premature optimization to me.  Between Emacs 25 and 26,
file notifications were disabled for global-auto-revert-mode, and I
don't think anyone complained very loudly.

> 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.

SAFE_ALLOCA does not usually allocate anything on the heap, so its
impact is negligible.

> 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.

It does not.  It happens rarely enough that the GLib target audience
doesn't care about when file notifications are actually dropped.

> 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.

Why is that so?