unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Clarification about auto-revert-mode and inotify
@ 2014-10-21  5:03 Dima Kogan
  0 siblings, 0 replies; 28+ messages in thread
From: Dima Kogan @ 2014-10-21  5:03 UTC (permalink / raw)
  To: emacs-devel

Hi.

I'm running an emacs recently built from source, and the
auto-revert-mode functionality is confusing to me.

I configured with --with-file-notification=inotify. With this I would
expect auto-revert-mode buffers to be updated immediately, possibly with
some throttling. I do not observe this; it still takes ~5 seconds for
the buffers to update.

Ltrace tells me that inotify_...() functions ARE being called. It also
tells me that inotify_callback() is called immediately after a file
update, but the buffer is not updated immediately. Is this intended
behavior or a bug? The recent 24.4 release touts inotify support in its
NEWS; does this mean simply that auto-revert is more efficient?

dima



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Clarification about auto-revert-mode and inotify
@ 2014-10-21  6:01 Dima Kogan
  0 siblings, 0 replies; 28+ messages in thread
From: Dima Kogan @ 2014-10-21  6:01 UTC (permalink / raw)
  To: emacs-devel

Hi.

I'm running an emacs recently built from source, and the
auto-revert-mode functionality is confusing to me.

I configured with --with-file-notification=inotify. With this I would
expect auto-revert-mode buffers to be updated immediately, possibly with
some throttling. I do not observe this; it still takes ~5 seconds for
the buffers to update.

Ltrace tells me that inotify_...() functions ARE being called. It also
tells me that inotify_callback() is called immediately after a file
update, but the buffer is not updated immediately. Is this intended
behavior or a bug? The recent 24.4 release touts inotify support in its
NEWS; does this mean simply that auto-revert is more efficient?

dima



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Clarification about auto-revert-mode and inotify
@ 2014-10-21  7:00 Dima Kogan
  2014-10-21 13:43 ` Stefan Monnier
  0 siblings, 1 reply; 28+ messages in thread
From: Dima Kogan @ 2014-10-21  7:00 UTC (permalink / raw)
  To: emacs-devel

Hi.

I'm running an emacs recently built from source, and the
auto-revert-mode functionality is confusing to me.

I configured with --with-file-notification=inotify. With this I would
expect auto-revert-mode buffers to be updated immediately, possibly with
some throttling. I do not observe this; it still takes ~5 seconds for
the buffers to update.

Ltrace tells me that inotify_...() functions ARE being called. It also
tells me that inotify_callback() is called immediately after a file
update, but the buffer is not updated immediately. Is this intended
behavior or a bug? The recent 24.4 release touts inotify support in its
NEWS; does this mean simply that auto-revert is more efficient?

dima



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Clarification about auto-revert-mode and inotify
  2014-10-21  7:00 Clarification about auto-revert-mode and inotify Dima Kogan
@ 2014-10-21 13:43 ` Stefan Monnier
  2014-10-21 15:08   ` Eli Zaretskii
  2014-10-21 17:59   ` Dima Kogan
  0 siblings, 2 replies; 28+ messages in thread
From: Stefan Monnier @ 2014-10-21 13:43 UTC (permalink / raw)
  To: Dima Kogan; +Cc: emacs-devel

> NEWS; does this mean simply that auto-revert is more efficient?

Indeed, IIUC the current auto-revert-mode implementation only uses the
file-notification code to reduce the amount of polling, but the revert
itself is still only performed from the 5s timer.

I don't think there's a strong technical reason for that,


        Stefan



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Clarification about auto-revert-mode and inotify
  2014-10-21 13:43 ` Stefan Monnier
@ 2014-10-21 15:08   ` Eli Zaretskii
  2014-10-21 16:28     ` Stefan Monnier
  2014-10-21 17:59   ` Dima Kogan
  1 sibling, 1 reply; 28+ messages in thread
From: Eli Zaretskii @ 2014-10-21 15:08 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: lists, emacs-devel

> From: Stefan Monnier <monnier@IRO.UMontreal.CA>
> Date: Tue, 21 Oct 2014 09:43:29 -0400
> Cc: emacs-devel@gnu.org
> 
> > NEWS; does this mean simply that auto-revert is more efficient?
> 
> Indeed, IIUC the current auto-revert-mode implementation only uses the
> file-notification code to reduce the amount of polling, but the revert
> itself is still only performed from the 5s timer.
> 
> I don't think there's a strong technical reason for that,

The value of auto-revert-interval can be customized to a smaller
value.



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Clarification about auto-revert-mode and inotify
  2014-10-21 15:08   ` Eli Zaretskii
@ 2014-10-21 16:28     ` Stefan Monnier
  2014-10-21 17:47       ` Eli Zaretskii
  0 siblings, 1 reply; 28+ messages in thread
From: Stefan Monnier @ 2014-10-21 16:28 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: lists, emacs-devel

> The value of auto-revert-interval can be customized to
> a smaller value.

But what happens if you set it to, say, 0.01s ?


        Stefan



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Clarification about auto-revert-mode and inotify
  2014-10-21 16:28     ` Stefan Monnier
@ 2014-10-21 17:47       ` Eli Zaretskii
  2014-10-21 20:23         ` Stefan Monnier
  0 siblings, 1 reply; 28+ messages in thread
