Re: A whole lotta auto-saving going

[Stefan Monnier <monnier@iro.umontreal.ca>]

>> But the `do_display` argument indicates that if redisplay is needed it
>> can happen without returning from `wait_reading_process_output`.
> Do we get to where that causes redisplay in this case?

I don't know, but we should because the code does pass a true value for
this parameter.

>> > I think a simple solution to this would be to check the time passed
>> > after sit_for returns, and if some of the wait time is left, not call
>> > auto-save.  This would mimic what happened before the offending
>> > changeset.
>> The patch below implements that option.
> Thanks.
> I see you look at the number of bytes read, but what about the case
> that a process exited?

I don't know anything about that case, sorry (i.e. I don't know if it
causes `wait_reading_process_output` to exit early).  So ... let me test it.
Hmm... interesting.  I used the current `master` code with the following
base test case:

    src/emacs -Q --eval '(start-process "toto" "*scratch*" "sh" "-c" "while sleep 1; do echo hi; done")' ~/tmp/foo.c

This triggers the problem where auto-save is done pretty much after
every keystroke (with ~1s delay).  But if I change that to:

    src/emacs -Q --eval '(add-hook `post-command-hook (lambda () (start-process "toto" "*scratch*" "sh" "-c" "sleep 1; echo hi")))' ~/tmp/foo.c

then the problem doesn't seem to occur any more (or rather it still
does, but more rarely, with a pattern I have trouble discerning).
It does occur, OTOH with:

    src/emacs -Q --eval '(add-hook `post-command-hook (lambda () (start-process "toto" "*scratch*" "sh" "-c" "sleep 1; echo hi; sleep 1")))' ~/tmp/foo.c

So it looks like running sentinels does not trigger this problem (it
may even avoid it if the sentinel is run "at the same time" as we
receive output).

With the patch applied, none of the above test cases exhibit the problem
of "A whole lotta auto-saving going".

>> There's one other call to `sit_for` which can be affected:
>>
>> 	  tem0 = sit_for (Vecho_keystrokes, 1, 1);
>> 	  unbind_to (count, Qnil);
>> 	  if (EQ (tem0, Qt)
>> 	      && ! CONSP (Vunread_command_events))
>> 	    echo_now ();
>>
>> I believe it's an improvement there as well.
>
> Why do you believe that?

Because I remember noticing that the echo-keystrokes output sometimes
was displayed earlier than expected (and that it seemed related to
process output).

> It's a different use case, and I don't think we saw any adverse
> effects there from the removal of the buffer-switch "event".
> Are there any adverse effects?

The fact that echo-keystrokes were displayed earlier than expected was
not a serious issue, so I didn't bother to report it.  But I do think
it's an adverse effect.

This said, the current patch has the inverse effect (just like the old
code): auto-save (and echo-keystrokes) can be delayed (potentially
indefinitely) by process output since the `sit_for` call will only
return nil if we waited until timeout without being interrupted by any
process output.  That's why I think in the long run it would be good to
change `sit_for` so it actually does wait the specified time even when
process output comes in (just like The Elisp `sit-for` function does).


        Stefan