[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#53432: [PATCH] Avoid losing keyboard input when inotify is too busy
From: |
Michael Albinus |
Subject: |
bug#53432: [PATCH] Avoid losing keyboard input when inotify is too busy [and 1 more messages] |
Date: |
Mon, 24 Jan 2022 15:42:33 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/29.0.50 (gnu/linux) |
Ian Jackson <ijackson@chiark.greenend.org.uk> writes:
> Hi. Thanks for your really helpful reply!
Hi Ian,
> I guess we probably want a "file notifications might have been missed"
> callback. Having it be a callback like a normal file event callback
> will probably make it easiest to deal with.
Yep.
> Another possibility would be simply to fire every file notification
> event with a dummy set of information. But that would probably
> involve changing a lot more code.
Yes, and it might also be a performance degradation.
>> 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.
>
> Hrm. I don't think I have an opinion about this. But I think the
> design with a fixed size buffer, implying that input to the buffer
> needs to be suspended and resumed, means that *every* piece of code
> that might write into this buffer must also participate in the flow
> control mechanism.
>
> I don't think separating out the input buffers would remove that
> requirement. Ie, the ability to cope correctly with a flood of events
> (of whatever kind) will remain necessary.
Yes. But if we divide the keyboard buffer (also good for mouse events
and other events which do not harm) from the D-Bus and file notification
events, such a check is needed at fewer places.
>> 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.
>
> Yes. How alarming. Well, this is a bigger problem.
>
> From my personal point of view I have effort available to fix inotify
> and kqueue, provided we can agree a way forward. I looked at kqueue.c
> and it writes only file notification events to the buffer, so the same
> "file notifications possibly missed" callback would suffice.
Yes. Same also for dbusbind.c and gfilenotify.c. For w32notify.c I'm not
sure, but likely also the same situation.
> But I think the keyboard buffer suspend/resume, with
> event-generating-code specific callbacks for suspend
> ("hold_keyboard_input" etc. calls in "kbd_buffer_store_buffered_event")
> and resume (in "kbd_buffer_get_event") is not a terrible design.
>
> I think every one of these other event sources ought to be able to
> partake of this scheme (adding their own hooks like I proposed for
> inotify) without undue difficulty.
Yes. But I believe we don't need to handle lost events. If we have a
mean to inform the upper libraries, filenotify.el and dbus.el, about
this case, it would be sufficient. A simple check whether the event
buffer is full.
> The alternative would seem to be some kind of dynamic registration
> system. As I say, splitting the event input buffer into "keyboard"
> and "other things" doesn't seem like it would obviate the need for
> this plubming.
The advantage of splitting into "keyboard" and "other things" buffers
would be, that the keyboard buffer doesn't overrun, whatever burst of
D-Bus or file notification events arrives.
> I was imagining a callback hook into the inotify code for "keyboard
> buffer has become empty", which would check for "have we set the flag
> for lost events". If the flag was set, it would drain the inotify fd,
> make *one* "lost events" callback, and start reading the inotify fd
> again.
>
> The result would be that if any of the buffers overflowed, we would
> stop processing inotify events altogether until emacs has caught up
> with the user's keyboard input, and then (via the file notification
> "maybe lost" callback, do one check on every file which we might want
> to revert).
I don't believe that the native backends, like inotify.c, deserve too
much intelligence. They shall do stupid event receiving and reporting,
with a callback invoked in case of problems. This callback must be
clever then.
>> > 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.
>
> Would it be OK to postpone splitting this out ? I don't think
> splitting this up is necessary to fix these lost notifications.
Not to fix lost events. But it helps to fix lost keyboard strokes.
> I think I will let experts on glib, and the glib code in emacs, handle
> this.
>
> I guess if I'm fixing this for inotify, I should at least add a
> comment next to kbd_buffer_store_event saying "you may not call this
> unless you participate in the flow control protocol" and add FIXMEs to
> the call sites that currently don't.
>
> All of those use cases will have the practical consequences of these
> bugs reduced by using a bigger buffer.
>
> I guess the next thing I should do is go and read the file
> notification lisp code to see how a "please do a one-off poll"
> callback could be implemented.
Hmm. We are still in different camps about the approach ...
> Ian.
Best regards, Michael.
- bug#53432: [PATCH] Avoid losing keyboard input when inotify is too busy [and 1 more messages], (continued)
- bug#53432: [PATCH] Avoid losing keyboard input when inotify is too busy [and 1 more messages], Eli Zaretskii, 2022/01/23
- bug#53432: [PATCH] Avoid losing keyboard input when inotify is too busy [and 1 more messages], Michael Albinus, 2022/01/23
- bug#53432: [PATCH] Avoid losing keyboard input when inotify is too busy [and 1 more messages], Po Lu, 2022/01/22
- bug#53432: [PATCH] Avoid losing keyboard input when inotify is too busy [and 1 more messages], Ian Jackson, 2022/01/23
- bug#53432: [PATCH] Avoid losing keyboard input when inotify is too busy [and 1 more messages], Michael Albinus, 2022/01/23
- bug#53432: [PATCH] Avoid losing keyboard input when inotify is too busy [and 1 more messages], Ian Jackson, 2022/01/23
- bug#53432: [PATCH] Avoid losing keyboard input when inotify is too busy [and 1 more messages], Po Lu, 2022/01/23
- bug#53432: [PATCH] Avoid losing keyboard input when inotify is too busy [and 1 more messages],
Michael Albinus <=
- bug#53432: [PATCH] Avoid losing keyboard input when inotify is too busy [and 1 more messages], Eli Zaretskii, 2022/01/24
- bug#53432: [PATCH] Avoid losing keyboard input when inotify is too busy [and 1 more messages], Michael Albinus, 2022/01/24
- bug#53432: [PATCH] Avoid losing keyboard input when inotify is too busy [and 1 more messages], Eli Zaretskii, 2022/01/24
- bug#53432: [PATCH] Avoid losing keyboard input when inotify is too busy [and 1 more messages], Michael Albinus, 2022/01/24
- bug#53432: [PATCH] Avoid losing keyboard input when inotify is too busy [and 1 more messages], Eli Zaretskii, 2022/01/24
- bug#53432: [PATCH] Avoid losing keyboard input when inotify is too busy [and 1 more messages], Ian Jackson, 2022/01/24
- bug#53432: [PATCH] Avoid losing keyboard input when inotify is too busy [and 1 more messages], Michael Albinus, 2022/01/24
- bug#53432: [PATCH] Avoid losing keyboard input when inotify is too busy [and 1 more messages], Ian Jackson, 2022/01/24
- bug#53432: [PATCH] Avoid losing keyboard input when inotify is too busy [and 1 more messages], Michael Albinus, 2022/01/25
- bug#53432: [PATCH] Avoid losing keyboard input when inotify is too busy [and 1 more messages], Po Lu, 2022/01/23