From: Eli Zaretskii @ 2014-10-21 17:47 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: lists, emacs-devel

> From: Stefan Monnier <monnier@IRO.UMontreal.CA>
> Cc: lists@dima.secretsauce.net, emacs-devel@gnu.org
> Date: Tue, 21 Oct 2014 12:28:54 -0400
> 
> > The value of auto-revert-interval can be customized to
> > a smaller value.
> 
> But what happens if you set it to, say, 0.01s ?

Not sure I understand the question.  The timer runs every 0.01s and
refreshes the buffers that are marked as being in need of reverting.
But you already know that.



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Clarification about auto-revert-mode and inotify
  2014-10-21 13:43 ` Stefan Monnier
  2014-10-21 15:08   ` Eli Zaretskii
@ 2014-10-21 17:59   ` Dima Kogan
  2014-10-21 18:34     ` Eli Zaretskii
  1 sibling, 1 reply; 28+ messages in thread
From: Dima Kogan @ 2014-10-21 17:59 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

[-- Attachment #1: Type: text/plain, Size: 731 bytes --]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>> NEWS; does this mean simply that auto-revert is more efficient?
>
> Indeed, IIUC the current auto-revert-mode implementation only uses the
> file-notification code to reduce the amount of polling, but the revert
> itself is still only performed from the 5s timer.
>
> I don't think there's a strong technical reason for that,

OK.

I'm attaching a prototype patch to fix this. It updates the buffer
immediately when a change is detected, unless the previous such update
happened very recently. If it DID happen very recently, the normal
timer-based update kicks in later.

Is such a patch reasonable? If so, I'll add the proper comments and
docstrings, and resubmit it.

dima


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: emacs_notify_immediate_prototype.patch --]
[-- Type: text/x-diff, Size: 1489 bytes --]

diff --git a/lisp/autorevert.el b/lisp/autorevert.el
index f1074e2..00e1664 100644
--- a/lisp/autorevert.el
+++ b/lisp/autorevert.el
@@ -531,6 +533,15 @@ will use an up-to-date value of `auto-revert-interval'"
       ;; Fallback to file checks.
       (set (make-local-variable 'auto-revert-use-notify) nil))))
 
+
+
+(defvar auto-revert-notify-lockout-timer nil
+  "Lockout timer used by Auto-Revert Mode.")
+(make-variable-buffer-local 'auto-revert-notify-lockout-timer)
+
+(defvar auto-revert-notify-lockout-interval 3
+  "Lockout interval for the notify timers")
+
 (defun auto-revert-notify-handler (event)
   "Handle an EVENT returned from file notification."
   (with-demoted-errors
@@ -566,6 +577,15 @@ will use an up-to-date value of `auto-revert-interval'"
                                 (file-name-nondirectory buffer-file-name)))))
                 ;; Mark buffer modified.
                 (setq auto-revert-notify-modified-p t)
+
+                (unless (timerp auto-revert-notify-lockout-timer)
+                  (auto-revert-handler 1)
+
+                  (setq auto-revert-notify-lockout-timer
+                        (run-with-timer auto-revert-notify-lockout-interval nil                                        
+                                        (lambda ()
+                                          (setq auto-revert-notify-lockout-timer nil)))))
+                
                 ;; No need to check other buffers.
                 (cl-return)))))))))
 

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: Clarification about auto-revert-mode and inotify
  2014-10-21 17:59   ` Dima Kogan
@ 2014-10-21 18:34     ` Eli Zaretskii
  2014-10-21 18:59       ` Dima Kogan
  0 siblings, 1 reply; 28+ messages in thread
From: Eli Zaretskii @ 2014-10-21 18:34 UTC (permalink / raw)
  To: Dima Kogan; +Cc: monnier, emacs-devel

> From: Dima Kogan <lists@dima.secretsauce.net>
> Date: Tue, 21 Oct 2014 10:59:29 -0700
> Cc: emacs-devel@gnu.org
> 
> > Indeed, IIUC the current auto-revert-mode implementation only uses the
> > file-notification code to reduce the amount of polling, but the revert
> > itself is still only performed from the 5s timer.
> >
> > I don't think there's a strong technical reason for that,
> 
> OK.
> 
> I'm attaching a prototype patch to fix this. It updates the buffer
> immediately when a change is detected, unless the previous such update
> happened very recently. If it DID happen very recently, the normal
> timer-based update kicks in later.
> 
> Is such a patch reasonable? If so, I'll add the proper comments and
> docstrings, and resubmit it.

I'm sorry, but I still don't understand what's wrong with customizing
auto-revert-interval.  It seems to allow what you want, and also what
others might want.  So where's the problem that requires another time,
and the rest of the complexity you are suggesting?



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Clarification about auto-revert-mode and inotify
  2014-10-21 18:34     ` Eli Zaretskii
@ 2014-10-21 18:59       ` Dima Kogan
  2014-10-21 19:36         ` Eli Zaretskii
  0 siblings, 1 reply; 28+ messages in thread
