From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Ian Jackson Newsgroups: gmane.emacs.bugs Subject: bug#53432: [PATCH] Avoid losing keyboard input when inotify is too busy [and 1 more messages] Date: Mon, 24 Jan 2022 18:59:12 +0000 Message-ID: <25070.63232.560314.823060@chiark.greenend.org.uk> References: <25067.17249.604070.872185@chiark.greenend.org.uk> <838rv8nua8.fsf@gnu.org> <87r190wqo1.fsf@yahoo.com> <25068.23625.512978.147194@chiark.greenend.org.uk> <87a6fntgfh.fsf@yahoo.com> <25069.23134.887206.241281@chiark.greenend.org.uk> <877daqtnkw.fsf@gmx.de> <25069.40682.688423.883151@chiark.greenend.org.uk> <87lez5ry92.fsf@gmx.de> <25070.51632.699234.228230@chiark.greenend.org.uk> <87bl01rrmu.fsf@gmx.de> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="13906"; mail-complaints-to="usenet@ciao.gmane.io" Cc: Po Lu , 53432@debbugs.gnu.org To: Michael Albinus Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Mon Jan 24 20:27:28 2022 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1nC4zu-0003Gu-PF for geb-bug-gnu-emacs@m.gmane-mx.org; Mon, 24 Jan 2022 20:27:26 +0100 Original-Received: from localhost ([::1]:50016 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1nC4zt-000434-Lo for geb-bug-gnu-emacs@m.gmane-mx.org; Mon, 24 Jan 2022 14:27:25 -0500 Original-Received: from eggs.gnu.org ([209.51.188.92]:50474) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1nC4ZP-00069d-Bd for bug-gnu-emacs@gnu.org; Mon, 24 Jan 2022 14:00:03 -0500 Original-Received: from debbugs.gnu.org ([209.51.188.43]:52965) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1nC4ZO-0007Kc-UO for bug-gnu-emacs@gnu.org; Mon, 24 Jan 2022 14:00:02 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1nC4ZO-0007B5-QE for bug-gnu-emacs@gnu.org; Mon, 24 Jan 2022 14:00:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Ian Jackson Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Mon, 24 Jan 2022 19:00:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 53432 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 53432-submit@debbugs.gnu.org id=B53432.164305075827505 (code B ref 53432); Mon, 24 Jan 2022 19:00:02 +0000 Original-Received: (at 53432) by debbugs.gnu.org; 24 Jan 2022 18:59:18 +0000 Original-Received: from localhost ([127.0.0.1]:45868 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1nC4Yf-00079Z-I6 for submit@debbugs.gnu.org; Mon, 24 Jan 2022 13:59:18 -0500 Original-Received: from permutation-city.chiark.greenend.org.uk ([212.13.197.230]:50362 helo=chiark.greenend.org.uk ident=Debian-exim) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1nC4Yd-00079Q-De for 53432@debbugs.gnu.org; Mon, 24 Jan 2022 13:59:16 -0500 Original-Received: by chiark.greenend.org.uk (Debian Exim 4.89 #1) with local (return-path ijackson@chiark.greenend.org.uk) id 1nC4Ya-0002DP-LT; Mon, 24 Jan 2022 18:59:12 +0000 In-Reply-To: <87bl01rrmu.fsf@gmx.de> X-Mailer: VM 8.2.0b under 24.4.1 (i586-pc-linux-gnu) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.io gmane.emacs.bugs:225108 Archived-At: Michael Albinus writes ("Re: bug#53432: [PATCH] Avoid losing keyboard input when inotify is too busy [and 1 more messages]"): > Ian Jackson writes: > > I think we are just having a semantic difference here. To my mind > > "handling" lost events *consists of* notifying the next layer up that > > "some events may have been lost". Eventually this informaton will get > > to somewhere that can do something sensible with it. > > Yes, if we are speaking about the file notification backends, that's > all. And that's what I meant (sometimes, you must forgive me if I use > unprecise English). Oh, no need to apologise! It is very easy when discussing such complicated things for people to misunderstand each other. I just meant to clarify. > > The file notification code must have a "check all files we are > > polling" facility, for when file events are not available. Perhaps > > this is buried in a polling loop, but it could be made separately > > callable. > > I don't believe we can implement something at filenotify.el level. It is > up to the applications to handle lost events. autorevert could fall back > to polling, and it could continue to trust file notification events when > the situation becomes normal. How it is implement shall be the > responsibility of the application. Right, I think you are suggesting that the "some file event got lost" notification should bubble up to autorevert. Either directly, or via filenotify, I guess - not sure which you are suggesting would be better. That sounds correct to me. > > I think that the correct response to discovering that file events have > > been lost, is to do a one-off poll of every file. Perhaps we would > > want to rate limit this, but that should be done by *deferring* a > > poll-all, not by *skipping* one, as skipping would introduce a race > > that might lose a buffer revert. But I don't think this is actually > > necessary. In any case no keyboard events will be lost. > > For autorevert (as example), using polling would sync all buffers > towards the real state of the file on disk. Restarting file notification > later on wouldn't be a problem. Precisely. > > So there would want to be some kind of timer there, and the system > > would have two modes. It seems simpler to me to have only one mode. > > Writing the code to switch modes and coordinate everything seems > > error-prone to me. > > Timer doesn't sound good to me. A better approach might be, if the file > notification libraries send a "back to normal" triger, perhaps also via > a callback. 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.) 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. 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. 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. 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. 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. 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. -- Ian Jackson 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.