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: Sun, 23 Jan 2022 13:38:38 +0000 Message-ID: <25069.23134.887206.241281@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> 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="26722"; mail-complaints-to="usenet@ciao.gmane.io" Cc: 53432@debbugs.gnu.org To: Po Lu Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Sun Jan 23 14:39:29 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 1nBd5a-0006ip-JX for geb-bug-gnu-emacs@m.gmane-mx.org; Sun, 23 Jan 2022 14:39:26 +0100 Original-Received: from localhost ([::1]:41834 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1nBd5Z-0005Xl-Kx for geb-bug-gnu-emacs@m.gmane-mx.org; Sun, 23 Jan 2022 08:39:25 -0500 Original-Received: from eggs.gnu.org ([209.51.188.92]:49434) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1nBd5D-0005Us-PE for bug-gnu-emacs@gnu.org; Sun, 23 Jan 2022 08:39:03 -0500 Original-Received: from debbugs.gnu.org ([209.51.188.43]:46006) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1nBd5C-0001cI-GC for bug-gnu-emacs@gnu.org; Sun, 23 Jan 2022 08:39:03 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1nBd5C-0001bB-FM for bug-gnu-emacs@gnu.org; Sun, 23 Jan 2022 08:39: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: Sun, 23 Jan 2022 13:39: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.16429451256088 (code B ref 53432); Sun, 23 Jan 2022 13:39:02 +0000 Original-Received: (at 53432) by debbugs.gnu.org; 23 Jan 2022 13:38:45 +0000 Original-Received: from localhost ([127.0.0.1]:38904 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1nBd4v-0001a7-DQ for submit@debbugs.gnu.org; Sun, 23 Jan 2022 08:38:45 -0500 Original-Received: from permutation-city.chiark.greenend.org.uk ([212.13.197.230]:33282 helo=chiark.greenend.org.uk ident=Debian-exim) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1nBd4t-0001Zw-Cc for 53432@debbugs.gnu.org; Sun, 23 Jan 2022 08:38:43 -0500 Original-Received: by chiark.greenend.org.uk (Debian Exim 4.89 #1) with local (return-path ijackson@chiark.greenend.org.uk) id 1nBd4p-0006AK-T2; Sun, 23 Jan 2022 13:38:40 +0000 In-Reply-To: <87a6fntgfh.fsf@yahoo.com> 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:224913 Archived-At: Po Lu writes ("Re: bug#53432: [PATCH] Avoid losing keyboard input when inotify is too busy [and 1 more messages]"): > It can't end up with the user unknowingly editing the wrong version of a > file, because Emacs will ask him: > > foo.h changed on disk; really edit the buffer? (y, n, r or C-h) Good point. That means that losing the file notifications isn't terrible. It's still not great, though. In fact now that you point this out, I think this (lost file notifications resulting in this prompt) has happened to me on more than one occasion. It made me uneasy; I thought "why does it sometimes do this, is there something I am doing wrong, or did I miskey?" 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. 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. 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. 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. 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. 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 ? 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 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. > > If it is OK for file notifications to get lost, because the need for a > > buffer revert will be picked up another way somehow, then your > > suggestion makes perfect sense to me. But in that case, I don't > > understand why this code doesn't use a buffer of fixed size (or, at > > least, limited size). > > Which code? The keyboard buffer is the "buffer of fixed size" in the > inotify code. 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. If we can have some proper way of handling lost inotify events (including IN_Q_OVERFLOW) then the inotify code could have a fixed size buffer or a fixed limit on the amount it owuld read. For performance reasons (see above) this ought to be reasonably large (tens of kbytes I guess). > 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. Po Lu writes ("Re: bug#53432: [PATCH] Avoid losing keyboard input when inotify is too busy [and 1 more messages]"): > The underlying objection is that it's IMHO wrong to allocate anything on > the heap to cater to "abuse" of a feature, in this case inotify. I really regret (ab)using the word "abuse" in my original message. I experienced actual lost keystrokes with emacs 26 in in my usual configuration. I don't think I was doing anything unreasonable. The build system of the project I was working on does do quite a lot of file writing in a single directory but that is hardly so unusual. 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. Thanks, Ian. -- 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.