From: Dima Kogan @ 2014-10-21 18:59 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: monnier, emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Dima Kogan <lists@dima.secretsauce.net>
>> 
>> > Indeed, IIUC the current auto-revert-mode implementation only uses the
>> > file-notification code to reduce the amount of polling, but the revert
>> > itself is still only performed from the 5s timer.
>> 
>> I'm attaching a prototype patch to fix this.
>
> I'm sorry, but I still don't understand what's wrong with customizing
> auto-revert-interval.  It seems to allow what you want, and also what
> others might want.  So where's the problem that requires another time,
> and the rest of the complexity you are suggesting?

The issue I'm trying to solve is that there's a delay between when a
file is modified and when auto-revert-mode kicks in. Compare the
user-observable behavior of running 'tail -f file' in a shell, and doing
the supposed equivalent of auto-revert-tail-mode in emacs: with 'tail
-f' the users sees updates instantly, but with auto-revert-tail-mode
they do not.

Before emacs knew about things like inotify, getting this parity would
require rapid polling by emacs, but now we can do this efficiently.
Emacs already does 99% of this, the attached patch just does the last
little bit.



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Clarification about auto-revert-mode and inotify
  2014-10-21 18:59       ` Dima Kogan
@ 2014-10-21 19:36         ` Eli Zaretskii
  2014-10-21 19:39           ` Dima Kogan
  0 siblings, 1 reply; 28+ messages in thread
From: Eli Zaretskii @ 2014-10-21 19:36 UTC (permalink / raw)
  To: Dima Kogan; +Cc: monnier, emacs-devel

> From: Dima Kogan <dima@secretsauce.net>
> Cc: monnier@IRO.UMontreal.CA, emacs-devel@gnu.org
> Date: Tue, 21 Oct 2014 11:59:00 -0700
> 
> The issue I'm trying to solve is that there's a delay between when a
> file is modified and when auto-revert-mode kicks in.

A "delay" of, say, 0.01 seconds is indistinguishable from zero.

> Compare the user-observable behavior of running 'tail -f file' in a
> shell, and doing the supposed equivalent of auto-revert-tail-mode in
> emacs: with 'tail -f' the users sees updates instantly, but with
> auto-revert-tail-mode they do not.

Did you try decreasing the interval to  low enough value?  If not,
please do.

> Before emacs knew about things like inotify, getting this parity would
> require rapid polling by emacs, but now we can do this efficiently.

File notifications are not immediate anyway, because they enter the
Emacs input queue.  Which also means that if Emacs is busy doing
something, the buffer won't be reverted until Emacs is idle.

So I see no real advantages in your suggestion over what we already
have.



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Clarification about auto-revert-mode and inotify
  2014-10-21 19:36         ` Eli Zaretskii
@ 2014-10-21 19:39           ` Dima Kogan
  2014-10-21 20:09             ` Eli Zaretskii
  0 siblings, 1 reply; 28+ messages in thread
From: Dima Kogan @ 2014-10-21 19:39 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: monnier, emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Dima Kogan <dima@secretsauce.net>
>> Cc: monnier@IRO.UMontreal.CA, emacs-devel@gnu.org
>> Date: Tue, 21 Oct 2014 11:59:00 -0700
>> 
>> The issue I'm trying to solve is that there's a delay between when a
>> file is modified and when auto-revert-mode kicks in.
>
> A "delay" of, say, 0.01 seconds is indistinguishable from zero.

OK. Maybe I'm not understanding something then. If I set the delay in
the current code to 0.01 seconds, then isn't emacs polling its flag 100
times a second? I recognize that it's not looking at the file system
itself, but rather at the flag set by inotify, but this is still work
that does not need to be done.

If we have inotify telling us when files are updated, then we don't NEED
to poll anything. This is in fact the whole point of inotify.

The prototype patch I attached earlier reacts to inotify events as soon
as they happen, with some throttling, so if a file is updated every 10
seconds, say, then you'll get each update immediately with no polling.

If a file is updated every 0.1 seconds, however, then you'll get the
first update immediately, and the others with a period of 1 second. The
current code set to poll every 0.01 seconds would not only do stuff
every 0.01 seconds, but it'll also actually revert the buffer every 0.1
seconds, which is potentially inefficient.

The bulk of my patch is the throttling logic. If I take that out, then
the patch is one added line.



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Clarification about auto-revert-mode and inotify
  2014-10-21 19:39           ` Dima Kogan
@ 2014-10-21 20:09             ` Eli Zaretskii
  2014-10-21 20:29               ` Dima Kogan
  2014-10-22 10:13               ` Florian Weimer
  0 siblings, 2 replies; 28+ messages in thread
From: Eli Zaretskii @ 2014-10-21 20:09 UTC (permalink / raw)
  To: Dima Kogan; +Cc: monnier, emacs-devel

> From: Dima Kogan <dima@secretsauce.net>
> Cc: monnier@IRO.UMontreal.CA, emacs-devel@gnu.org
> Date: Tue, 21 Oct 2014 12:39:47 -0700
> 
> If I set the delay in the current code to 0.01 seconds, then isn't
> emacs polling its flag 100 times a second?

Yes.

> I recognize that it's not looking at the file system itself, but
> rather at the flag set by inotify, but this is still work that does
> not need to be done.
> 
> If we have inotify telling us when files are updated, then we don't NEED
> to poll anything. This is in fact the whole point of inotify.

