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

[Michael Albinus]

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

Hi,

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

I agree. But then, inotify (and the other file notification backends)
shall inform that this happened. Either by an error signal, or by
calling a handler.

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

For this, I have no opinion. However, I remember vaguely that Stefan
said once that we shall not (mis-)use the keyboard buffer for incoming
events, like from file notification or from D-Bus. Another mechanism is
needed. This idea hasn't been implemented.

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

The following packages inject events via the keyboard buffer:
dbusbind.c, gfilenotify.c, inotify.c, kqueue.c, w32notify.c. All of them
could see a burst of incoming events. It isn't just an inotify problem.

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

Yep. When the file notification backend reports a loss of event, the
autorevert package might fall back to polling. Perhaps we could even
raise another signal that things are back to normal, and packages like
autorevert could switch back to file notification.

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

Yep, this another case (beside of keyboard buffer full) where a "file
notification broken" signal is useful.

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

Don't know off-hand, but it shall be possible. However, if autorevert
falls back to polling anyway, we get this for free.

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

Agreed.

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

As said above: perhaps it isn't the best idea to handle such events via
the keyboard buffer.

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

glib uses several native backends, like inotify or kqueue. It has also a
backend which polls. However, I don't know whether it switches the
backend during runtime.

> Thanks,
> Ian.

Best regards, Michael.