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 Ian,

> Let me consider the semantics of these notifications you are
> suggesting in a bit more detail.  I hope you will forgive the
> resulting screed:
>
> As I understand your proposal, the system starts in "normal" state.
> In "normal" state, the file notification system should promise that if
>   1. a file notification is set up
>   2. subsequently, that file is modified
> then the file notification system will make one of these callbacks
>   A. file notification event
>   B. "file notifications not working" callback
> (And, I gyuess, that this will be done "reasonably promptly" in some
> sense; I'm not sure if we need to define this.)

Thinking about, we don't need another callback. We can define a special
file notification event which tells us. Something like

'(file-notify <descriptor> overflow)

where <descriptor> is the result of a respective `file-notify-add-watch'
call, and `overflow' is a new action. `file-notify-handle-event' calls
the registered application specific handler with this event; that
handler is responsible for whatever.

I see two cases where we could inject such an event: the keyboard buffer
size exceeds a given high water mark, like 80% or 90%. Or (in the
inotify case) we receive an IN_Q_OVERFLOW event. Other backends might
know further situations when such an event is needed.

> If B happens then the system is now in "non-working" state.  In
> non-working state, all bets are off.  Users like autorevert must take
> alternative measures.

Yep. Application specific handlers, like `auto-revert-notify-handler',
must react.

> At some point, the file notification system will make a "file
> notifications working again now" callback.  I think this has to
> promise that, if:
>   1a. a file notification was set up
>   1b. the "notifications working" callback was made more recently
>        than any "notifications not working" callback
> (in either order) and then *subsequently*
>   2. a relevant file is notified
> then a notification will occur, as above.  Obviously it can't sensibly
> make guarantees about what happened before the "working" notifcation.

This might be signalled by a another artifical event like

'(file-notify <descriptor> normal)

which is treated like the other event. It could happen if the keyboard
buffer size falls below a given low water mark, say 40% or 50%. Or (in
the inotify case) we receive events w/o IN_Q_OVERFLOW bit mask.

(Action names like `overflow' and `normal' are random, any better
proposal is appreciated).

> How must a user of this API respond ?  Consider autorevert.  Suppose
> the notifications were declared "non-working" and autorevert resorted
> to polling.
>
> When autorevert is told things are working again, it wants to go back
> to not polling.  But it doesn't know whether notifications were lost
> between its last poll, and the file notifications becoming normal
> again.  There might be a few leftover lost events.
>
> So autorevert must poll every file again, once.  I think it makes most
> sense to do this immediately, rather than wait for the next poll
> interval.  That allows autorevert to avoid a confusing special-case
> transitional state where it is not polling but still has one poll to
> do - and it means the user gets the buffers reverted as soon as
> possible rather than after a timeout.

I believe it would be sufficient to signal "after next poll we're going
back to file notifications". This is transitional, but not very hard to
implement. And we will wait 5 seconds max, which doesn't sound too terrible.

> Now, consider the behaviour of the file notification system.
>
> In practice, inotify.c will probably want to declare itself working as
> soon as the event buffer it is writing to becomes empty.  Doing so
> sooner merely imposes more load on a busy emacs; doing so later
> degrades the user experience by having the emacs unnecessarily wait
> before catching up with the work that needs to be done.
>
> What this means is that the file notification system is only in the
> "non-working" state when emacs is too busy handling important events
> to catch its breath and handle file notifications.  Ie, only when
> emacs is not idle.
>
> In this state, we do not actually want to poll for file notifications!
> It doesn't make sense to make this fire off a timer.

Well, we need to read the incoming events, but we won't push them to the
keyboard buffer. Until the low watermark has been reached, which means
Emacs isn't too busy anymore.

> So autorevert does not actually need to do anything with the "not
> working" notification.  It should wait for the "working again"
> notification, and respond by polling all the files exactly once each.

Why not do default polling, every 5 seconds? As said, the "not working"
state doesn't mean always that Emacs is too busy. It could also be the
case, that too many events are fired by the kernel, events are lost,
and we are informed about by IN_Q_OVERFLOW.

> I think a similar situation will arise for other kinds of event
> generation.  So we don't actually need two separate notifications.
> We only need one, which means: "err, sorry, it turns out things
> weren't working, but I think they're OK now - and I'll let you know
> again if not".
>
> The result of my approach as I describe above will be an emacs which,
> when overloaded, suspends file event handling in order to handle user
> input, but which promptly catches up with deferred work when it can.

Well, injecting the two new events gives the decision what to do to the
application handlers. They can just sit and wait, or they can try to
receive needed information otherwise, like autorevert does with polling.
And such a strategy could be customized.

Best regards, Michael.