What do you think happens when inotify sends its notification?  The
notification will not be known to Emacs until Emacs reads from the
file descriptor allocated for inotify's notifications.  And how does
Emacs know it should read from that descriptor?  It polls it, together
with the other descriptors.  This is how the Emacs input mechanism
works.  And timers, which are used to poll for buffers that need
reverting, are just an additional job Emacs does in the same loop
where it waits for input, including from inotify, to become ready.

So what exactly is the difference between these two alternatives?



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Clarification about auto-revert-mode and inotify
  2014-10-21 17:47       ` Eli Zaretskii
@ 2014-10-21 20:23         ` Stefan Monnier
  2014-10-21 20:32           ` Eli Zaretskii
  0 siblings, 1 reply; 28+ messages in thread
From: Stefan Monnier @ 2014-10-21 20:23 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: lists, emacs-devel

> Not sure I understand the question.  The timer runs every 0.01s and
> refreshes the buffers that are marked as being in need of reverting.
> But you already know that.

I meant, what's the effect in terms of efficiency, responsiveness, etc...


        Stefan



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Clarification about auto-revert-mode and inotify
  2014-10-21 20:09             ` Eli Zaretskii
@ 2014-10-21 20:29               ` Dima Kogan
  2014-10-22  2:42                 ` Eli Zaretskii
  2014-10-22 10:13               ` Florian Weimer
  1 sibling, 1 reply; 28+ messages in thread
From: Dima Kogan @ 2014-10-21 20:29 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: monnier, emacs-devel


Eli Zaretskii <eliz@gnu.org> writes:

>> If we have inotify telling us when files are updated, then we don't NEED
>> to poll anything. This is in fact the whole point of inotify.
>
> What do you think happens when inotify sends its notification?  The
> notification will not be known to Emacs until Emacs reads from the
> file descriptor allocated for inotify's notifications.  And how does
> Emacs know it should read from that descriptor?  It polls it, together
> with the other descriptors.

I'm using Linux. I just did an strace, and it does not poll anything.
The main loop uses select() to look at a number of file descriptors, the
inotify one being just one of them. If there's nothing to do, emacs just
sits there in the select and does no work. This is as it should be.

Are you saying there's no cost to using the current code with a very low
timeout? If so, why is the default 5 seconds and not 0.01 seconds? If
emacs really is already churning when idle, why not lower the default?



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Clarification about auto-revert-mode and inotify
  2014-10-21 20:23         ` Stefan Monnier
@ 2014-10-21 20:32           ` Eli Zaretskii
  2014-10-21 21:32             ` Stefan Monnier
  0 siblings, 1 reply; 28+ messages in thread
From: Eli Zaretskii @ 2014-10-21 20:32 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: lists, emacs-devel

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: lists@dima.secretsauce.net,  emacs-devel@gnu.org
> Date: Tue, 21 Oct 2014 16:23:18 -0400
> 
> > Not sure I understand the question.  The timer runs every 0.01s and
> > refreshes the buffers that are marked as being in need of reverting.
> > But you already know that.
> 
> I meant, what's the effect in terms of efficiency, responsiveness, etc...

It's a timer that just walks the buffer list.



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Clarification about auto-revert-mode and inotify
  2014-10-21 20:32           ` Eli Zaretskii
@ 2014-10-21 21:32             ` Stefan Monnier
  2014-10-22  2:47               ` Eli Zaretskii
  0 siblings, 1 reply; 28+ messages in thread
From: Stefan Monnier @ 2014-10-21 21:32 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: lists, emacs-devel

> It's a timer that just walks the buffer list.

To me running this code 100 times per second sounds very expensive
CPU-wise.

I think that with inotify support, we should be able to handle
efficiently (i.e. with prompt refresh and with almost no CPU use when no
files are modified) a situation where the user enabled
global-auto-revert-mode with a 100 file buffers.


        Stefan



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Clarification about auto-revert-mode and inotify
  2014-10-21 20:29               ` Dima Kogan
@ 2014-10-22  2:42                 ` Eli Zaretskii
  2014-10-22  5:36                   ` Per Starbäck
  0 siblings, 1 reply; 28+ messages in thread
From: Eli Zaretskii @ 2014-10-22  2:42 UTC (permalink / raw)
  To: Dima Kogan; +Cc: monnier, emacs-devel

> From: Dima Kogan <dima@secretsauce.net>
> Cc: monnier@IRO.UMontreal.CA, emacs-devel@gnu.org
> Date: Tue, 21 Oct 2014 13:29:03 -0700
> 
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> If we have inotify telling us when files are updated, then we don't NEED
> >> to poll anything. This is in fact the whole point of inotify.
> >
> > What do you think happens when inotify sends its notification?  The
> > notification will not be known to Emacs until Emacs reads from the
> > file descriptor allocated for inotify's notifications.  And how does
> > Emacs know it should read from that descriptor?  It polls it, together
> > with the other descriptors.
> 
> I'm using Linux. I just did an strace, and it does not poll anything.
> The main loop uses select() to look at a number of file descriptors, the
> inotify one being just one of them. If there's nothing to do, emacs just
> sits there in the select and does no work. This is as it should be.

Did you look at the code in Emacs that calls 'select'?  If not, please
do: that's where the important details are.

> Are you saying there's no cost to using the current code with a very low
> timeout?

Did you try it?  If not, please do, and measure the CPU load.

> If so, why is the default 5 seconds and not 0.01 seconds?

Because there's no real reason to revert buffers more frequently than
that.  But if you want, you can.



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Clarification about auto-revert-mode and inotify
  2014-10-21 21:32             ` Stefan Monnier
@ 2014-10-22  2:47               ` Eli Zaretskii
  2014-10-22  3:20                 ` Stefan Monnier
  0 siblings, 1 reply; 28+ messages in thread
From: Eli Zaretskii @ 2014-10-22  2:47 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: lists, emacs-devel

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: lists@dima.secretsauce.net,  emacs-devel@gnu.org
> Date: Tue, 21 Oct 2014 17:32:28 -0400
> 
> > It's a timer that just walks the buffer list.
> 
> To me running this code 100 times per second sounds very expensive
> CPU-wise.

Any measurements to support that?

Anyway, if 0.01 s is too expensive, you can always make it 0.2 s,
which is also "instantaneous" in human reaction time terms, but is 20
times less expensive CPU-wise.

> I think that with inotify support, we should be able to handle
> efficiently (i.e. with prompt refresh and with almost no CPU use when no
> files are modified) a situation where the user enabled
> global-auto-revert-mode with a 100 file buffers.

You do remember that we actually watch the directory of the file, not
the file itself, right?  And you do know how the inotify events are
processed by Emacs, right?



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Clarification about auto-revert-mode and inotify
  2014-10-22  2:47               ` Eli Zaretskii
@ 2014-10-22  3:20                 ` Stefan Monnier
  2014-10-22 14:47                   ` Eli Zaretskii
  0 siblings, 1 reply; 28+ messages in thread
From: Stefan Monnier @ 2014-10-22  3:20 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: lists, emacs-devel

>> To me running this code 100 times per second sounds very expensive
>> CPU-wise.
> Any measurements to support that?

That's about 15% CPU use on my i7 laptop on my current session.
That corresponds to increasing the power consumption from 6.7W to 8.3W.

> Anyway, if 0.01 s is too expensive, you can always make it 0.2 s,
> which is also "instantaneous" in human reaction time terms, but is 20
> times less expensive CPU-wise.

That's better, indeed.  The other problem I see with just increasing the
timer frequency is the effect it has on those files not covered by
inotify (e.g. remote files).

>> I think that with inotify support, we should be able to handle
>> efficiently (i.e. with prompt refresh and with almost no CPU use when no
>> files are modified) a situation where the user enabled
>> global-auto-revert-mode with a 100 file buffers.
> You do remember that we actually watch the directory of the file, not
> the file itself, right?  And you do know how the inotify events are
> processed by Emacs, right?

I have no idea why that would matter.


        Stefan



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Clarification about auto-revert-mode and inotify
  2014-10-22  2:42                 ` Eli Zaretskii
@ 2014-10-22  5:36                   ` Per Starbäck
  2014-10-22 14:51                     ` Eli Zaretskii
  0 siblings, 1 reply; 28+ messages in thread
From: Per Starbäck @ 2014-10-22  5:36 UTC (permalink / raw)
  To: emacs-devel@gnu.org

>> If so, why is the default 5 seconds and not 0.01 seconds?
>
> Because there's no real reason to revert buffers more frequently than
> that.  But if you want, you can.

When I first tested auto-revert-mode I assumed it was broken since
nothing happened.
The original poster wasn't sure if the current behaviour was a bug or
not. Emacs is not
fulfilling user expectations for an auto revert mode.

No real reason? I have instead hard to think of a reason why anyone
would like a five
second delay for anything you'd use auto revert mode for. I can
imagine that someone
sometimes would like a minimum time between updates, if there's reason to
see what happens, step by step, but have much harder to think of a reason why
anyone would like such a long delay before the first update. I don't
rule out that there
might be such a reason in some rare case, but still, if not for
performance issues,
the default should clearly be "immediate" (by human standards) update.



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Clarification about auto-revert-mode and inotify
  2014-10-21 20:09             ` Eli Zaretskii
  2014-10-21 20:29               ` Dima Kogan
@ 2014-10-22 10:13               ` Florian Weimer
  1 sibling, 0 replies; 28+ messages in thread
From: Florian Weimer @ 2014-10-22 10:13 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel, Dima Kogan, monnier

* Eli Zaretskii:

>> From: Dima Kogan <dima@secretsauce.net>
>> Cc: monnier@IRO.UMontreal.CA, emacs-devel@gnu.org
>> Date: Tue, 21 Oct 2014 12:39:47 -0700
>> 
>> If I set the delay in the current code to 0.01 seconds, then isn't
>> emacs polling its flag 100 times a second?
>
> Yes.

But that's not acceptable anymore due to VMs and power management
requirements.



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Clarification about auto-revert-mode and inotify
  2014-10-22  3:20                 ` Stefan Monnier
@ 2014-10-22 14:47                   ` Eli Zaretskii
  2014-10-22 17:09                     ` Stefan Monnier
  0 siblings, 1 reply; 28+ messages in thread
From: Eli Zaretskii @ 2014-10-22 14:47 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: lists, emacs-devel

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: lists@dima.secretsauce.net,  emacs-devel@gnu.org
> Date: Tue, 21 Oct 2014 23:20:59 -0400
> 
> > Anyway, if 0.01 s is too expensive, you can always make it 0.2 s,
> > which is also "instantaneous" in human reaction time terms, but is 20
> > times less expensive CPU-wise.
> 
> That's better, indeed.  The other problem I see with just increasing the
> timer frequency is the effect it has on those files not covered by
> inotify (e.g. remote files).

Maybe we should have 2 separate options, then (if we are going to
reduce the default time interval).

> >> I think that with inotify support, we should be able to handle
> >> efficiently (i.e. with prompt refresh and with almost no CPU use when no
> >> files are modified) a situation where the user enabled
> >> global-auto-revert-mode with a 100 file buffers.
> > You do remember that we actually watch the directory of the file, not
> > the file itself, right?  And you do know how the inotify events are
> > processed by Emacs, right?
> 
> I have no idea why that would matter.

You are worried about scalability, and present requirements for it.
My point is that scalability is not affected only by the number of
buffers that have auto-revert turned on.  In a busy directory, we will
have to process many notifications that eventually turn out to be for
files we don't care about.  Processing each notification requires
looping through all the buffers, to find those which have auto-revert
turned on.  (We do cut short the search when we find the first client
buffer for a particular notification, but that doesn't help with
notifications for which there is no client buffer.)  So processing
notifications on a busy system might itself present scalability
problems and uses up a significant amount of CPU cycles, and I'm not
at all sure the goal of "prompt refresh and almost no CPU use" is
reachable in a way that is scalable to busy directories.

And what about the scalability of the change proposed by Dima?  It
causes each auto-reverted buffer to have its own timer, which would
mean dozens of timers running to support 100 buffers.  Wouldn't that
slow down Emacs's input loop?

And, of course, the revert operation itself also uses up CPU.  The
relatively long interval we use now in effect collapses all the
modifications to a file during the last 5 sec into a single revert.
Reverting "immediately" will revert on each notification, which could
be quite a lot.  How will that affect scalability?

In addition, the proposed way of throttling down the revert rate only
makes sense to me for files that change infrequently, because
frequently modified files will have all but the first modification
delayed.  That doesn't fit the "tail -f" use case, for example.
Sounds like the proposed change is targeting a very narrow niche of
use cases.

Bottom line, if scalability is a requirement, we need to have a
broader picture, and then consider the possible solutions based on
notifications in light of that.



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Clarification about auto-revert-mode and inotify
  2014-10-22  5:36                   ` Per Starbäck
@ 2014-10-22 14:51                     ` Eli Zaretskii
  2014-10-22 19:08                       ` Per Starbäck
  0 siblings, 1 reply; 28+ messages in thread
From: Eli Zaretskii @ 2014-10-22 14:51 UTC (permalink / raw)
  To: Per Starbäck; +Cc: emacs-devel

> Date: Wed, 22 Oct 2014 07:36:26 +0200
> From: Per Starbäck <per.starback@gmail.com>
> 
> When I first tested auto-revert-mode I assumed it was broken since
> nothing happened.  The original poster wasn't sure if the current
> behaviour was a bug or not. Emacs is not fulfilling user
> expectations for an auto revert mode.

I could agree with that (cannot really argue with expectations of
others, nor do I think we should), but if so, this situation exists
for a long time, because this default didn't change since 1997.  Maybe
it's time to change it, but that's a different discussion.

This thread is about the change in the latest Emacs, which adds
support for file notifications, and whether we should use that to make
"immediate" the default.  The relation to the time interval used to
check for buffers that need to be auto-reverted is only tangential,
mainly because I suggested to use that as the way to make the reaction
time shorter.

> No real reason? I have instead hard to think of a reason why anyone
> would like a five second delay for anything you'd use auto revert
> mode for. I can imagine that someone sometimes would like a minimum
> time between updates, if there's reason to see what happens, step by
> step, but have much harder to think of a reason why anyone would
> like such a long delay before the first update. I don't rule out
> that there might be such a reason in some rare case, but still, if
> not for performance issues, the default should clearly be
> "immediate" (by human standards) update.

I think we need to consider the broader picture here.  It is possible
that you have in mind a situation where only one buffer, or a small
number of buffers, are under auto-revert.  But what if there are
dozens of them?  And what about remote files, whose access is
expensive/slow?

So I'm not sure the decision is so clear here.  However, if you are
arguing for reducing the default time interval, I'm not opposed to
that.  All I said initially was that this customizable option is IMO
enough to give users "immediate" reverts if they want that.




^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Clarification about auto-revert-mode and inotify
  2014-10-22 14:47                   ` Eli Zaretskii
@ 2014-10-22 17:09                     ` Stefan Monnier
  2014-10-24  7:02                       ` Dima Kogan
  0 siblings, 1 reply; 28+ messages in thread
From: Stefan Monnier @ 2014-10-22 17:09 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: lists, emacs-devel

>> That's better, indeed.  The other problem I see with just increasing the
>> timer frequency is the effect it has on those files not covered by
>> inotify (e.g. remote files).
> Maybe we should have 2 separate options, then (if we are going to
> reduce the default time interval).

Maybe we should yes.

> My point is that scalability is not affected only by the number of
> buffers that have auto-revert turned on.  In a busy directory, we will
> have to process many notifications that eventually turn out to be for
> files we don't care about.  Processing each notification requires
> looping through all the buffers, to find those which have auto-revert
> turned on.

No, we use a hash table to associate buffers to particular
notify-descriptors so we only go through the buffers that do have
auto-revert enabled and that can be affected by changes in this
directory.  That can still be a lot of buffers, but is still much better
and that loop can be improved (e.g. hoisting the file-name-nondirectory
out of the loop).

> (We do cut short the search when we find the first client
> buffer for a particular notification, but that doesn't help with
> notifications for which there is no client buffer.)

Yes, I don't think this short cut makes any difference.

> And what about the scalability of the change proposed by Dima?  It
> causes each auto-reverted buffer to have its own timer, which would
> mean dozens of timers running to support 100 buffers.  Wouldn't that
> slow down Emacs's input loop?

Oh, yes, a timer per buffer would be a bad idea.

> And, of course, the revert operation itself also uses up CPU.  The
> relatively long interval we use now in effect collapses all the
> modifications to a file during the last 5 sec into a single revert.
> Reverting "immediately" will revert on each notification, which could
> be quite a lot.  How will that affect scalability?

That is indeed a good reason to throttle updates.

> Bottom line, if scalability is a requirement, we need to have a
> broader picture, and then consider the possible solutions based on
> notifications in light of that.

To me the main goals should be:
- *very* low resource use when the system is idle.
- prompt updates after a "one time" change.
- keep Emacs responsive even in case of constant updates.


        Stefan



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Clarification about auto-revert-mode and inotify
  2014-10-22 14:51                     ` Eli Zaretskii
@ 2014-10-22 19:08                       ` Per Starbäck
  2014-10-22 19:35                         ` Eli Zaretskii
  0 siblings, 1 reply; 28+ messages in thread
From: Per Starbäck @ 2014-10-22 19:08 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel@gnu.org

> I think we need to consider the broader picture here.  It is possible
> that you have in mind a situation where only one buffer, or a small
> number of buffers, are under auto-revert.  But what if there are
> dozens of them?  And what about remote files, whose access is
> expensive/slow?

I have no idea about the performance issues are. I mostly reacted to you saying
that there's no point in having "immediate" updates. For me it's
obvious that that's
what we would like. These five seconds can really be five seconds of
waiting for a user.
If dired auto-updates when new files are added I think it often will
be typical behavior
to start waiting for a reaction from Emacs immediately after you
create a file in some other way,
as an example.

Can there be a point in changing behaviour for buffers that are
displayed in some window?
(Or would it be too much overhead?)



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Clarification about auto-revert-mode and inotify
  2014-10-22 19:08                       ` Per Starbäck
@ 2014-10-22 19:35                         ` Eli Zaretskii
  0 siblings, 0 replies; 28+ messages in thread
From: Eli Zaretskii @ 2014-10-22 19:35 UTC (permalink / raw)
  To: Per Starbäck; +Cc: emacs-devel

> Date: Wed, 22 Oct 2014 21:08:37 +0200
> From: Per Starbäck <per.starback@gmail.com>
> Cc: "emacs-devel@gnu.org" <emacs-devel@gnu.org>
> 
> Can there be a point in changing behaviour for buffers that are
> displayed in some window?
> (Or would it be too much overhead?)

Could be a good optional feature, I think.




^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Clarification about auto-revert-mode and inotify
  2014-10-22 17:09                     ` Stefan Monnier
@ 2014-10-24  7:02                       ` Dima Kogan
  0 siblings, 0 replies; 28+ messages in thread
From: Dima Kogan @ 2014-10-24  7:02 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Eli Zaretskii, emacs-devel

[-- Attachment #1: Type: text/plain, Size: 2021 bytes --]

Hi.

This thread really lost its focus, so let me try to bring it back. My
goals with this thread and patch are what Stefan listed, with some extra
points:

- *very* low resource use when the system is idle.
- prompt updates after a "one time" change FOR LOCAL FILES
- keep Emacs responsive even in case of constant updates or many
  auto-revertable buffers

I'm making an assumption that it would be good to have this in stock
emacs, and not in some odd configuration many of us would never use,
like checking an event-driven flag 100 times every second.

I do agree that making things universally as responsive as a 'tail -f'
would take more work, mainly because each revert can be a heavy
operation. Right now I just want to take care of the low-hanging fruit,
so the above list is good.

Notifications (inotify, etc) are a good way to make this work, so I only
care about the case where notifications are available: local files only.

The patch I attached mostly achieves the goals outlined above, with the
exception of the cost incurred by the per-buffer timers. I'm attaching
another verion of this patch that does not use any timers at all. It
keeps a counter, incrementing during each 'auto-revert-buffers' call.
When an file notification fires, the buffer is updated immediately, and
the current value of the counter is saved. Net time an notification
fires, the immediate update happens ONLY if the counter is different
from the value stored. So there's one global variable (the counter), and
a per-buffer variable for each buffer of interest (the stored counter
value).

If there are specific objections to this patch, performance or
otherwise, please tell me.

Also, it appears that both gfile and inotify notification systems are
misbehaving in emacs right now, so serious tests of this patch probably
wouldn't yield good results yet. Conceptual criticism probably would
have to do for now (this wasn't a problem the last time :) ).

I'm looking at the notification issues now; more on that in a bit.

dima


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-auto-revert-mode-can-now-revert-immediately-in-respo.patch --]
[-- Type: text/x-diff, Size: 3665 bytes --]

From 567d8469dfb143786890c65de58bf2ce887e9ebd Mon Sep 17 00:00:00 2001
From: Dima Kogan <dima@secretsauce.net>
Date: Fri, 24 Oct 2014 19:44:43 -0700
Subject: [PATCH] auto-revert-mode can now revert immediately in response to a
 change event

If we have file notifications, we want to update the auto-revert buffers
immediately when a notification occurs. Since file updates can happen very
often, we want to skip some revert operations so that we don't spend all our
time reverting the buffer.

We do this by reverting immediately in response to the first in a flurry of
notifications. We suppress subsequent notifications until the next time
`auto-revert-buffers' is called (this happens on a timer with a period set by
`auto-revert-interval').
---
 lisp/autorevert.el | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/lisp/autorevert.el b/lisp/autorevert.el
index f1074e2..7de6ec1 100644
--- a/lisp/autorevert.el
+++ b/lisp/autorevert.el
@@ -531,6 +531,34 @@ will use an up-to-date value of `auto-revert-interval'"
       ;; Fallback to file checks.
       (set (make-local-variable 'auto-revert-use-notify) nil))))
 
+
+
+;; If we have file notifications, we want to update the auto-revert buffers
+;; immediately when a notification occurs. Since file updates can happen very
+;; often, we want to skip some revert operations so that we don't spend all our
+;; time reverting the buffer.
+;;
+;; We do this by reverting immediately in response to the first in a flurry of
+;; notifications. We suppress subsequent notifications until the next time
+;; `auto-revert-buffers' is called (this happens on a timer with a period set by
+;; `auto-revert-interval').
+(defvar auto-revert-buffers-counter 1
+  "Incremented each time `auto-revert-buffers' is called")
+(defvar auto-revert-buffers-counter-lockedout 0
+  "Buffer-local value to indicate whether we should immediately
+update the buffer on a notification event or not. If
+
+  (= auto-revert-buffers-counter-lockedout
+     auto-revert-buffers-counter)
+
+then the updates are locked out, and we wait until the next call
+of `auto-revert-buffers' to revert the buffer. If no lockout is
+present, then we revert immediately and set the lockout, so that
+no more reverts are possible until the next call of
+`auto-revert-buffers'")
+(make-variable-buffer-local 'auto-revert-buffers-counter-lockedout)
+
+
 (defun auto-revert-notify-handler (event)
   "Handle an EVENT returned from file notification."
   (with-demoted-errors
@@ -566,6 +594,14 @@ will use an up-to-date value of `auto-revert-interval'"
                                 (file-name-nondirectory buffer-file-name)))))
                 ;; Mark buffer modified.
                 (setq auto-revert-notify-modified-p t)
+
+                ;; Revert the buffer now if we're not locked out
+                (when (/= auto-revert-buffers-counter-lockedout
+                          auto-revert-buffers-counter)
+                  (auto-revert-handler)
+                  (setq auto-revert-buffers-counter-lockedout
+                        auto-revert-buffers-counter))
+
                 ;; No need to check other buffers.
                 (cl-return)))))))))
 
@@ -686,6 +722,10 @@ are checked first the next time this function is called.
 This function is also responsible for removing buffers no longer in
 Auto-Revert mode from `auto-revert-buffer-list', and for canceling
 the timer when no buffers need to be checked."
+
+  (setq auto-revert-buffers-counter
+        (1+ auto-revert-buffers-counter))
+
   (save-match-data
     (let ((bufs (if global-auto-revert-mode
 		    (buffer-list)
-- 
2.0.0


^ permalink raw reply related	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2014-10-24  7:02 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-21  7:00 Clarification about auto-revert-mode and inotify Dima Kogan
2014-10-21 13:43 ` Stefan Monnier
2014-10-21 15:08   ` Eli Zaretskii
2014-10-21 16:28     ` Stefan Monnier
2014-10-21 17:47       ` Eli Zaretskii
2014-10-21 20:23         ` Stefan Monnier
2014-10-21 20:32           ` Eli Zaretskii
2014-10-21 21:32             ` Stefan Monnier
2014-10-22  2:47               ` Eli Zaretskii
2014-10-22  3:20                 ` Stefan Monnier
2014-10-22 14:47                   ` Eli Zaretskii
2014-10-22 17:09                     ` Stefan Monnier
2014-10-24  7:02                       ` Dima Kogan
2014-10-21 17:59   ` Dima Kogan
2014-10-21 18:34     ` Eli Zaretskii
2014-10-21 18:59       ` Dima Kogan
2014-10-21 19:36         ` Eli Zaretskii
2014-10-21 19:39           ` Dima Kogan
2014-10-21 20:09             ` Eli Zaretskii
2014-10-21 20:29               ` Dima Kogan
2014-10-22  2:42                 ` Eli Zaretskii
2014-10-22  5:36                   ` Per Starbäck
2014-10-22 14:51                     ` Eli Zaretskii
2014-10-22 19:08                       ` Per Starbäck
2014-10-22 19:35                         ` Eli Zaretskii
2014-10-22 10:13               ` Florian Weimer
  -- strict thread matches above, loose matches on Subject: below --
2014-10-21  6:01 Dima Kogan
2014-10-21  5:03 Dima Kogan

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).