GNU bug report logs -
#15045
Point jumps inappropriately around time of Semantic lexing
Previous Next
To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 15045 in the body.
You can then email your comments to 15045 AT debbugs.gnu.org in the normal way.
Toggle the display of automated, internal messages from the tracker.
Report forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Wed, 07 Aug 2013 18:00:03 GMT)
Full text and
rfc822 format available.
Acknowledgement sent
to
Barry OReilly <gundaetiapo <at> gmail.com>:
New bug report received and forwarded. Copy sent to
bug-gnu-emacs <at> gnu.org.
(Wed, 07 Aug 2013 18:00:04 GMT)
Full text and
rfc822 format available.
Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
See this thread for the original description:
http://lists.gnu.org/archive/html/emacs-devel/2013-07/msg00328.html
The short of it is that I occasionally witness point jump to elsewhere
in the buffer while I'm editing and then back again. The change of
point is visually apparent in the display. This has consequences such
as causing undesired scrolling while editing.
To debug it, I used a change like this:
diff --git a/src/editfns.c b/src/editfns.c
index 50bde90..039e13f 100644
--- a/src/editfns.c
+++ b/src/editfns.c
@@ -234,6 +234,8 @@ The return value is POSITION. */)
(register Lisp_Object position)
{
ptrdiff_t pos;
+ bool noninteractive_old = noninteractive;
+ noninteractive = true;
if (MARKERP (position)
&& current_buffer == XMARKER (position)->buffer)
@@ -245,7 +247,19 @@ The return value is POSITION. */)
SET_PT_BOTH (ZV, ZV_BYTE);
else
SET_PT_BOTH (pos, marker_byte_position (position));
-
+ int btI = 0;
+ Lisp_Object theBacktrace;
+ do
+ {
+ theBacktrace = Fbacktrace_frame( make_number(btI) );
+ ++btI;
+ Fprin1(theBacktrace, Qnil);
+ printf("\017\n");
+ } while( ! EQ(theBacktrace, Qnil) );
+ { struct timespec debug_ts; char debug_dateStr[20]; {
clock_gettime(CLOCK_REALTIME, &debug_ts); struct tm mytm;
localtime_r(&debug_ts.tv_sec, &mytm); strftime(debug_dateStr, 20,
"%Y-%m-%dT%H:%M:%S", &mytm); }
+ printf( "%s.%09ld|pid:%d|tid:%ld|%s|%d| DEBUG: goto-char
marker pos=%ld\n", // TODO: debugging
+ debug_dateStr, debug_ts.tv_nsec,
getpid(), pthread_self(), __FILE__, __LINE__, pos ); fflush(stdout); }
+ noninteractive = noninteractive_old;
return position;
}
@@ -253,6 +267,19 @@ The return value is POSITION. */)
pos = clip_to_bounds (BEGV, XINT (position), ZV);
SET_PT (pos);
+ int btI = 0;
+ Lisp_Object theBacktrace;
+ do
+ {
+ theBacktrace = Fbacktrace_frame( make_number(btI) );
+ ++btI;
+ Fprin1(theBacktrace, Qnil);
+ printf("\017\n");
+ } while( ! EQ(theBacktrace, Qnil) );
+ { struct timespec debug_ts; char debug_dateStr[20]; {
clock_gettime(CLOCK_REALTIME, &debug_ts); struct tm mytm;
localtime_r(&debug_ts.tv_sec, &mytm); strftime(debug_dateStr, 20,
"%Y-%m-%dT%H:%M:%S", &mytm); }
+ printf( "%s.%09ld|pid:%d|tid:%ld|%s|%d| DEBUG: goto-char
position pos=%ld\n", // TODO: debugging
+ debug_dateStr, debug_ts.tv_nsec, getpid(),
pthread_self(), __FILE__, __LINE__, pos ); fflush(stdout); }
+ noninteractive = noninteractive_old;
return position;
}
I got a reproduction with these debug statements. I was editing a
Makefile in comments at about buffer position 3396 and observed point
temporarily jump to the beginning of the comment block at position
3084. I filtered the output for calls to go to position 3084 around
the time I witnessed this:
[backtrace A]
2013-08-01T10:37:28.119778000|pid:10485|tid:2342111488|editfns.c|281|
DEBUG: goto-char position pos=3084
[backtrace B]
2013-08-01T10:37:28.412962000|pid:10485|tid:2342111488|editfns.c|261|
DEBUG: goto-char marker pos=3084
[backtrace C]
2013-08-01T10:37:29.715413000|pid:10485|tid:2342111488|editfns.c|281|
DEBUG: goto-char position pos=3084
Strangly, the backtrace A and B are 35 and 45 empty (except my Ctrl-O
chars) stack frames respectively. Backtrace C is:
(t semantic-make-lexer 3083 3257 nil nil)^O
(t semantic-lex 3083 3257 nil)^O
(t semantic-parse-region-default 3083 3257 nil nil nil)^O
(t semantic-parse-region 3083 3257 nil)^O
(t semantic-edits-incremental-parser-1)^O
parser error: %S" error-message-string t] 4)))] 3)^O
(t semantic-parse-changes-default)^O
(t semantic-parse-changes)^O
(t semantic-fetch-tags)^O
(t byte-code "<U+008A><U+008A>À <U+0088>*Á<U+0087>"
[semantic-fetch-tags nil] 1)^O
(t semantic-idle-scheduler-refresh-tags)^O
(t byte-code "Æ^XÇpÇÆÈÉÊ
\"\"\"^YÆ<U+0089>^ZESCÆ^\^M;<U+0085>^\^@Ë^M!^^#^N$<U+0085>^^@ÌÍ!<U+0085>+^@^N^M?<U+0085>^^@^N%?<U+0085>^^@^N#<U+0084>E^@^M;<U+0083>E^@Î^M!<U+0084>R^@^N#<U+0085>^^@Ë^MÆÏ#<U+0085>^^@^N&ÐX<U+0086>^
^M;<U+0085>^@Ë^M!^^#^N$<U+0085>ï^@ÌÍ!<U+0085>¼^@^N^M?<U+0085>ï^@^N%?<U+0085>ï^@^N#<U+0084>Ö^@^M;<U+0083>Ö^@Î^M!<U+0084>ã^@^N#<U+0085>ï^@Ë^MÆÏ#<U+0085>ï^@^N&ÐX<U+0086>ï^@Ñ
^N&W)<U+0083>
^A^N(<U+0083>þ^@Ò <U+0088><U+0082>
^AÓÚÛ<U+008F><U+0088><U+0082>
^Ap^KB^S)^N*A<U+0089>^V*<U+0084>^?^@*^K^Q)^N,Æ^^-<U+0089>^^*<U+0083>o^A^N*@^V-<U+008A>^N+<U+0083>>^AÖ
<U+0084>8^A× <U+0083>>^AØ^N+Ü\"<U+0088>^N.<U+0083>I^AÝÞ^N-\"<U+0088>^N(<U+0083>U^A^N-
<U+0088><U+0082>Z^AÓßà<U+008F>
e-scheduler-queue service semantic-idle-scheduler-verbose-flag] 8)^O
(t semantic-idle-core-handler)^O
(t semantic-idle-scheduler-function)^O
(t apply semantic-idle-scheduler-function nil)^O
(t byte-code "rÂÃH\"<U+0088>)Á<U+0087>" [timer apply 5 6] 4)^O
(t timer-event-handler [t 0 1 0 t semantic-idle-scheduler-function nil
idle 0])^O
nil^O
I can see the call to goto-char in define-lex (the macro which creates
semantic-make-lexer). However, there is a save-excursion in effect at
the semantic-idle-core-handler frame, so this goto-char wouldn't seem
to be the same goto-char I observe in the display.
I'm not sure about the empty backtraces. Is the code I used to print
backtraces valid? In my many runs, empty backtraces are very rare.
I have since started using Fbacktrace() instead of the more long
winded code above. Unfortunately I'm having a reproduction drought in
the past week.
One additional observation not previously noted is that I see this bug
much more often when editing comments or strings. However, I'm fairly
sure I've seen it when editing straight code too.
In GNU Emacs 24.3.50.1 (x86_64-unknown-linux-gnu, GTK+ Version 2.10.4)
of 2013-06-18 on psd15
Windowing system distributor `The X.Org Foundation', version 11.0.70101000
System Description: Red Hat Enterprise Linux Client release 5.4 (Tikanga)
Configured using:
`configure
--prefix=/redacted/user/boreilly/sw/emacs-install-trunk-20899d085afe62520113b5acbfe3dbba57823dc9
--with-gif=no'
Important settings:
value of $LANG: en_US.UTF-8
value of $XMODIFIERS: @im=none
locale-coding-system: utf-8-unix
default enable-multibyte-characters: t
Major mode: Lisp Interaction
Minor modes in effect:
outline-minor-mode: t
global-whitespace-mode: t
global-ede-mode: t
global-semanticdb-minor-mode: t
global-semantic-idle-scheduler-mode: t
semantic-mode: t
evil-mode: t
evil-local-mode: t
global-undo-tree-mode: t
undo-tree-mode: t
show-paren-mode: t
delete-selection-mode: t
global-auto-revert-mode: t
tooltip-mode: t
mouse-wheel-mode: t
menu-bar-mode: t
file-name-shadow-mode: t
global-font-lock-mode: t
font-lock-mode: t
blink-cursor-mode: t
auto-composition-mode: t
auto-encryption-mode: t
auto-compression-mode: t
column-number-mode: t
line-number-mode: t
transient-mark-mode: t
Recent input:
M-x r e p o r t - <tab> <return>
Recent messages:
Loading redacted
Loading redacted
Loading whitespace...done
2013-08-07T13:47:52.565183 Loading tags file: redacted
Starting a new list of tags tables
2013-08-07T13:47:52.716876 Finished loading init file.
2013-08-07T13:47:52.723671 Inside my-prog-mode-hook
2013-08-07T13:47:52.732424 Inside my-emacs-lisp-mode-hook for buffer *scratch*
For information about GNU Emacs and the GNU system, type C-h C-a.
2013-08-07T13:47:52.748012 ---------------- Finished with
my-emacs-startup-hook. ----------------
Load-path shadows:
None found.
Features:
(shadow sort gnus-util mail-extr emacsbug message format-spec rfc822 mml
mml-sec mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev
gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums mm-util
mail-prsvr mail-utils noutline outline easy-mmode my-config warnings
semantic/lex-spp etags package cl-macs whitespace cus-start cus-load
my-proj ede/cpp-root ede/speedbar ede/files ede ede/base ede/auto
ede/source eieio-speedbar speedbar sb-image dframe eieio-custom wid-edit
semantic/db-mode semantic/idle semantic/bovine/gcc semantic/dep
semantic/ia semantic/analyze/refs semantic/db-find semantic/db-ref
semantic/senator semantic/decorate pulse semantic/analyze semantic/sort
semantic/scope semantic/analyze/fcn semantic/db gv eieio-base
semantic/ctxt semantic/format ezimage semantic/tag-ls semantic/find
semantic/util-modes easymenu semantic/util semantic semantic/tag
semantic/lex semantic/fw eieio byte-opt bytecomp byte-compile cconv
eieio-core mode-local cedet evil evil-integration evil-maps
evil-commands evil-types evil-search evil-ex evil-macros evil-repeat
evil-states evil-core evil-common windmove rect evil-digraphs evil-vars
ring edmacro kmacro undo-tree diff goto-chg rainbow-delimiters my-util
advice help-fns electric paren delsel autorevert cl nadvice cl-lib
time-date tooltip ediff-hook vc-hooks lisp-float-type mwheel x-win x-dnd
tool-bar dnd fontset image regexp-opt fringe tabulated-list newcomment
lisp-mode register page menu-bar rfn-eshadow timer select scroll-bar
mouse jit-lock font-lock syntax facemenu font-core frame cham georgian
utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean
japanese hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese case-table epa-hook jka-cmpr-hook help simple abbrev
minibuffer loaddefs button faces cus-face macroexp files text-properties
overlay sha1 md5 base64 format env code-pages mule custom widget
hashtable-print-readable backquote make-network-process dbusbind
dynamic-setting system-font-setting font-render-setting move-toolbar gtk
x-toolkit x multi-tty emacs)
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Wed, 07 Aug 2013 18:31:01 GMT)
Full text and
rfc822 format available.
Message #8 received at 15045 <at> debbugs.gnu.org (full text, mbox):
> The short of it is that I occasionally witness point jump to elsewhere
> in the buffer while I'm editing and then back again.
So it's "displayed at A" then "displayed at B" then "displayed at
A again"? what happens between each one of those 3 displays?
If "nothing", then I suspect there's something like a `sit-for'
somewhere that causes a redisplay in the middle of the command (i.e. in
the middle of a save-excursion).
Stefan
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Wed, 07 Aug 2013 18:43:02 GMT)
Full text and
rfc822 format available.
Message #11 received at 15045 <at> debbugs.gnu.org (full text, mbox):
Stefan Monnier writes:
>> The short of it is that I occasionally witness point jump to elsewhere
>> in the buffer while I'm editing and then back again.
>
> So it's "displayed at A" then "displayed at B" then "displayed at
> A again"? what happens between each one of those 3 displays?
>
> If "nothing", then I suspect there's something like a `sit-for'
> somewhere that causes a redisplay in the middle of the command (i.e. in
> the middle of a save-excursion).
I sometimes see this as well, and yes, "nothing" happens between those
three displays. I also think there's a redisplay triggered by another
background task while Semantic does its idle parsing stuff. I surely
won't rule out that we're missing a save-excursion somewhere in
Semantic, but I think that if this were the case, it should happen much
more often.
I wonder which background tasks could trigger such redisplays. Maybe an
update of the mode-line? How could I debug this? The problem is that
this happens maybe every 15 minutes or so...
-David
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Wed, 07 Aug 2013 19:32:02 GMT)
Full text and
rfc822 format available.
Message #14 received at 15045 <at> debbugs.gnu.org (full text, mbox):
> So it's "displayed at A" then "displayed at B" then "displayed at A
> again"? what happens between each one of those 3 displays?
No. A, B, and C are backtraces. Specifically, the backtraces printed
when goto-char went to the offending position at the beginning of the
comment block. I don't know which of the three is the one I see
actually displayed. The timestamps are too close together for me to be
certain.
I'm interested to see if I get more empty backtraces at the next
reproduction. I don't know what they could mean.
> If "nothing", then I suspect there's something like a `sit-for'
> somewhere that causes a redisplay in the middle of the command (i.e.
> in the middle of a save-excursion).
Ahh, I didn't know about sit-for. It might explain what I'm seeing.
I added a debug statement to Fredisplay. In a simple run (no
reproduction of the bug yet), I get a few of these:
2013-08-07T15:18:09.576922000|pid:11494|tid:47776720943360|dispnew.c|5822|
DEBUG: redisplay
redisplay()
sit-for(0)
jit-lock-deferred-fontify()
apply(jit-lock-deferred-fontify nil)
byte-code("rÁÂHÃH\")Á" [timer apply 5 6] 4)
timer-event-handler([t 0 0 10000 t jit-lock-deferred-fontify nil idle 0])
I don't suppose there are ways deferred jit lock would execute during
Semantic lexing?
Anyway, next time I see the bug, I expect I'll get better data.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Wed, 07 Aug 2013 19:45:02 GMT)
Full text and
rfc822 format available.
Message #17 received at 15045 <at> debbugs.gnu.org (full text, mbox):
> Date: Wed, 7 Aug 2013 15:31:38 -0400
> From: Barry OReilly <gundaetiapo <at> gmail.com>
> Cc: 15045 <at> debbugs.gnu.org
>
> > So it's "displayed at A" then "displayed at B" then "displayed at A
> > again"? what happens between each one of those 3 displays?
>
> No. A, B, and C are backtraces. Specifically, the backtraces printed
> when goto-char went to the offending position at the beginning of the
> comment block. I don't know which of the three is the one I see
> actually displayed. The timestamps are too close together for me to be
> certain.
>
> I'm interested to see if I get more empty backtraces at the next
> reproduction. I don't know what they could mean.
>
> > If "nothing", then I suspect there's something like a `sit-for'
> > somewhere that causes a redisplay in the middle of the command (i.e.
> > in the middle of a save-excursion).
>
> Ahh, I didn't know about sit-for. It might explain what I'm seeing.
>
> I added a debug statement to Fredisplay. In a simple run (no
> reproduction of the bug yet), I get a few of these:
>
> 2013-08-07T15:18:09.576922000|pid:11494|tid:47776720943360|dispnew.c|5822|
> DEBUG: redisplay
> redisplay()
> sit-for(0)
> jit-lock-deferred-fontify()
> apply(jit-lock-deferred-fontify nil)
> byte-code("rÁÂHÃH\")Á" [timer apply 5 6] 4)
> timer-event-handler([t 0 0 10000 t jit-lock-deferred-fontify nil idle 0])
>
> I don't suppose there are ways deferred jit lock would execute during
> Semantic lexing?
I think the jit-lock you see is the result of the scrolling, not its
reason.
> Anyway, next time I see the bug, I expect I'll get better data.
In the discussion that you cite at the beginning of this bug report, I
suggested a different approach to finding out the culprit, by using
the redisplay tracing facility. Can you try that and post the
results? It might give better clues.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Wed, 07 Aug 2013 20:40:02 GMT)
Full text and
rfc822 format available.
Message #20 received at 15045 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
> In the discussion that you cite at the beginning of this bug report,
> I suggested a different approach to finding out the culprit, by
> using the redisplay tracing facility. Can you try that and post the
> results? It might give better clues.
Though I can't reproduce it at will, I do have a trace-redisplay from
when I saw the bug in a Java file. Specifically, I saw point move
inappropriately, but redisplay didn't need to scroll. It's attached in
case you find it useful.
[trace-redisplay-pt-move.txt (text/plain, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Wed, 07 Aug 2013 21:24:01 GMT)
Full text and
rfc822 format available.
Message #23 received at 15045 <at> debbugs.gnu.org (full text, mbox):
>> So it's "displayed at A" then "displayed at B" then "displayed at A
>> again"? what happens between each one of those 3 displays?
> No. A, B, and C are backtraces.
A, B, and C are local variables whose meaning depends on the email in
which they're used. Yours have nothing to do with mine. Sorry I reused
the same identifiers in the same thread.
> 2013-08-07T15:18:09.576922000|pid:11494|tid:47776720943360|dispnew.c|5822|
> DEBUG: redisplay
> redisplay()
> sit-for(0)
> jit-lock-deferred-fontify()
> apply(jit-lock-deferred-fontify nil)
> byte-code("rÁÂHÃH\"ˆ)Á‡" [timer apply 5 6] 4)
> timer-event-handler([t 0 0 10000 t jit-lock-deferred-fontify nil idle 0])
Running timers is pretty much the same as running redisplay: both are
things that can happen either between commands or in things like
`sit-for', so no, jit-lock is unlikely to be the culprit.
Maybe a way to check that is:
in Fredisplay, walk the specpdl stack looking for
a save_excursion_restore where the saved position is different from the
current value of point in that buffer.
Stefan
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Thu, 08 Aug 2013 02:42:01 GMT)
Full text and
rfc822 format available.
Message #26 received at 15045 <at> debbugs.gnu.org (full text, mbox):
> Date: Wed, 7 Aug 2013 16:39:29 -0400
> From: Barry OReilly <gundaetiapo <at> gmail.com>
> Cc: deng <at> randomsample.de, 15045 <at> debbugs.gnu.org
>
> > In the discussion that you cite at the beginning of this bug report,
> > I suggested a different approach to finding out the culprit, by
> > using the redisplay tracing facility. Can you try that and post the
> > results? It might give better clues.
>
> Though I can't reproduce it at will, I do have a trace-redisplay from
> when I saw the bug in a Java file. Specifically, I saw point move
> inappropriately, but redisplay didn't need to scroll. It's attached in
> case you find it useful.
Can you identify the area in this trace where the unwarranted scroll
was visible?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Thu, 08 Aug 2013 17:08:02 GMT)
Full text and
rfc822 format available.
Message #29 received at 15045 <at> debbugs.gnu.org (full text, mbox):
Eli:
> Can you identify the area in this trace where the unwarranted scroll
> was visible?
Barry:
> but redisplay didn't need to scroll
Undesired scrolling is a downstream symptom. Upstream from it is point
visibly moving around inappropriately. I have scroll-margin set to 4,
so when point briefly moves into the top scroll-margin, I get
undesired scrolling. I don't know that I've seen point move to outside
the visible part of the buffer. If so, then users with scroll-margin 0
won't see undesired scrolling, but would still see point moving
around.
Thus, I'm debugging the symptom of point moving.
> A, B, and C are local variables whose meaning depends on the email
> in which they're used.
Ah, so it's not in "thread local storage".
> in Fredisplay, walk the specpdl stack looking for a
> save_excursion_restore where the saved position is different from
> the current value of point in that buffer.
Thanks for the tip. I tested this change and it seems to implement
what you described.
diff --git a/src/dispnew.c b/src/dispnew.c
index 522a0e6..cf0103e 100644
--- a/src/dispnew.c
+++ b/src/dispnew.c
@@ -5815,6 +5815,32 @@ immediately by pending input. */)
(Lisp_Object force)
{
ptrdiff_t count;
+ bool noninteractive_old = noninteractive;
+ noninteractive = true;
+ Lisp_Object curPtMarker = Fpoint_marker();
+ union specbinding *pdl = specpdl_ptr;
+ while (pdl > specpdl)
+ {
+ --pdl;
+ if (pdl->kind == SPECPDL_UNWIND
+ && pdl->unwind.func == save_excursion_restore
+ && ! EQ (Fequal (XSAVE_OBJECT (pdl->unwind.arg, 0),
curPtMarker), Qt))
+ {
+ { struct timespec debug_ts; char debug_dateStr[20]; {
clock_gettime(CLOCK_REALTIME, &debug_ts); struct tm mytm;
localtime_r(&debug_ts.tv_sec, &mytm); strftime(debug_dateStr, 20,
"%Y-%m-%dT%H:%M:%S", &mytm)
+ printf( "%s.%09ld|pid:%d|tid:%ld|%s|%d| DEBUG: Found
save_excursion_restore with mismatched point markers ", // TODO:
debugging
+ debug_dateStr, debug_ts.tv_nsec, getpid(),
pthread_self(), __FILE__, __LINE__ ); }
+ Fprin1(XSAVE_OBJECT (pdl->unwind.arg, 0), Qnil);
+ Fprin1(curPtMarker, Qnil);
+ printf("\n");
+ Fbacktrace();
+ fflush(stdout);
+ }
+ }
+ /* { struct timespec debug_ts; char debug_dateStr[20]; {
clock_gettime(CLOCK_REALTIME, &debug_ts); struct tm mytm;
localtime_r(&debug_ts.tv_sec, &mytm); strftime(debug_dateStr, 20,
"%Y-%m-%dT%H:%M:%S", &mytm); } */
+ /* printf( "%s.%09ld|pid:%d|tid:%ld|%s|%d| DEBUG: redisplay
\n", // TODO: debugging */
+ /* debug_dateStr, debug_ts.tv_nsec,
getpid(), pthread_self(), __FILE__, __LINE__ ); fflush(stdout); } */
+ /* Fbacktrace(); */
+ noninteractive = noninteractive_old;
swallow_events (1);
if ((detect_input_pending_run_timers (1)
We'll see what I get next time it comes up.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Thu, 08 Aug 2013 17:22:02 GMT)
Full text and
rfc822 format available.
Message #32 received at 15045 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
[Adding Eric to CC]
David Engster writes:
> Stefan Monnier writes:
>>> The short of it is that I occasionally witness point jump to elsewhere
>>> in the buffer while I'm editing and then back again.
>>
>> So it's "displayed at A" then "displayed at B" then "displayed at
>> A again"? what happens between each one of those 3 displays?
>>
>> If "nothing", then I suspect there's something like a `sit-for'
>> somewhere that causes a redisplay in the middle of the command (i.e. in
>> the middle of a save-excursion).
>
> I sometimes see this as well, and yes, "nothing" happens between those
> three displays. I also think there's a redisplay triggered by another
> background task while Semantic does its idle parsing stuff.
I think I tracked this down. I first instrumented `sit-for' to see which
background tasks trigger it. On my machine, this happens by two things:
`jit-lock-deferred-fontify', and `display-time-event-handler'.
The display-time handler is called twice per minute: I have
(display-time) in my .emacs, and every full minute the time gets updated
in the mode-line. Also, it is called in the
gnus-after-get-new-news-hook, and since I check for new mails in the
background, it gets called through this, too.
It's kinda hard to trigger this problem through jit-lock, since its idle
time is much smaller than the one from Semantic. So I disabled it and
tried to trigger the jump by stopping typing at roughly XX:XX:59. The
semantic idle function kicks in after 1 second, and lo and behold, I saw
a jump. It's still difficult to reproduce, but I managed to get two
backtraces in the past hour, which are attached.
As you can see, the display-time-event-handler does indeed interrupt the
lexing phase. It does a `sit-for', the display jumps. Not sure what
happens after that. Does the semantic-idle function resume? Anyway,
somehow point gets back to its original position, and through
`trace-redisplay', I saw the following on stderr:
0x7f35178 (test.cpp): same window start
0x7f35178 (test.cpp): 1
redisplay_preserve_echo_area (2)
redisplay_internal 0
0x7f35178 (test.cpp): try_scrolling
redisplay_preserve_echo_area (8)
redisplay_internal 0
0x7f35178 (test.cpp): same window start
0x7f35178 (test.cpp): 1
0x7f35178 (test.cpp): try_scrolling
redisplay_preserve_echo_area (8)
I guess this just says that redisplay made point visible by scrolling,
right?
You might wonder how the display-time-event-handler can interrupt the
Semantic lexer. In the two backtraces, you see that it calls
`accept-process-output' and `input-pending-p'. This is hidden inside the
macro `semantic-throw-on-input', which can be called in code wrapped
inside `semantic-exit-on-input'; it's our poor-man's 'yield'. It's used
extensively in the idle function code, and it's just there to do a
non-local exit in case the user does something. However, now I know that
it also allows other timers to run.
If you look in the `define-lex' macro, you see that it calls
`semantic-throw-on-input' after each identified token. The problem is
that it does not restore the cursor position before that, so I guess the
fix is simply to change this call to
(save-excursion
(goto-char starting-position)
(semantic-throw-on-input 'lex))
Obviously, we will have to check all other calls to
`semantic-throw-on-input' for this as well. However, I also wonder if
display-time-event-handler couldn't just call `force-mode-line-update';
or does this repaint the whole display as well?
-David
[backtr1.txt (text/plain, inline)]
sit-for(0)
display-time-event-handler()
apply(display-time-event-handler nil)
byte-code("[snipped]" [timer apply 5 6] 4)
timer-event-handler([t 20995 27860 0 60 display-time-event-handler nil nil 0])
accept-process-output()
semantic-c-lexer(2886 7946 nil nil)
semantic-lex(2886 7946 nil)
semantic-parse-region-default(2886 7946 nil nil nil)
semantic-parse-region-c-mode(2886 7946 nil nil nil)
semantic-parse-region(2886 7946 nil)
semantic-edits-incremental-parser-1()
byte-code("[snipped]" [err message "incremental parser error: %S" error-message-string t] 4)))] 3)
semantic-parse-changes-default()
semantic-parse-changes()
semantic-fetch-tags()
byte-code("[snipped]" [semantic-fetch-tags nil] 1)
semantic-idle-scheduler-refresh-tags()
byte-code("[snipped]")
semantic-idle-core-handler()
semantic-idle-scheduler-function()
apply(semantic-idle-scheduler-function nil)
timer-event-handler([t 0 1 0 t semantic-idle-scheduler-function nil idle 0])
[backtr2.txt (text/plain, inline)]
sit-for(0)
display-time-event-handler()
apply(display-time-event-handler nil)
byte-code("[snipped]" [timer apply 5 6] 4)
timer-event-handler([t 20995 36800 0 60 display-time-event-handler nil nil 0])
input-pending-p()
semantic-c-lexer(2939 7944 nil nil)
semantic-lex(2939 7944 nil)
semantic-parse-region-default(2939 7944 bovine-inner-scope nil t)
semantic-parse-region-c-mode(2939 7944 bovine-inner-scope nil t)
semantic-parse-region(2939 7944 bovine-inner-scope nil t)
semantic-get-local-variables-default()
semantic-get-local-variables-c++-mode()
semantic-get-local-variables()
semantic-get-all-local-variables-default(nil)
semantic-get-all-local-variables()
byte-code("[snipped]" [scopecache eieio-oset localvar semantic-get-all-local-variables] 4)
semantic-calculate-scope(7797)
semantic-analyze-current-context-default(7797)
semantic-analyze-current-context(7797)
byte-code("[snipped]" [semantic-analyze-current-context] 2)
semantic-idle-summary-current-symbol-info-context()
semantic-idle-summary-current-symbol-info-default()
semantic-idle-summary-current-symbol-info-c-mode()
semantic-idle-summary-current-symbol-info()
semantic-idle-summary-idle-function()
funcall(semantic-idle-summary-idle-function)
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Thu, 08 Aug 2013 17:47:02 GMT)
Full text and
rfc822 format available.
Message #35 received at 15045 <at> debbugs.gnu.org (full text, mbox):
> Date: Thu, 8 Aug 2013 13:07:01 -0400
> From: Barry OReilly <gundaetiapo <at> gmail.com>
> Cc: deng <at> randomsample.de, 15045 <at> debbugs.gnu.org
>
> Eli:
> > Can you identify the area in this trace where the unwarranted scroll
> > was visible?
>
> Barry:
> > but redisplay didn't need to scroll
>
> Undesired scrolling is a downstream symptom. Upstream from it is point
> visibly moving around inappropriately.
I think you are wrong. Emacs moves point in its Lisp code all over
the place, and that never causes any unwarranted scrolling, nor should
it ever display point in anything but the final position -- unless the
Lisp code itself forces redisplay.
But even if you are right, knowing which parts of the display engine
are involved in this will allow us to put breakpoints in a few
strategic places, and produce backtraces, both in C and in Lisp, which
will show which Lisp code triggers the problem. This is IMO better
than trying to guess which Lisp primitives are involved in point
movement, because you are likely to guess wrong. By contrast,
redisplay is certainly involved, so using it will most probably show
us the light much sooner and easier.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Thu, 08 Aug 2013 18:07:01 GMT)
Full text and
rfc822 format available.
Message #38 received at 15045 <at> debbugs.gnu.org (full text, mbox):
> sit-for(0)
> display-time-event-handler()
> apply(display-time-event-handler nil)
> byte-code("[snipped]" [timer apply 5 6] 4)
> timer-event-handler([t 20995 27860 0 60 display-time-event-handler nil nil 0])
> accept-process-output()
> semantic-c-lexer(2886 7946 nil nil)
Right, that would do it.
What happens if you remove the calls to sit-for from time.el?
Stefan
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Thu, 08 Aug 2013 18:14:01 GMT)
Full text and
rfc822 format available.
Message #41 received at 15045 <at> debbugs.gnu.org (full text, mbox):
Stefan Monnier writes:
>> sit-for(0)
>> display-time-event-handler()
>> apply(display-time-event-handler nil)
>> byte-code("[snipped]" [timer apply 5 6] 4)
>> timer-event-handler([t 20995 27860 0 60 display-time-event-handler nil nil 0])
>> accept-process-output()
>> semantic-c-lexer(2886 7946 nil nil)
>
> Right, that would do it.
> What happens if you remove the calls to sit-for from time.el?
I would have thought that the time in the mode-line does not get updated
when Emacs is idle, but that does still work. So I've no idea why the
sit-for is there.
-David
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Thu, 08 Aug 2013 20:04:02 GMT)
Full text and
rfc822 format available.
Message #44 received at 15045 <at> debbugs.gnu.org (full text, mbox):
>> Undesired scrolling is a downstream symptom. Upstream from it is
>> point visibly moving around inappropriately.
> I think you are wrong. Emacs moves point in its Lisp code all over
> the place, and that never causes any unwarranted scrolling, nor
> should it ever display point in anything but the final position --
> unless the Lisp code itself forces redisplay.
My use of the adverb "visibly" was meant to indicate redisplay of
point at other positions. Sorry to have confused you.
> timer-event-handler([t 20995 27860 0 60 display-time-event-handler nil nil 0])
> accept-process-output()
> semantic-c-lexer(2886 7946 nil nil)
and
> timer-event-handler([t 20995 36800 0 60 display-time-event-handler nil nil 0])
> input-pending-p()
> semantic-c-lexer(2939 7944 nil nil)
This is indeed a key finding.
If arbitrary timers can execute during the lexer's call to
accept-process-output or input-pending-p, then doesn't that mean
jit-lock-deferred-fontify can run too? If removing timers' sit-for
calls is the solution, then what's to become of
jit-lock-deferred-fontify's call to sit-for? Doesn't deferred jit
locking necessarily have to call redisplay?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Thu, 08 Aug 2013 20:31:01 GMT)
Full text and
rfc822 format available.
Message #47 received at 15045 <at> debbugs.gnu.org (full text, mbox):
Barry OReilly writes:
>> timer-event-handler([t 20995 27860 0 60 display-time-event-handler nil nil 0])
>> accept-process-output()
>> semantic-c-lexer(2886 7946 nil nil)
>
> and
>
>> timer-event-handler([t 20995 36800 0 60 display-time-event-handler nil nil 0])
>> input-pending-p()
>> semantic-c-lexer(2939 7944 nil nil)
>
> This is indeed a key finding.
>
> If arbitrary timers can execute during the lexer's call to
> accept-process-output or input-pending-p, then doesn't that mean
> jit-lock-deferred-fontify can run too?
Under certain circumstances I guess it could, but usually, deferred
jit-lock will happen after a keypress, and that would already make the
Semantic idle function quit.
> If removing timers' sit-for calls is the solution, then what's to
> become of jit-lock-deferred-fontify's call to sit-for?
That is not the solution, at least not in general. As I've written, I
think the correct fix is for Semantic to make sure we restore point
before calling `semantic-throw-on-input'. Or maybe we should do this in
`semantic-exit-on-input'; I'm not sure.
However, doing redisplay in timers is not nice. If it is not necessary,
like maybe in the time-display handler, then it should be removed.
> Doesn't deferred jit locking necessarily have to call redisplay?
I would think so, too.
-David
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Thu, 08 Aug 2013 21:27:01 GMT)
Full text and
rfc822 format available.
Message #50 received at 15045 <at> debbugs.gnu.org (full text, mbox):
> If arbitrary timers can execute during the lexer's call to
> accept-process-output or input-pending-p, then doesn't that mean
> jit-lock-deferred-fontify can run too?
Yes.
> If removing timers' sit-for calls is the solution,
It's a workaround.
> then what's to become of jit-lock-deferred-fontify's call to sit-for?
> Doesn't deferred jit locking necessarily have to call redisplay?
Yes, tho I guess if absolutely needed, we could probably arrange for
jit-lock-deferred-fontify not to call sit-for. I'm not completely sure
how that could work, but it doesn't sound impossible.
Stefan
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Thu, 08 Aug 2013 21:40:01 GMT)
Full text and
rfc822 format available.
Message #53 received at 15045 <at> debbugs.gnu.org (full text, mbox):
> From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
> Cc: Barry OReilly <gundaetiapo <at> gmail.com>, 15045 <at> debbugs.gnu.org,
> "Eric M. Ludlam" <eric <at> siege-engine.com>, Eli Zaretskii <eliz <at> gnu.org>
> Date: Thu, 08 Aug 2013 14:06:28 -0400
>
> > sit-for(0)
> > display-time-event-handler()
> > apply(display-time-event-handler nil)
> > byte-code("[snipped]" [timer apply 5 6] 4)
> > timer-event-handler([t 20995 27860 0 60 display-time-event-handler nil nil 0])
> > accept-process-output()
> > semantic-c-lexer(2886 7946 nil nil)
>
> Right, that would do it.
> What happens if you remove the calls to sit-for from time.el?
You cannot ensure redisplay without that.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Thu, 08 Aug 2013 21:48:01 GMT)
Full text and
rfc822 format available.
Message #56 received at 15045 <at> debbugs.gnu.org (full text, mbox):
> Date: Thu, 8 Aug 2013 16:03:30 -0400
> From: Barry OReilly <gundaetiapo <at> gmail.com>
> Cc: Stefan Monnier <monnier <at> iro.umontreal.ca>, 15045 <at> debbugs.gnu.org,
> "Eric M. Ludlam" <eric <at> siege-engine.com>, Eli Zaretskii <eliz <at> gnu.org>
>
> If arbitrary timers can execute during the lexer's call to
> accept-process-output or input-pending-p, then doesn't that mean
> jit-lock-deferred-fontify can run too?
Not normally, because jit-lock-deferred-fontify runs off an idle
timer, and Emacs is not idle in the situation you describe.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Thu, 08 Aug 2013 21:50:02 GMT)
Full text and
rfc822 format available.
Message #59 received at 15045 <at> debbugs.gnu.org (full text, mbox):
> From: David Engster <deng <at> randomsample.de>
> Cc: Stefan Monnier <monnier <at> iro.umontreal.ca>, 15045 <at> debbugs.gnu.org, "Eric M. Ludlam" <eric <at> siege-engine.com>, Eli Zaretskii <eliz <at> gnu.org>
> Date: Thu, 08 Aug 2013 22:30:26 +0200
>
> However, doing redisplay in timers is not nice.
Why not?
> > Doesn't deferred jit locking necessarily have to call redisplay?
>
> I would think so, too.
But since jit-lock-deferred-fontify only happens when Emacs is idle,
there's no problem with that, since Emacs enters redisplay also when
it is idle.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Thu, 08 Aug 2013 21:58:02 GMT)
Full text and
rfc822 format available.
Message #62 received at 15045 <at> debbugs.gnu.org (full text, mbox):
> From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
> Cc: David Engster <deng <at> randomsample.de>, 15045 <at> debbugs.gnu.org,
> "Eric M. Ludlam" <eric <at> siege-engine.com>, Eli Zaretskii <eliz <at> gnu.org>
> Date: Thu, 08 Aug 2013 17:26:50 -0400
>
> > If arbitrary timers can execute during the lexer's call to
> > accept-process-output or input-pending-p, then doesn't that mean
> > jit-lock-deferred-fontify can run too?
>
> Yes.
No, I don't think so, because idle timers don't run.
> > If removing timers' sit-for calls is the solution,
>
> It's a workaround.
I think the right solution is to not call input-pending-p etc. during
lexer run.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Thu, 08 Aug 2013 22:52:01 GMT)
Full text and
rfc822 format available.
Message #65 received at 15045 <at> debbugs.gnu.org (full text, mbox):
>> > If arbitrary timers can execute during the lexer's call to
>> > accept-process-output or input-pending-p, then doesn't that mean
>> > jit-lock-deferred-fontify can run too?
>> Yes.
> No, I don't think so, because idle timers don't run.
You might be right, indeed.
>> > If removing timers' sit-for calls is the solution,
>> It's a workaround.
> I think the right solution is to not call input-pending-p etc. during
> lexer run.
Not calling accept-process-output might be tricky.
And not calling input-pending-p can also require very significant
code changes (I think support for concurrency would help a lot here).
Stefan
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Thu, 08 Aug 2013 22:52:02 GMT)
Full text and
rfc822 format available.
Message #68 received at 15045 <at> debbugs.gnu.org (full text, mbox):
>> Right, that would do it.
>> What happens if you remove the calls to sit-for from time.el?
> You cannot ensure redisplay without that.
I don't know what scenario you have in mind.
Stefan
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Fri, 09 Aug 2013 03:27:02 GMT)
Full text and
rfc822 format available.
Message #71 received at 15045 <at> debbugs.gnu.org (full text, mbox):
On 08/08/2013 01:21 PM, David Engster wrote:
> [Adding Eric to CC]
>
> David Engster writes:
>
> It's kinda hard to trigger this problem through jit-lock, since its idle
> time is much smaller than the one from Semantic. So I disabled it and
> tried to trigger the jump by stopping typing at roughly XX:XX:59. The
> semantic idle function kicks in after 1 second, and lo and behold, I saw
> a jump. It's still difficult to reproduce, but I managed to get two
> backtraces in the past hour, which are attached.
>
> As you can see, the display-time-event-handler does indeed interrupt the
> lexing phase. It does a `sit-for', the display jumps. Not sure what
> happens after that. Does the semantic-idle function resume? Anyway,
> somehow point gets back to its original position, and through
> `trace-redisplay', I saw the following on stderr:
David, this is some impressing debugging. Thanks for investigating so
thoroughly.
> You might wonder how the display-time-event-handler can interrupt the
> Semantic lexer. In the two backtraces, you see that it calls
> `accept-process-output' and `input-pending-p'. This is hidden inside the
> macro `semantic-throw-on-input', which can be called in code wrapped
> inside `semantic-exit-on-input'; it's our poor-man's 'yield'. It's used
> extensively in the idle function code, and it's just there to do a
> non-local exit in case the user does something. However, now I know that
> it also allows other timers to run.
>
> If you look in the `define-lex' macro, you see that it calls
> `semantic-throw-on-input' after each identified token. The problem is
> that it does not restore the cursor position before that, so I guess the
> fix is simply to change this call to
>
> (save-excursion
> (goto-char starting-position)
> (semantic-throw-on-input 'lex))
I pulled up some of the bigger C files in Emacs, ran the lexer, and they
all took less than .3 second to lex. It may also be safe to remove this
interactive optimization and just depend on input checking that happens
during parsing.
The detriment is that if someone had a really big file, they might
notice that Emacs stops responding if lexing takes a long time.
Eric
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Fri, 09 Aug 2013 05:37:02 GMT)
Full text and
rfc822 format available.
Message #74 received at 15045 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii writes:
>> From: David Engster <deng <at> randomsample.de>
>> Cc: Stefan Monnier <monnier <at> iro.umontreal.ca>,
>> 15045 <at> debbugs.gnu.org, "Eric M. Ludlam" <eric <at> siege-engine.com>, Eli
>> Zaretskii <eliz <at> gnu.org>
>> Date: Thu, 08 Aug 2013 22:30:26 +0200
>>
>> However, doing redisplay in timers is not nice.
>
> Why not?
Because doing redisplay is a user-visible side effect, and in general,
timers shouldn't have those. AFAICS, any function that calls things like
`accept-process-output', `input-pending-p' or `sit-for' inside a
`save-excursion' might get an unwanted scrolling effect if point is
temporarily moved to some invisible location.
In fact, I just understood another bug in speck-mode (which is similar
to flycheck). I sometimes had unwanted scrolls there, too, and I now saw
that those also happen at every full minute while typing.
>> > Doesn't deferred jit locking necessarily have to call redisplay?
>>
>> I would think so, too.
>
> But since jit-lock-deferred-fontify only happens when Emacs is idle,
> there's no problem with that, since Emacs enters redisplay also when
> it is idle.
I thought that the jit-lock timer is a non-idle timer, but you are
right.
-David
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Fri, 09 Aug 2013 07:54:02 GMT)
Full text and
rfc822 format available.
Message #77 received at 15045 <at> debbugs.gnu.org (full text, mbox):
> From: David Engster <deng <at> randomsample.de>
> Cc: gundaetiapo <at> gmail.com, monnier <at> iro.umontreal.ca, 15045 <at> debbugs.gnu.org, eric <at> siege-engine.com
> Date: Fri, 09 Aug 2013 07:36:07 +0200
>
> >> However, doing redisplay in timers is not nice.
> >
> > Why not?
>
> Because doing redisplay is a user-visible side effect, and in general,
> timers shouldn't have those.
How else can a timer such as that of display-time do its thing? IOW,
when the purpose of a timer is to cause something to be displayed,
they have no alternative but to force redisplay.
> AFAICS, any function that calls things like
> `accept-process-output', `input-pending-p' or `sit-for' inside a
> `save-excursion' might get an unwanted scrolling effect if point is
> temporarily moved to some invisible location.
You mean, because of timers that might get run and cause redisplay?
Yes, that's possible. But the way to avoid this is to never call
these when point is in a place the user won't expect. Forcing the
Emacs community not to trigger redisplay inside timers is IMO _not_
the right way, because this is unnecessarily restrictive, and would
disallow a whole bunch of useful features for no good reason.
IOW, it's the caller of input-pending-p etc. that is the fault here,
not the timers that it inadvertently lets run.
> In fact, I just understood another bug in speck-mode (which is similar
> to flycheck). I sometimes had unwanted scrolls there, too, and I now saw
> that those also happen at every full minute while typing.
Then there's the same bug there, that's all.
Doing things in the background is hard in Emacs, so it's a small
wonder some modes get it wrong.
> >> > Doesn't deferred jit locking necessarily have to call redisplay?
> >>
> >> I would think so, too.
> >
> > But since jit-lock-deferred-fontify only happens when Emacs is idle,
> > there's no problem with that, since Emacs enters redisplay also when
> > it is idle.
>
> I thought that the jit-lock timer is a non-idle timer, but you are
> right.
There's no jit-lock timer. There are timers for jit-lock-stealth and
for jit-lock-deferred-fontify (both optional features), and they are
all idle timers. JIT Lock itself is not triggered by any timer, it is
invoked by redisplay when a portion of text that is not fontified yet
comes into view.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Fri, 09 Aug 2013 07:55:01 GMT)
Full text and
rfc822 format available.
Message #80 received at 15045 <at> debbugs.gnu.org (full text, mbox):
> From: Stefan Monnier <monnier <at> iro.umontreal.ca>
> Cc: gundaetiapo <at> gmail.com, deng <at> randomsample.de, 15045 <at> debbugs.gnu.org, eric <at> siege-engine.com
> Date: Thu, 08 Aug 2013 18:50:42 -0400
>
> > I think the right solution is to not call input-pending-p etc. during
> > lexer run.
>
> Not calling accept-process-output might be tricky.
> And not calling input-pending-p can also require very significant
> code changes (I think support for concurrency would help a lot here).
Then perhaps the code which calls them should make sure to move point
to where the user expects it, before it does call those APIs.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Fri, 09 Aug 2013 07:57:01 GMT)
Full text and
rfc822 format available.
Message #83 received at 15045 <at> debbugs.gnu.org (full text, mbox):
> From: Stefan Monnier <monnier <at> iro.umontreal.ca>
> Cc: deng <at> randomsample.de, gundaetiapo <at> gmail.com, 15045 <at> debbugs.gnu.org, eric <at> siege-engine.com
> Date: Thu, 08 Aug 2013 18:51:17 -0400
>
> >> Right, that would do it.
> >> What happens if you remove the calls to sit-for from time.el?
> > You cannot ensure redisplay without that.
>
> I don't know what scenario you have in mind.
Any one. Emacs enters redisplay for any number of reasons, but you
can never be sure it will do so at any specific point unless you force
redisplay at that point. As you well know, in general, while Lisp
code runs, Emacs does not redisplay.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Fri, 09 Aug 2013 09:13:02 GMT)
Full text and
rfc822 format available.
Message #86 received at 15045 <at> debbugs.gnu.org (full text, mbox):
> In fact, I just understood another bug in speck-mode (which is similar
> to flycheck). I sometimes had unwanted scrolls there, too, and I now saw
> that those also happen at every full minute while typing.
What precisely is the bug in speck-mode? Is it that it doesn't restore
`point' when input arrives?
martin
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Fri, 09 Aug 2013 11:51:02 GMT)
Full text and
rfc822 format available.
Message #89 received at 15045 <at> debbugs.gnu.org (full text, mbox):
On 08/09/2013 03:53 AM, Eli Zaretskii wrote:
>> AFAICS, any function that calls things like
>> `accept-process-output', `input-pending-p' or `sit-for' inside a
>> `save-excursion' might get an unwanted scrolling effect if point is
>> temporarily moved to some invisible location.
>
> You mean, because of timers that might get run and cause redisplay?
> Yes, that's possible. But the way to avoid this is to never call
> these when point is in a place the user won't expect. Forcing the
> Emacs community not to trigger redisplay inside timers is IMO _not_
> the right way, because this is unnecessarily restrictive, and would
> disallow a whole bunch of useful features for no good reason.
>
> IOW, it's the caller of input-pending-p etc. that is the fault here,
> not the timers that it inadvertently lets run.
In my mind, to anyone using input-pending-p to deal with responsiveness
in long running code, a timer that causes a redisplay is the source of
the problem. In addition, it is clear that anyone with a sit-for in
their timer will think that moving the point and calling input-pending-p
is a bug.
Even if we resolve which is the real bug, or at least the proposed
solution here, people will still cause redisplays in timeres, and call
input-pending-p in long running code, and those problems will need to be
debugged again in the future.
It seems like it would be more developer friendly for Emacs to enforce
some kind of behavior with timers so as to remove this class of
collision. Here are some random options:
* In a timer, disallow dispatch of other timers until done.
* Record redisplay requests in timers, and force redisplay when the
timer exists.
* In input-pending-p, dont' allow new timers to run,
doc only talks about command input, not timers.
* In timers, redisplay always assumes you don't want to move point and
puts it back before the draw.
It is clear to me that there are valid use cases for both redisplay in a
timer, and input-pending-p being used in long-running programs. Adding
funky restrictions to either seems untenable with the size of the Emacs
dev community. It would be better to tackle the problem at its source.
Eric
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Fri, 09 Aug 2013 13:33:01 GMT)
Full text and
rfc822 format available.
Message #92 received at 15045 <at> debbugs.gnu.org (full text, mbox):
> Date: Fri, 09 Aug 2013 07:50:19 -0400
> From: "Eric M. Ludlam" <eric <at> siege-engine.com>
> CC: David Engster <deng <at> randomsample.de>, gundaetiapo <at> gmail.com,
> monnier <at> iro.umontreal.ca, 15045 <at> debbugs.gnu.org
>
> > IOW, it's the caller of input-pending-p etc. that is the fault here,
> > not the timers that it inadvertently lets run.
>
> In my mind, to anyone using input-pending-p to deal with responsiveness
> in long running code, a timer that causes a redisplay is the source of
> the problem.
Emacs is a complex piece of software. We cannot remove that
complexity no matter how hard we try. And yes, this requires
developers to be aware of what can happen when they call certain
APIs. There's nothing new here.
> Even if we resolve which is the real bug, or at least the proposed
> solution here, people will still cause redisplays in timeres, and call
> input-pending-p in long running code, and those problems will need to be
> debugged again in the future.
Again, nothing new. It's okay to propose solutions to these issues,
but a "solution" which tells timers not to force redisplay is a
non-starter, IMO, because some of them cannot do their thing without
displaying something at specific times.
> * In a timer, disallow dispatch of other timers until done.
Sounds not a good idea to me, as too many useful features use timers,
and users will be surprised to see them sometimes unable to run.
> * Record redisplay requests in timers, and force redisplay when the
> timer exists.
Not good for timers that must display at certain times, with minimal
delays.
> * In input-pending-p, dont' allow new timers to run,
> doc only talks about command input, not timers.
What if input arrives because of a timer?
> * In timers, redisplay always assumes you don't want to move point and
> puts it back before the draw.
Redisplay doesn't have any idea where is "back". When redisplay is
entered, it finds point at a certain location. It has no idea where
it was before, all it knows (in some cases, not all of them) is where
point was in each window during the last redisplay cycle, which could
be 5 msec ago or 5 min ago.
> It is clear to me that there are valid use cases for both redisplay in a
> timer, and input-pending-p being used in long-running programs. Adding
> funky restrictions to either seems untenable with the size of the Emacs
> dev community. It would be better to tackle the problem at its source.
I'm sorry, but I don't see "the source" here. What is the source of
the problem, in your opinion?
In my opinion, the source of the problem is that application code
moves point to a place it didn't intend the user to see, and then
calls some APIs that trigger redisplay and reveal that location of
point (and potentially even cause a scroll). The solution could be
(a) avoid triggering redisplay, or (b) restore point before calling
something that could cause redisplay. What other solutions do you
envision that don't limit the basic features involved in this?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Fri, 09 Aug 2013 14:04:01 GMT)
Full text and
rfc822 format available.
Message #95 received at 15045 <at> debbugs.gnu.org (full text, mbox):
>> >> Right, that would do it.
>> >> What happens if you remove the calls to sit-for from time.el?
>> > You cannot ensure redisplay without that.
>> I don't know what scenario you have in mind.
> Any one. Emacs enters redisplay for any number of reasons, but you
> can never be sure it will do so at any specific point unless you force
> redisplay at that point. As you well know, in general, while Lisp
> code runs, Emacs does not redisplay.
Of course, but that's true in general. What makes it more true in
display-time-event-handler? Remember that display-time-update (called
just before the sit-for) ends with a call to force-mode-line-update.
In practice, is there any important scenario where
display-time-event-handler's sit-for is useful?
Stefan
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Fri, 09 Aug 2013 14:05:02 GMT)
Full text and
rfc822 format available.
Message #98 received at 15045 <at> debbugs.gnu.org (full text, mbox):
> In my mind, to anyone using input-pending-p to deal with responsiveness in
Actually, now that I think about it. Why would input-pending-p
run timers? That sounds wrong.
Of course, fixing it won't change anything to the OP's problem since he
also has backtraces where the problem is triggered via
accept-process-output.
Stefan
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Fri, 09 Aug 2013 14:17:02 GMT)
Full text and
rfc822 format available.
Message #101 received at 15045 <at> debbugs.gnu.org (full text, mbox):
> From: Stefan Monnier <monnier <at> iro.umontreal.ca>
> Cc: deng <at> randomsample.de, gundaetiapo <at> gmail.com, 15045 <at> debbugs.gnu.org, eric <at> siege-engine.com
> Date: Fri, 09 Aug 2013 10:03:05 -0400
>
> >> >> Right, that would do it.
> >> >> What happens if you remove the calls to sit-for from time.el?
> >> > You cannot ensure redisplay without that.
> >> I don't know what scenario you have in mind.
> > Any one. Emacs enters redisplay for any number of reasons, but you
> > can never be sure it will do so at any specific point unless you force
> > redisplay at that point. As you well know, in general, while Lisp
> > code runs, Emacs does not redisplay.
>
> Of course, but that's true in general. What makes it more true in
> display-time-event-handler?
Why should we care? Good engineering does not build things on what
"currently happens to work", because that will eventually break,
given enough development.
> Remember that display-time-update (called just before the sit-for)
> ends with a call to force-mode-line-update.
Whose effect no one really understands.
> In practice, is there any important scenario where
> display-time-event-handler's sit-for is useful?
I never analyzed this to tell.
And anyway, display-time is just one case of a timer that needs to
force redisplay.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Fri, 09 Aug 2013 14:21:02 GMT)
Full text and
rfc822 format available.
Message #104 received at 15045 <at> debbugs.gnu.org (full text, mbox):
> From: Stefan Monnier <monnier <at> iro.umontreal.ca>
> Cc: Eli Zaretskii <eliz <at> gnu.org>, David Engster <deng <at> randomsample.de>, gundaetiapo <at> gmail.com, 15045 <at> debbugs.gnu.org
> Date: Fri, 09 Aug 2013 10:04:41 -0400
>
> > In my mind, to anyone using input-pending-p to deal with responsiveness in
>
> Actually, now that I think about it. Why would input-pending-p
> run timers?
The code is very explicit:
DEFUN ("input-pending-p", Finput_pending_p, Sinput_pending_p, 0, 0, 0,
doc: /* Return t if command input is currently available with no wait.
Actually, the value is nil only if we can be sure that no input is available;
if there is a doubt, the value is t. */)
(void)
{
if (!NILP (Vunread_command_events)
|| !NILP (Vunread_post_input_method_events)
|| !NILP (Vunread_input_method_events))
return (Qt);
/* Process non-user-visible events (Bug#10195). */
process_special_events ();
return (get_input_pending (READABLE_EVENTS_DO_TIMERS_NOW
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| READABLE_EVENTS_FILTER_EVENTS)
? Qt : Qnil);
> That sounds wrong.
Are you saying that using the READABLE_EVENTS_FILTER_EVENTS flag above
is wrong?
> Of course, fixing it won't change anything to the OP's problem since he
> also has backtraces where the problem is triggered via
> accept-process-output.
Right. Emacs generally always runs timers when it waits.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Fri, 09 Aug 2013 16:11:02 GMT)
Full text and
rfc822 format available.
Message #107 received at 15045 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii writes:
>> From: David Engster <deng <at> randomsample.de>
>> Cc: gundaetiapo <at> gmail.com, monnier <at> iro.umontreal.ca,
>> 15045 <at> debbugs.gnu.org, eric <at> siege-engine.com
>
>> Date: Fri, 09 Aug 2013 07:36:07 +0200
>>
>> >> However, doing redisplay in timers is not nice.
>> >
>> > Why not?
>>
>> Because doing redisplay is a user-visible side effect, and in general,
>> timers shouldn't have those.
>
> How else can a timer such as that of display-time do its thing? IOW,
> when the purpose of a timer is to cause something to be displayed,
> they have no alternative but to force redisplay.
As you know, Emacs does not have strict timers. If you say to Emacs: one
minute from now, say 'foo', and Emacs happens to be busy one minute from
now, nothing will happen until it waits again. So I wonder: with such a
lenient definition of "timer", does the wait for the next redisplay
really matter? After which idle time does redisplay kick in anyway?
>> AFAICS, any function that calls things like
>> `accept-process-output', `input-pending-p' or `sit-for' inside a
>> `save-excursion' might get an unwanted scrolling effect if point is
>> temporarily moved to some invisible location.
>
> You mean, because of timers that might get run and cause redisplay?
Yes.
> Yes, that's possible. But the way to avoid this is to never call
> these when point is in a place the user won't expect. Forcing the
> Emacs community not to trigger redisplay inside timers is IMO _not_
> the right way, because this is unnecessarily restrictive, and would
> disallow a whole bunch of useful features for no good reason.
I said: "doing redisplay in timers is not nice". I did not say that it
should be forbidden (if that's even possible).
However, I do propose the following:
- Deleting the 'sit-for' in the display-time-event-handler, since I
would think that it is the most likely suspect for a non-idle timer
that does redisplay in a usual Emacs session. The
force-mode-line-update should be enough, and even if it isn't: if the
minute is updated half a second too late, we can live with that (we
can already live with a displayed 'load' value that's up to a minute
old).
- In (info "(elisp) Timers"), the manual already discourages certain
things in timers, like changing buffer content or waiting with
`sit-for'. I think we should add something like this:
"Also, enforcing a redisplay in non-idle timers is problematic, since
they may run at times when other functions have moved point or
narrowed the buffer temporarily. The redisplay would make these
visible, leading to unwanted scrolling or sudden narrowing and
subsequent widening of the buffer. You should rather try if waiting
for the next redisplay after the timer has run is sufficient for your
needs. If you just change the mode-line, use `force-mode-line-update'
instead of a full redisplay."
- We should add information to the documentation of
`access-process-output' and `input-pending-p', like
"Note that timers may run during this function, which may trigger a
redisplay. So before calling this function, you should make sure that
any temporary changes you made to point position or the buffer (like
narrowing) are undone, as to not make these changes visible."
As a sidenote: What "bunch of useful features" do you have in mind for
non-idle timers that do redisplay? They shouldn't alter buffer content,
so what can they do that would need immediate redisplay? Displaying some
kind of progress/time indicator is pretty much the only thing I can come
up with.
> IOW, it's the caller of input-pending-p etc. that is the fault here,
> not the timers that it inadvertently lets run.
Until yesterday, I was completely unaware that `accept-process-output'
or `input-pending-p' could run timers that do redisplay, and I think I'm
in good company. The doc-strings do not mention it, so there must be
lots of third-party code which does not pay attention to this, so we
should try that this bug triggers as little as possible.
>> In fact, I just understood another bug in speck-mode (which is similar
>> to flycheck). I sometimes had unwanted scrolls there, too, and I now saw
>> that those also happen at every full minute while typing.
>
> Then there's the same bug there, that's all.
Yes. But it shows that this bug is easy to make, and it's hard to track
down. Also, the bug is slightly different in that speck does not only
alter point, but also narrows the buffer, which is even more scary than
unwanted scrolling.
>> I thought that the jit-lock timer is a non-idle timer, but you are
>> right.
>
> There's no jit-lock timer.
Yes. Sorry for being imprecise. I meant the jit-lock defer timer.
-David
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Fri, 09 Aug 2013 16:28:01 GMT)
Full text and
rfc822 format available.
Message #110 received at 15045 <at> debbugs.gnu.org (full text, mbox):
martin rudalics writes:
>> In fact, I just understood another bug in speck-mode (which is similar
>> to flycheck). I sometimes had unwanted scrolls there, too, and I now saw
>> that those also happen at every full minute while typing.
>
> What precisely is the bug in speck-mode? Is it that it doesn't restore
> `point' when input arrives?
The main problem is that speck narrows the buffer before calling
`accept-process-output'. If that happens while the
time-display-event-handler runs, you suddenly see a narrowed buffer,
which is soon widened again. Pretty scary.
It's quite easy to reproduce, since speck runs much longer than the
Semantic idle function. Here's a recipe:
- Put a bunch of text in the kill ring
- Create a new text buffer and activate speck
- Activate `display-time'
- At roughly XX:58, yank the text into the buffer and wait
Here's the backtrace from a `debug' call in `sit-for':
sit-for(0)
display-time-event-handler()
apply(display-time-event-handler nil)
byte-code("" [timer apply 5 6] 4)
timer-event-handler([t 20997 5988 0 60 display-time-event-handler nil nil 0])
accept-process-output(nil 0.01)
speck-chunk()
speck-multi-chunk()
speck-chunks()
speck-line()
speck-window(t)
speck-windows(t)
apply(speck-windows t)
byte-code("" [timer apply 5 6] 4)
timer-event-handler([t 0 1 262453 nil speck-windows (t) idle 785000])
The fix would be to undo any narrowing and restore point before calling
`accept-process-output'.
-David
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Fri, 09 Aug 2013 17:11:01 GMT)
Full text and
rfc822 format available.
Message #113 received at 15045 <at> debbugs.gnu.org (full text, mbox):
> The main problem is that speck narrows the buffer before calling
> `accept-process-output'. If that happens while the
> time-display-event-handler runs, you suddenly see a narrowed buffer,
> which is soon widened again. Pretty scary.
Very annoying, at least.
> The fix would be to undo any narrowing and restore point before calling
> `accept-process-output'.
Very inconvenient, at least.
Many thanks for the explanation and the investigative work, martin
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Fri, 09 Aug 2013 17:35:01 GMT)
Full text and
rfc822 format available.
Message #116 received at 15045 <at> debbugs.gnu.org (full text, mbox):
> Why should we care? Good engineering does not build things on what
> "currently happens to work", because that will eventually break,
> given enough development.
I care because if I don't have a positive reason to keep this call to
sit-for I'm going to remove it.
>> In practice, is there any important scenario where
>> display-time-event-handler's sit-for is useful?
Thanks. So as far as you know it's not any more useful than adding
sit-for at the end of any other function.
> And anyway, display-time is just one case of a timer that needs to
> force redisplay.
No disagreement here. This bug report just made me bump into this
tangentially related call to sit-for which I'll be happy to remove.
Stefan
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Fri, 09 Aug 2013 18:21:02 GMT)
Full text and
rfc822 format available.
Message #119 received at 15045 <at> debbugs.gnu.org (full text, mbox):
>> That sounds wrong.
> Are you saying that using the READABLE_EVENTS_FILTER_EVENTS flag above
> is wrong?
I think so: input-pending-p is not expected to wait, so I don't see any
reason to run timers.
Stefan
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Fri, 09 Aug 2013 18:24:01 GMT)
Full text and
rfc822 format available.
Message #122 received at 15045 <at> debbugs.gnu.org (full text, mbox):
> From: Stefan Monnier <monnier <at> iro.umontreal.ca>
> Cc: deng <at> randomsample.de, gundaetiapo <at> gmail.com, 15045 <at> debbugs.gnu.org, eric <at> siege-engine.com
> Date: Fri, 09 Aug 2013 13:34:25 -0400
>
> > Why should we care? Good engineering does not build things on what
> > "currently happens to work", because that will eventually break,
> > given enough development.
>
> I care because if I don't have a positive reason to keep this call to
> sit-for I'm going to remove it.
If you don't care about a bug report that will have you re-add it,
feel free.
> Thanks. So as far as you know it's not any more useful than adding
> sit-for at the end of any other function.
Only those that need to make sure their effect is displayed.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Fri, 09 Aug 2013 18:32:02 GMT)
Full text and
rfc822 format available.
Message #125 received at 15045 <at> debbugs.gnu.org (full text, mbox):
> From: David Engster <deng <at> randomsample.de>
> Cc: gundaetiapo <at> gmail.com, monnier <at> iro.umontreal.ca, 15045 <at> debbugs.gnu.org, eric <at> siege-engine.com
> Date: Fri, 09 Aug 2013 18:10:04 +0200
>
> >> >> However, doing redisplay in timers is not nice.
> >> >
> >> > Why not?
> >>
> >> Because doing redisplay is a user-visible side effect, and in general,
> >> timers shouldn't have those.
> >
> > How else can a timer such as that of display-time do its thing? IOW,
> > when the purpose of a timer is to cause something to be displayed,
> > they have no alternative but to force redisplay.
>
> As you know, Emacs does not have strict timers. If you say to Emacs: one
> minute from now, say 'foo', and Emacs happens to be busy one minute from
> now, nothing will happen until it waits again. So I wonder: with such a
> lenient definition of "timer", does the wait for the next redisplay
> really matter?
Emacs very seldom is busy for a long time, so in practice you will see
the 1-min timer do its thing pretty much on time.
> After which idle time does redisplay kick in anyway?
Redisplay doesn't count idle time to kick in, so there's no answer to
your question. Redisplay is triggered by several reasons, one of them
being waiting for keyboard input, which you might call "being idle",
although that's not exactly accurate.
> As a sidenote: What "bunch of useful features" do you have in mind for
> non-idle timers that do redisplay? They shouldn't alter buffer content,
> so what can they do that would need immediate redisplay? Displaying some
> kind of progress/time indicator is pretty much the only thing I can come
> up with.
Any display-related feature, such as blinking something, displaying
something ion the mode line or header line, etc. Altering a buffer
can be one of the effects, btw, e.g. in a feature that shows the list
of processes on a system and refreshes that list every second.
> > IOW, it's the caller of input-pending-p etc. that is the fault here,
> > not the timers that it inadvertently lets run.
>
> Until yesterday, I was completely unaware that `accept-process-output'
> or `input-pending-p' could run timers that do redisplay, and I think I'm
> in good company. The doc-strings do not mention it, so there must be
> lots of third-party code which does not pay attention to this, so we
> should try that this bug triggers as little as possible.
I agree that the doc strings should make a prominent reference to this
issue.
> >> In fact, I just understood another bug in speck-mode (which is similar
> >> to flycheck). I sometimes had unwanted scrolls there, too, and I now saw
> >> that those also happen at every full minute while typing.
> >
> > Then there's the same bug there, that's all.
>
> Yes. But it shows that this bug is easy to make, and it's hard to track
> down.
It is also very rare.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Fri, 09 Aug 2013 18:39:02 GMT)
Full text and
rfc822 format available.
Message #128 received at 15045 <at> debbugs.gnu.org (full text, mbox):
> From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
> Cc: eric <at> siege-engine.com, deng <at> randomsample.de, gundaetiapo <at> gmail.com,
> 15045 <at> debbugs.gnu.org
> Date: Fri, 09 Aug 2013 14:08:13 -0400
>
> >> That sounds wrong.
> > Are you saying that using the READABLE_EVENTS_FILTER_EVENTS flag above
> > is wrong?
>
> I think so: input-pending-p is not expected to wait, so I don't see any
> reason to run timers.
I think the only reason, as with all the other places where we run
timers, is to make Emacs look appear more responsive, notwithstanding
the on-going processing.
(And I meant the READABLE_EVENTS_DO_TIMERS_NOW flag, of course.)
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Fri, 09 Aug 2013 18:42:02 GMT)
Full text and
rfc822 format available.
Message #131 received at 15045 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii writes:
> I think the only reason, as with all the other places where we run
> timers, is to make Emacs look appear more responsive, notwithstanding
> the on-going processing.
Is there some easy way to get a list of functions which may cause timers
to run? IMHO all those functions would need a word of warning in their
doc-string.
-David
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Fri, 09 Aug 2013 18:51:02 GMT)
Full text and
rfc822 format available.
Message #134 received at 15045 <at> debbugs.gnu.org (full text, mbox):
I just removed the two calls to sit-for from time.el.
We'll see if someone screams showing a scenario where they were useful.
Stefan
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Fri, 09 Aug 2013 18:52:01 GMT)
Full text and
rfc822 format available.
Message #137 received at 15045 <at> debbugs.gnu.org (full text, mbox):
>> What precisely is the bug in speck-mode? Is it that it doesn't restore
>> `point' when input arrives?
> The main problem is that speck narrows the buffer before calling
> `accept-process-output'.
Yes, that's a bug in speck-mode.
Stefan
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Fri, 09 Aug 2013 20:50:02 GMT)
Full text and
rfc822 format available.
Message #140 received at 15045 <at> debbugs.gnu.org (full text, mbox):
> From: David Engster <deng <at> randomsample.de>
> Cc: Stefan Monnier <monnier <at> IRO.UMontreal.CA>, eric <at> siege-engine.com, gundaetiapo <at> gmail.com, 15045 <at> debbugs.gnu.org
> Date: Fri, 09 Aug 2013 20:41:44 +0200
>
> Eli Zaretskii writes:
> > I think the only reason, as with all the other places where we run
> > timers, is to make Emacs look appear more responsive, notwithstanding
> > the on-going processing.
>
> Is there some easy way to get a list of functions which may cause timers
> to run?
Scanning the C sources is what I'd do. Not sure if that can go by the
"easy way" handle, though.
> IMHO all those functions would need a word of warning in their
> doc-string.
Yep.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Fri, 09 Aug 2013 21:37:02 GMT)
Full text and
rfc822 format available.
Message #143 received at 15045 <at> debbugs.gnu.org (full text, mbox):
>> I think the only reason, as with all the other places where we run
>> timers, is to make Emacs look appear more responsive, notwithstanding
>> the on-going processing.
> Is there some easy way to get a list of functions which may cause timers
> to run? IMHO all those functions would need a word of warning in their
> doc-string.
Basically, read-event, read-char, read-key-sequence (and friends), and
accept-process-output and input-pending-p, plus any function that calls
one of them (e.g. sit-for).
Stefan
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Fri, 09 Aug 2013 21:47:02 GMT)
Full text and
rfc822 format available.
Message #146 received at 15045 <at> debbugs.gnu.org (full text, mbox):
>> >> That sounds wrong.
>> > Are you saying that using the READABLE_EVENTS_DO_TIMERS_NOW flag above
>> > is wrong?
>> I think so: input-pending-p is not expected to wait, so I don't see any
>> reason to run timers.
> I think the only reason, as with all the other places where we run
> timers, is to make Emacs look appear more responsive, notwithstanding
> the on-going processing.
I suspect that the users of input-pending-p which care about running
timers would be better served by (sit-for 0 t). Currently, it's 100%
equivalent, but the intention is a lot more clear.
Stefan
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Sat, 10 Aug 2013 09:43:01 GMT)
Full text and
rfc822 format available.
Message #149 received at 15045 <at> debbugs.gnu.org (full text, mbox):
Stefan Monnier writes:
>>> I think the only reason, as with all the other places where we run
>>> timers, is to make Emacs look appear more responsive, notwithstanding
>>> the on-going processing.
>> Is there some easy way to get a list of functions which may cause timers
>> to run? IMHO all those functions would need a word of warning in their
>> doc-string.
>
> Basically, read-event, read-char, read-key-sequence (and friends), and
> accept-process-output and input-pending-p,
I think we should mention this issue in their doc-strings. Should they
all get same blurb like "This might run timers that trigger redisplay,
so you should make sure that etc." or should they just get a short
notice like "This function runs timers (see 'some-doc-string or info
page').".
> plus any function that calls one of them (e.g. sit-for).
Those are at least a few hundred, so we cannot possibly alter all the
doc-strings for those.
-David
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Sat, 10 Aug 2013 09:55:01 GMT)
Full text and
rfc822 format available.
Message #152 received at 15045 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii writes:
>> From: David Engster <deng <at> randomsample.de>
>> Yes. But it shows that this bug is easy to make, and it's hard to track
>> down.
>
> It is also very rare.
I don't think the bug is rare. I do think that this bug being triggered
is rare, since we don't have many non-idle timers in Emacs that trigger
redisplay (are there others in Emacs core besides the display-time event
handler?). It might be fun to work for a while with a dummy timer which
triggers redisplay once a second or so.
I also think this bug is seldomly reported because it's impossible to
give a recipe if you don't know that it is a timer that triggers it
(which is why I never bothered to report the bug in speck-mode).
-David
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Sat, 10 Aug 2013 10:24:02 GMT)
Full text and
rfc822 format available.
Message #155 received at 15045 <at> debbugs.gnu.org (full text, mbox):
> From: David Engster <deng <at> randomsample.de>
> Cc: gundaetiapo <at> gmail.com, monnier <at> iro.umontreal.ca, 15045 <at> debbugs.gnu.org, eric <at> siege-engine.com
> Date: Sat, 10 Aug 2013 11:54:32 +0200
>
> Eli Zaretskii writes:
> >> From: David Engster <deng <at> randomsample.de>
> >> Yes. But it shows that this bug is easy to make, and it's hard to track
> >> down.
> >
> > It is also very rare.
>
> I don't think the bug is rare. I do think that this bug being triggered
> is rare, since we don't have many non-idle timers in Emacs that trigger
> redisplay
The last sentence exactly means that the bug is rare.
> (are there others in Emacs core besides the display-time event
> handler?).
And so does this.
> It might be fun to work for a while with a dummy timer which
> triggers redisplay once a second or so.
Customize display-time-interval to 1 (after restoring the sit-for that
got deleted), and you've got that. I'm running for years with that
variable customized to 5 sec, and never saw any unwarranted movement
of point or scrolling. That's "rare" in my book.
> I also think this bug is seldomly reported because it's impossible to
> give a recipe if you don't know that it is a timer that triggers it
> (which is why I never bothered to report the bug in speck-mode).
The bug database is replete with problems that are not reproducible,
so I'm quite sure people would report them regardless.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Sat, 10 Aug 2013 18:07:01 GMT)
Full text and
rfc822 format available.
Message #158 received at 15045 <at> debbugs.gnu.org (full text, mbox):
>>> It is also very rare.
>> I don't think the bug is rare. I do think that this bug being
>> triggered is rare, since we don't have many non-idle timers in
>> Emacs that trigger redisplay
> That's "rare" in my book.
Three people in this thread independently witnessed the bug. Given the
proportion of Emacs users who follow the mailing lists, calling this
rare is questionable.
> are there others in Emacs core besides the display-time event
> handler?
I have never used display-time-mode, so another timer is involved,
possibly non core. I don't see this bug nearly as often as David's
"every 15 minutes". I would guess 10 times in the past several
months. I'm running Emacs now with the debugging changes Stefan
suggested:
> in Fredisplay, walk the specpdl stack looking for a
> save_excursion_restore where the saved position is different from
> the current value of point in that buffer.
as the change I posted:
http://debbugs.gnu.org/cgi/bugreport.cgi?bug=15045#29
I did not witness the bug yesterday, but I did hit the debug
statements a few times. In those cases the Fprin1 and Fbacktrace calls
printed empty strings, and coincided with Emacs pausing for a few
seconds. I don't know why I'd get empty strings. Maybe there's a flaw
in that debugging code?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Mon, 14 Oct 2013 19:33:01 GMT)
Full text and
rfc822 format available.
Message #161 received at 15045 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
> >> >> That sounds wrong.
> >> > Are you saying that using the READABLE_EVENTS_DO_TIMERS_NOW flag
above
> >> > is wrong?
> >> I think so: input-pending-p is not expected to wait, so I don't see any
> >> reason to run timers.
> > I think the only reason, as with all the other places where we run
> > timers, is to make Emacs look appear more responsive, notwithstanding
> > the on-going processing.
>
> I suspect that the users of input-pending-p which care about running
> timers would be better served by (sit-for 0 t). Currently, it's 100%
> equivalent, but the intention is a lot more clear.
Given this has come up again separately in bug 15567, may I make the
change to input-pending-p?
diff --git a/src/keyboard.c b/src/keyboard.c
index e0cd4d4..fdb7c7d 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -9962,8 +9962,7 @@ if there is a doubt, the value is t. */)
/* Process non-user-visible events (Bug#10195). */
process_special_events ();
- return (get_input_pending (READABLE_EVENTS_DO_TIMERS_NOW
- | READABLE_EVENTS_FILTER_EVENTS)
+ return (get_input_pending (READABLE_EVENTS_FILTER_EVENTS)
? Qt : Qnil);
}
'make check' passes.
Eli Zaretskii:
> What if input arrives because of a timer?
If this is an issue, I propose input-pending-p return t if a timer is
ready to run. Then the long running task would exit, unwind its
save-excursion, then allow the timer to run. Perhaps this contorts the
meaning of "input", but OTOH the documentation for input-pending-p
already states that a false positive return can happen:
Return t if command input is currently available with no wait.
Actually, the value is nil only if we can be sure that no input is
available; if there is a doubt, the value is t.
Would you want this in the same changeset as the patch above? Or not
worry about it until "someone screams"?
[Message part 2 (text/html, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Mon, 14 Oct 2013 19:52:02 GMT)
Full text and
rfc822 format available.
Message #164 received at 15045 <at> debbugs.gnu.org (full text, mbox):
> Date: Mon, 14 Oct 2013 15:32:40 -0400
> From: Barry OReilly <gundaetiapo <at> gmail.com>
> Cc: David Engster <deng <at> randomsample.de>, Stefan Monnier <monnier <at> iro.umontreal.ca>,
> 15045 <at> debbugs.gnu.org, Eric Ludlam <eric <at> siege-engine.com>
>
> Given this has come up again separately in bug 15567, may I make the
> change to input-pending-p?
>
> diff --git a/src/keyboard.c b/src/keyboard.c
> index e0cd4d4..fdb7c7d 100644
> --- a/src/keyboard.c
> +++ b/src/keyboard.c
> @@ -9962,8 +9962,7 @@ if there is a doubt, the value is t. */)
> /* Process non-user-visible events (Bug#10195). */
> process_special_events ();
>
> - return (get_input_pending (READABLE_EVENTS_DO_TIMERS_NOW
> - | READABLE_EVENTS_FILTER_EVENTS)
> + return (get_input_pending (READABLE_EVENTS_FILTER_EVENTS)
> ? Qt : Qnil);
> }
>
> 'make check' passes.
FWIW, I'm against such changes. You can never know how many places
will be subtly broken by changes like that in such a popular
interface.
The original problem is rare enough, and should IMO be fixed in the
code where it happens. Making changes like above on behalf of such
marginal use cases is a tail wagging the dog.
> Eli Zaretskii:
> > What if input arrives because of a timer?
>
> If this is an issue, I propose input-pending-p return t if a timer is
> ready to run.
Likewise: too serious change in behavior for too obscure a reason.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Tue, 15 Oct 2013 13:43:02 GMT)
Full text and
rfc822 format available.
Message #167 received at 15045 <at> debbugs.gnu.org (full text, mbox):
> FWIW, I'm against such changes. You can never know how many places
> will be subtly broken by changes like that in such a popular
> interface.
Hmm... I kind of agree with both:
- this is a subtle change affecting behavior in a corner case and it's
difficult to know which code might care about it.
- nothing in input-pending-p indicates this is some kind of
blocking/waiting operation which might hence correspond to a "yield
point". On the contrary, the intuition behind input-pending-p would
make you think that the whole point of it is to let you find out if
there's input *without waiting*.
So I tend to agree with Barry that input-pending-p should not run
timers. Not just based on his particular problem case, but on the basis
of what ideally input-pending-p should do.
Arguably it shouldn't process_special_events either, tho obviously,
removing this call would introduce a known breakage, so we should keep
this for now (until we have a better fix for it).
> The original problem is rare enough, and should IMO be fixed in the
> code where it happens.
It can be difficult to "fix" it there, because there's simply no other
operation that lets you detect if there's input in the queue without
running arbitrary code, so it can require non-trivial refactoring.
>> > What if input arrives because of a timer?
>> If this is an issue, I propose input-pending-p return t if a timer is
>> ready to run.
> Likewise: too serious change in behavior for too obscure a reason.
100% agreement. `input-pending-p' is meant to detect *user input*, and
timers do not count as such (and input generated by timers doesn't
count as "user input" either).
Stefan
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Tue, 15 Oct 2013 14:13:02 GMT)
Full text and
rfc822 format available.
Message #170 received at 15045 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
> The original problem is rare enough
You're wrong to call it rare. Dave Milter, David Engster, and I have
seen it separately. It has occurred because of tasks in Semantic,
Speck, and now NXML. That input-pending-p behaves this way is
apparently surprising to the Elisp developer. It is even more
surprising to the user upon witnessing such a jarring symptom.
> So I tend to agree with Barry that input-pending-p should not run
> timers. Not just based on his particular problem case, but on the
> basis of what ideally input-pending-p should do.
I don't claim the change solves the issue completely. This bug report
would have to stay open because Semantic also calls
accept-process-output while point is on an excursion.
Since Stefan had said input-pending-p behavior seems wrong, I followed
up in the hopes we can both correct a wrong and reduce how common this
user-visible symptom is.
[Message part 2 (text/html, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Tue, 15 Oct 2013 16:30:02 GMT)
Full text and
rfc822 format available.
Message #173 received at 15045 <at> debbugs.gnu.org (full text, mbox):
> Date: Tue, 15 Oct 2013 10:12:32 -0400
> From: Barry OReilly <gundaetiapo <at> gmail.com>
> Cc: Eli Zaretskii <eliz <at> gnu.org>, David Engster <deng <at> randomsample.de>, 15045 <at> debbugs.gnu.org,
> Eric Ludlam <eric <at> siege-engine.com>
>
> > The original problem is rare enough
>
> You're wrong to call it rare. Dave Milter, David Engster, and I have
> seen it separately.
For functionality so widely and freely used in every corner of Emacs,
anything is "rare" in my book, except if it totally screws up Emacs
for almost everyone. input-pending-p is all over the place, so if it
were not "rare", we would be facing an outcry long ago.
> That input-pending-p behaves this way is apparently surprising to
> the Elisp developer.
Yes, it _is_ surprising. But if every surprising behavior will
necessarily lead to change in long entrenched behavior, Emacs will
never be stable.
> It is even more surprising to the user upon witnessing such a
> jarring symptom.
I didn't say that problem shouldn't be solved. I didn't say we should
let users deal with this. So I find this remark unfair.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Tue, 15 Oct 2013 17:10:02 GMT)
Full text and
rfc822 format available.
Message #176 received at 15045 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
I'm thinking I should augment the patch so as the observable behavior
of (sit-for 0) doesn't change. It would no longer run timers if only
input-pending-p changes as described. Currently:
(defun sit-for (seconds &optional nodisp obsolete)
[...]
(cond
[...]
((input-pending-p)
nil)
((<= seconds 0)
(or nodisp (redisplay)))
[...]
I considered whether (<= seconds 0) could fall through to the t case
instead. The read_char function has a complex implementation, so it's
not clear doing so wouldn't change behavior.
Is there another Lisp function that does timer_check and little else,
which the (<= seconds 0) case could call? Should I write a new subr
for it?
Looking ahead to the concurrency branch, should (sit-for 0 t) and
(thread-yield) have different behaviors? Or perhaps the former calls
the latter? I don't see thread-yield checking for timers to run,
should it? (And it didn't look like timers were split out into their
own threads on the concurrency branch.)
[Message part 2 (text/html, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Tue, 15 Oct 2013 18:50:01 GMT)
Full text and
rfc822 format available.
Message #179 received at 15045 <at> debbugs.gnu.org (full text, mbox):
> Date: Tue, 15 Oct 2013 13:08:58 -0400
> From: Barry OReilly <gundaetiapo <at> gmail.com>
> Cc: Stefan Monnier <monnier <at> iro.umontreal.ca>, David Engster <deng <at> randomsample.de>,
> 15045 <at> debbugs.gnu.org, Eric Ludlam <eric <at> siege-engine.com>
>
> (defun sit-for (seconds &optional nodisp obsolete)
> [...]
> (cond
> [...]
> ((input-pending-p)
> nil)
> ((<= seconds 0)
> (or nodisp (redisplay)))
> [...]
>
> I considered whether (<= seconds 0) could fall through to the t case
> instead.
Maybe I'm missing something, but wouldn't that bypass redisplay?
Currently, (sit-for 0) is a very popular method to trigger redisplay;
changing that _will_ bite a lot of code out there.
> The read_char function has a complex implementation, so it's not
> clear doing so wouldn't change behavior.
Are you assuming that read_char always redisplays? That's not true,
AFAIK.
> Is there another Lisp function that does timer_check and little else,
> which the (<= seconds 0) case could call? Should I write a new subr
> for it?
Not sure I understand the question, sorry.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Tue, 15 Oct 2013 19:20:02 GMT)
Full text and
rfc822 format available.
Message #182 received at 15045 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
What I'm saying is that
[...]
((input-pending-p)
nil)
((<= seconds 0)
(or nodisp (redisplay)))
[...]
would become
[...]
((input-pending-p)
nil)
((<= seconds 0)
(to-be-determined-call-to-timer-check)
(or nodisp (redisplay)))
[...]
The idea is to compensate for input-pending-p no longer calling
timer_check so as sit-for will behave the same.
[Message part 2 (text/html, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Wed, 16 Oct 2013 02:58:01 GMT)
Full text and
rfc822 format available.
Message #185 received at 15045 <at> debbugs.gnu.org (full text, mbox):
> of (sit-for 0) doesn't change. It would no longer run timers if only
> input-pending-p changes as described. Currently:
> (defun sit-for (seconds &optional nodisp obsolete)
> [...]
> (cond
> [...]
> ((input-pending-p)
> nil)
> ((<= seconds 0)
> (or nodisp (redisplay)))
> [...]
Indeed, the Elisp version of sit-for is a mess.
> I considered whether (<= seconds 0) could fall through to the t case
> instead.
The easiest solution is to add an optional argument `check-timers' to
input-pending-p, so you can preserve the exact same behavior.
In the long run, we should try and clean up sit-for, but you may not
have the courage to dig into this right now.
> Looking ahead to the concurrency branch, should (sit-for 0 t) and
> (thread-yield) have different behaviors?
Not really.
> I don't see thread-yield checking for timers to run, should it?
Yes, it should.
> (And it didn't look like timers were split out into their own threads
> on the concurrency branch.)
The current state of the concurrency branch is not always
representative of what it should ideally look like ;-)
But even if timers are made to belong to a thread, thread-yield should
probably check timers.
Stefan
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Wed, 16 Oct 2013 14:58:02 GMT)
Full text and
rfc822 format available.
Message #188 received at 15045 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
> The easiest solution is to add an optional argument `check-timers'
> to input-pending-p, so you can preserve the exact same behavior.
I assume you mean no-check-timers, nil being default.
> In the long run, we should try and clean up sit-for, but you may not
> have the courage to dig into this right now.
I'm arguing I don't need to dig into sit-for's complexity to maintain
its current behavior. If (sit-for 0 ...) calls something like
thread-yield which in turn calls timer_check, then all ways of calling
sit-for would have unchanged behavior.
Even so, because of other calls to input-pending-p not through
sit-for, there could conceivably be regressions in the latencies of
timers, due to background tasks not yielding when they used to. If we
discovered such cases, the solution is straight forward: replace
(input-pending-p) with (sit-for 0 t) in the offending background task.
But I understand the objection.
If the no-check-timers arg solution is what you want, I'll prepare a
patch to add it and have Semantic and NXML use it.
Also, what do you think are the merits of putting in the
specpdl-walking check discussed earlier, enabled with
--enable-checking? I can't see a legit case where check_timers is
called when point is on an excursion. Such code should not make any
assumptions about the other timers the user is running which might
call redisplay.
> The current state of the concurrency branch is not always
> representative of what it should ideally look like ;-) But even if
> timers are made to belong to a thread, thread-yield should probably
> check timers.
The timer behavior on the concurrency branch seems reasonable. Moving
them to their own threads might be complex, not least because of
concerns that the global lock will be yielded often enough to not
regress timer latencies.
[Message part 2 (text/html, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Wed, 16 Oct 2013 17:51:02 GMT)
Full text and
rfc822 format available.
Message #191 received at 15045 <at> debbugs.gnu.org (full text, mbox):
>> The easiest solution is to add an optional argument `check-timers'
>> to input-pending-p, so you can preserve the exact same behavior.
> I assume you mean no-check-timers, nil being default.
No, I meant `check-timers', so the default behavior is modified, but
you can recover the previous behavior by passing an extra argument.
>> In the long run, we should try and clean up sit-for, but you may not
>> have the courage to dig into this right now.
> I'm arguing I don't need to dig into sit-for's complexity to maintain
> its current behavior. If (sit-for 0 ...) calls something like
> thread-yield which in turn calls timer_check, then all ways of calling
> sit-for would have unchanged behavior.
That's theory: there is no thread-yield right now, nor any timer-check.
> Also, what do you think are the merits of putting in the
> specpdl-walking check discussed earlier, enabled with
> --enable-checking?
I like the idea, tho I'd have to see the code first to know
whether it should depend on --enable-checking or something else.
Stefan
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Wed, 16 Oct 2013 18:33:02 GMT)
Full text and
rfc822 format available.
Message #194 received at 15045 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
> That's theory: there is no thread-yield right now, nor any
> timer-check.
Why not add either? We could implement thread-yield on trunk to call
the timer_check C function and nothing else for now. Or call it
"yield" and rename "thread-yield" to "yield" on the concurrency
branch.
[Message part 2 (text/html, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Thu, 17 Oct 2013 15:04:01 GMT)
Full text and
rfc822 format available.
Message #197 received at 15045 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Here's the solution you requested. Let me know that it's good to
install.
diff --git a/ChangeLog b/ChangeLog
index a755b5c..8b58ebc 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,15 @@
+2013-10-17 Barry O'Reilly <gundaetiapo <at> gmail.com>
+
+ Change how input-pending-p checks for timers to run. Its default
+ changes from checking to not checking. Its new check-timers param
+ allows for prior behavior. (Bug#15045).
+ * src/keyboard.c (Finput_pending_p): Accept optional check-timers
+ param.
+ * lisp/subr.el (sit-for): Call (input-pending-p t) so as to behave
+ as before.
+ * test/automated/timer-tests.el: New file. Tests that (sit-for 0)
+ allows another timer to run.
+
2013-10-13 Glenn Morris <rgm <at> gnu.org>
* configure.ac [alpha]: Explicit error in non-ELF case. (Bug#15601)
diff --git a/etc/NEWS b/etc/NEWS
index ddb9a9f..06372f9 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -611,6 +611,9 @@ low-level libraries gfilenotify.c, inotify.c or
w32notify.c.
^L
* Incompatible Lisp Changes in Emacs 24.4
+** `(input-pending-p)' no longer runs other timers which are ready to
+run. The new optional CHECK-TIMERS param allows for the prior behavior.
+
** `defvar' and `defcustom' in a let-binding affect the "external" default.
** The syntax of ?» and ?« is now punctuation instead of matched parens.
diff --git a/lisp/subr.el b/lisp/subr.el
index 0d03e9a..952b9b6 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -2222,7 +2222,7 @@ floating point support."
(noninteractive
(sleep-for seconds)
t)
- ((input-pending-p)
+ ((input-pending-p t)
nil)
((<= seconds 0)
(or nodisp (redisplay)))
diff --git a/src/keyboard.c b/src/keyboard.c
index e0cd4d4..7f8692a 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -9947,12 +9947,13 @@ requeued_events_pending_p (void)
return (!NILP (Vunread_command_events));
}
-
-DEFUN ("input-pending-p", Finput_pending_p, Sinput_pending_p, 0, 0, 0,
+DEFUN ("input-pending-p", Finput_pending_p, Sinput_pending_p, 0, 1, 0,
doc: /* Return t if command input is currently available with no
wait.
Actually, the value is nil only if we can be sure that no input is
available;
-if there is a doubt, the value is t. */)
- (void)
+if there is a doubt, the value is t.
+
+If CHECK-TIMERS is non-nil, timers that are ready to run will do so. */)
+ (Lisp_Object check_timers)
{
if (!NILP (Vunread_command_events)
|| !NILP (Vunread_post_input_method_events)
@@ -9962,8 +9963,9 @@ if there is a doubt, the value is t. */)
/* Process non-user-visible events (Bug#10195). */
process_special_events ();
- return (get_input_pending (READABLE_EVENTS_DO_TIMERS_NOW
- | READABLE_EVENTS_FILTER_EVENTS)
+ return (get_input_pending ((EQ (check_timers, Qnil)
+ ? 0 : READABLE_EVENTS_DO_TIMERS_NOW)
+ | READABLE_EVENTS_FILTER_EVENTS)
? Qt : Qnil);
}
diff --git a/test/automated/timer-tests.el b/test/automated/timer-tests.el
new file mode 100644
index 0000000..b4cb1e6
--- /dev/null
+++ b/test/automated/timer-tests.el
@@ -0,0 +1,42 @@
+;;; timer-tests.el --- tests for timers -*- coding: utf-8;
lexical-binding:t -*-
+
+;; Copyright (C) 2013 Free Software Foundation, Inc.
+
+;; This file is part of GNU Emacs.
+
+;; This program is free software: you can redistribute it and/or
+;; modify it under the terms of the GNU General Public License as
+;; published by the Free Software Foundation, either version 3 of the
+;; License, or (at your option) any later version.
+;;
+;; This program is distributed in the hope that it will be useful, but
+;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+;; General Public License for more details.
+;;
+;; You should have received a copy of the GNU General Public License
+;; along with this program. If not, see `http://www.gnu.org/licenses/'.
+
+;;; Commentary:
+
+;;; Code:
+
+(require 'ert)
+
+(ert-deftest timer-tests-sit-for ()
+ (let ((timer-ran nil)
+ (timeout (time-add (current-time)
+ '(0 10 0 0)))
+ ;; Want sit-for behavior when interactive
+ (noninteractive nil))
+ (run-at-time '(0 1 0 0)
+ nil
+ (lambda ()
+ (setq timer-ran t)))
+ (while (not timer-ran)
+ (should (time-less-p (current-time)
+ timeout))
+ (sit-for 0 t))))
+
+;;; timer-tests.el ends here
+
[Message part 2 (text/html, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Thu, 17 Oct 2013 18:20:02 GMT)
Full text and
rfc822 format available.
Message #200 received at 15045 <at> debbugs.gnu.org (full text, mbox):
> Here's the solution you requested. Let me know that it's good to
> install.
Looks fine. See comments below.
> + Change how input-pending-p checks for timers to run. Its default
> + changes from checking to not checking. Its new check-timers param
> + allows for prior behavior.
Good.
"Don't run timers in input-pending-p" would have sufficed.
> +** `(input-pending-p)' no longer runs other timers which are ready to
> +run. The new optional CHECK-TIMERS param allows for the prior behavior.
Please use 2 spaces after a full-stop (see sentence-end-double-space).
> + return (get_input_pending ((EQ (check_timers, Qnil)
EQ (check_timers, Qnil) => NILP (check_timers)
> +;;; timer-tests.el --- tests for timers -*- coding: utf-8;
> lexical-binding:t -*-
The "coding:utf-8" is redundant nowadays.
> +(require 'ert)
IIUC this `require' is also redundant.
> +(ert-deftest timer-tests-sit-for ()
> + (let ((timer-ran nil)
> + (timeout (time-add (current-time)
> + '(0 10 0 0)))
> + ;; Want sit-for behavior when interactive
> + (noninteractive nil))
> + (run-at-time '(0 1 0 0)
> + nil
> + (lambda ()
> + (setq timer-ran t)))
> + (while (not timer-ran)
> + (should (time-less-p (current-time)
> + timeout))
> + (sit-for 0 t))))
I think there's a race-condition, here:
- let's say we're at time < timeout.
- we run sit-for, which does not run the timer since we're still <timeout.
- time advances to > timeout.
- we check (should (time-less-p (current-time) timeout))
- we complain unjustly.
I.e. we should test, after running sit-for, that the "current time before
running sit-for" was < timeout.
Stefan
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Thu, 17 Oct 2013 20:02:02 GMT)
Full text and
rfc822 format available.
Message #203 received at 15045 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
> I think there's a race-condition, here:
> - let's say we're at time < timeout.
> - we run sit-for, which does not run the timer since we're still <timeout.
> - time advances to > timeout.
> - we check (should (time-less-p (current-time) timeout))
> - we complain unjustly.
This statement isn't right:
> - we run sit-for, which does not run the timer since we're still <timeout.
If sit-for doesn't run the timer at any time in T+1s to T+10s, then
it's supposed to fail.
However, reconsidering the test, I don't think the timing is
necessary, so I've simplified it.
diff --git a/ChangeLog b/ChangeLog
index a755b5c..553abe2 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,14 @@
+2013-10-17 Barry O'Reilly <gundaetiapo <at> gmail.com>
+
+ Don't run timers in input-pending-p. Its new check-timers param
+ provides the prior behavior. (Bug#15045).
+ * src/keyboard.c (Finput_pending_p): Accept optional check-timers
+ param.
+ * lisp/subr.el (sit-for): Call (input-pending-p t) so as to behave
+ as before.
+ * test/automated/timer-tests.el: New file. Tests that (sit-for 0)
+ allows another timer to run.
+
2013-10-13 Glenn Morris <rgm <at> gnu.org>
* configure.ac [alpha]: Explicit error in non-ELF case. (Bug#15601)
diff --git a/etc/NEWS b/etc/NEWS
index ddb9a9f..963c372 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -611,6 +611,9 @@ low-level libraries gfilenotify.c, inotify.c or
w32notify.c.
* Incompatible Lisp Changes in Emacs 24.4
+** `(input-pending-p)' no longer runs other timers which are ready to
+run. The new optional CHECK-TIMERS param allows for the prior behavior.
+
** `defvar' and `defcustom' in a let-binding affect the "external" default.
** The syntax of ?» and ?« is now punctuation instead of matched parens.
diff --git a/lisp/subr.el b/lisp/subr.el
index 0d03e9a..952b9b6 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -2222,7 +2222,7 @@ floating point support."
(noninteractive
(sleep-for seconds)
t)
- ((input-pending-p)
+ ((input-pending-p t)
nil)
((<= seconds 0)
(or nodisp (redisplay)))
diff --git a/src/keyboard.c b/src/keyboard.c
index bb8fefa..85a1ce7 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -9947,12 +9947,13 @@ requeued_events_pending_p (void)
return (!NILP (Vunread_command_events));
}
-
-DEFUN ("input-pending-p", Finput_pending_p, Sinput_pending_p, 0, 0, 0,
+DEFUN ("input-pending-p", Finput_pending_p, Sinput_pending_p, 0, 1, 0,
doc: /* Return t if command input is currently available with no
wait.
Actually, the value is nil only if we can be sure that no input is
available;
-if there is a doubt, the value is t. */)
- (void)
+if there is a doubt, the value is t.
+
+If CHECK-TIMERS is non-nil, timers that are ready to run will do so. */)
+ (Lisp_Object check_timers)
{
if (!NILP (Vunread_command_events)
|| !NILP (Vunread_post_input_method_events)
@@ -9962,8 +9963,9 @@ if there is a doubt, the value is t. */)
/* Process non-user-visible events (Bug#10195). */
process_special_events ();
- return (get_input_pending (READABLE_EVENTS_DO_TIMERS_NOW
- | READABLE_EVENTS_FILTER_EVENTS)
+ return (get_input_pending ((NILP (check_timers)
+ ? 0 : READABLE_EVENTS_DO_TIMERS_NOW)
+ | READABLE_EVENTS_FILTER_EVENTS)
? Qt : Qnil);
}
diff --git a/test/automated/timer-tests.el b/test/automated/timer-tests.el
new file mode 100644
index 0000000..71a9b96
--- /dev/null
+++ b/test/automated/timer-tests.el
@@ -0,0 +1,38 @@
+;;; timer-tests.el --- tests for timers -*- lexical-binding:t -*-
+
+;; Copyright (C) 2013 Free Software Foundation, Inc.
+
+;; This file is part of GNU Emacs.
+
+;; This program is free software: you can redistribute it and/or
+;; modify it under the terms of the GNU General Public License as
+;; published by the Free Software Foundation, either version 3 of the
+;; License, or (at your option) any later version.
+;;
+;; This program is distributed in the hope that it will be useful, but
+;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+;; General Public License for more details.
+;;
+;; You should have received a copy of the GNU General Public License
+;; along with this program. If not, see `http://www.gnu.org/licenses/'.
+
+;;; Commentary:
+
+;;; Code:
+
+(ert-deftest timer-tests-sit-for ()
+ (let ((timer-ran nil)
+ ;; Want sit-for behavior when interactive
+ (noninteractive nil))
+ (run-at-time '(0 0 0 0)
+ nil
+ (lambda () (setq timer-ran t)))
+ ;; The test assumes run-at-time didn't take the liberty of firing
+ ;; the timer, so assert the test's assumption
+ (should (not timer-ran))
+ (sit-for 0 t)
+ (should timer-ran)))
+
+;;; timer-tests.el ends here
+
[Message part 2 (text/html, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Fri, 18 Oct 2013 00:28:01 GMT)
Full text and
rfc822 format available.
Message #206 received at 15045 <at> debbugs.gnu.org (full text, mbox):
> However, reconsidering the test, I don't think the timing is
> necessary, so I've simplified it.
Looks good, please install,
Stefan
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Fri, 18 Oct 2013 14:04:02 GMT)
Full text and
rfc822 format available.
Message #209 received at 15045 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Rev 114704. Leaving this open because of Semantic use of
accept-process-output during an excursion.
[Message part 2 (text/html, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Fri, 25 Oct 2013 19:16:01 GMT)
Full text and
rfc822 format available.
Message #212 received at 15045 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
In terms of solving the issue of how Semantic calls
accept-process-output, here are two proposals:
1: Per David Engster:
> If you look in the `define-lex' macro, you see that it calls
> `semantic-throw-on-input' after each identified token. The problem is
> that it does not restore the cursor position before that, so I guess the
> fix is simply to change this call to
>
> (save-excursion
> (goto-char starting-position)
> (semantic-throw-on-input 'lex))
Here's a patch to implement the basic idea. My debugging indicates
accept-process-output can be called when lexing other buffers, so I
saved off the marker rather than just position.
That said, I have never personally witnessed point unexpectedly
redisplayed in another buffer -- only within the same buffer. That
could be a consequence of how it interacts with the
jit-lock-deferred-fontify idle timer. (I have confirmed with my debug
statements that Semantic is the outer timer and
jit-lock-deferred-fontify is the inner timer which redisplays.)
Please let me know if this fix is suitable.
diff --git a/lisp/cedet/semantic/fw.el b/lisp/cedet/semantic/fw.el
index 825cdc9..869d183 100644
--- a/lisp/cedet/semantic/fw.el
+++ b/lisp/cedet/semantic/fw.el
@@ -369,6 +369,8 @@ later installation should be done in MODE hook."
;;
(defvar semantic-current-input-throw-symbol nil
"The current throw symbol for `semantic-exit-on-input'.")
+(defvar semantic--on-input-start-marker nil
+ "The marker when starting a semantic-exit-on-input form.")
(defmacro semantic-exit-on-input (symbol &rest forms)
"Using SYMBOL as an argument to `throw', execute FORMS.
@@ -376,7 +378,8 @@ If FORMS includes a call to `semantic-throw-on-input',
then
if a user presses any key during execution, this form macro
will exit with the value passed to `semantic-throw-on-input'.
If FORMS completes, then the return value is the same as `progn'."
- `(let ((semantic-current-input-throw-symbol ,symbol))
+ `(let ((semantic-current-input-throw-symbol ,symbol)
+ (semantic--on-input-start-marker (point-marker)))
(catch ,symbol
,@forms)))
(put 'semantic-exit-on-input 'lisp-indent-function 1)
@@ -387,7 +390,16 @@ FROM is an indication of where this function is called
from as a value
to pass to `throw'. It is recommended to use the name of the function
calling this one."
`(when (and semantic-current-input-throw-symbol
- (or (input-pending-p) (accept-process-output)))
+ (or (input-pending-p)
+ (save-excursion
+ ;; Timers might run during accept-process-output.
+ ;; If they redisplay, point must be where the user
+ ;; expects. (Bug#15045)
+ (set-buffer (marker-buffer
+ semantic--on-input-start-marker))
+ (goto-char (marker-position
+ semantic--on-input-start-marker))
+ (accept-process-output))))
(throw semantic-current-input-throw-symbol ,from)))
^L
2: Extend accept-process-output to allow suppressing timer_check.
Actually, there's an input arg already for that, but only applicable
if the PROCESS arg is non nil.
[Message part 2 (text/html, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Thu, 14 Nov 2013 18:22:02 GMT)
Full text and
rfc822 format available.
Message #215 received at 15045 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Eric, David,
I would very much like to use Semantic without unwanted scrolling, so I'd
appreciate your feedback. How is the patch I submitted? What else do I need
to do or investigate?
Patch: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=15045#212
[Message part 2 (text/html, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Sat, 16 Nov 2013 04:16:02 GMT)
Full text and
rfc822 format available.
Message #218 received at 15045 <at> debbugs.gnu.org (full text, mbox):
> I would very much like to use Semantic without unwanted scrolling, so I'd
> appreciate your feedback. How is the patch I submitted? What else do I need
> to do or investigate?
> Patch: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=15045#212
The patch is a bit "ugly" (in the sense that it's a bit too ad-hoc), but
it's probably what it takes (tho maybe semantic-exit-on-input could be
replaced by while-no-input, which does not suffer from this problem,
but it's not a simple replacement).
But the main problem I see is that you already have a few "trivial"
patches installed, so this is summing up to something non-trivial which
requires copyright paperwork.
If that's OK with you, then, please fill the form below and send it to
the FSF as instructed, so they can send you the relevant paperwork.
Stefan
Please email the following information to assign <at> gnu.org, and we
will send you the assignment form for your past and future changes.
Please use your full legal name (in ASCII characters) as the subject
line of the message.
----------------------------------------------------------------------
REQUEST: SEND FORM FOR PAST AND FUTURE CHANGES
[What is the name of the program or package you're contributing to?]
Emacs
[Did you copy any files or text written by someone else in these changes?
Even if that material is free software, we need to know about it.]
[Do you have an employer who might have a basis to claim to own
your changes? Do you attend a school which might make such a claim?]
[For the copyright registration, what country are you a citizen of?]
[What year were you born?]
[Please write your email address here.]
[Please write your postal address here.]
[Which files have you changed so far, and which new files have you written
so far?]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Sat, 16 Nov 2013 04:56:02 GMT)
Full text and
rfc822 format available.
Message #221 received at 15045 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
> The patch is a bit "ugly" (in the sense that it's a bit too ad-hoc)
Yes, I feel the same way. What seems more appropriate is if there was
something like process-output-p which would indicate to Semantic that
process output is ready to accept, and thus that Semantic needs to
unwind out of its save-excursion form before allowing
accept-process-output. Does the while-no-input you mentioned exit in
that circumstance?
> But the main problem I see is that you already have a few "trivial"
> patches installed, so this is summing up to something non-trivial
> which requires copyright paperwork.
That's already done and you granted me commit privs to Emacs and
ELPA. Let me know if you want me to forward the docs again confirming
the processing is complete.
[Message part 2 (text/html, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Sat, 16 Nov 2013 17:38:02 GMT)
Full text and
rfc822 format available.
Message #224 received at 15045 <at> debbugs.gnu.org (full text, mbox):
> accept-process-output. Does the while-no-input you mentioned exit in
> that circumstance?
Inside a while-no-input, if the user hits a key (or clicks), then
a `throw' is executed that exits the while-no-input.
>> But the main problem I see is that you already have a few "trivial"
>> patches installed, so this is summing up to something non-trivial
>> which requires copyright paperwork.
> That's already done and you granted me commit privs to Emacs and
> ELPA. Let me know if you want me to forward the docs again confirming
> the processing is complete.
Oh, sorry, I didn't look up the copyright.list properly. Then please
go ahead and install your change.
Stefan
Reply sent
to
Barry OReilly <gundaetiapo <at> gmail.com>:
You have taken responsibility.
(Sat, 16 Nov 2013 20:34:01 GMT)
Full text and
rfc822 format available.
Notification sent
to
Barry OReilly <gundaetiapo <at> gmail.com>:
bug acknowledged by developer.
(Sat, 16 Nov 2013 20:34:02 GMT)
Full text and
rfc822 format available.
Message #229 received at 15045-done <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
> Inside a while-no-input, if the user hits a key (or clicks), then a
> `throw' is executed that exits the while-no-input.
I thought input-pending-p covers that base. If we use while-no-input,
don't we still need to use accept-process-output, thus not solving the
problem?
> Then please go ahead and install your change.
Thanks, rev 115123. Closing.
[Message part 2 (text/html, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#15045; Package
emacs.
(Sat, 16 Nov 2013 21:30:02 GMT)
Full text and
rfc822 format available.
Message #232 received at 15045-done <at> debbugs.gnu.org (full text, mbox):
>> Inside a while-no-input, if the user hits a key (or clicks), then a
>> `throw' is executed that exits the while-no-input.
> I thought input-pending-p covers that base.
The difference is that input-pending-p does polling (you have to keep
calling input-pending-p), whereas while-no-input is interrupt based (it
will stop whatever you're doing when the key comes in).
> If we use while-no-input, don't we still need to use
> accept-process-output, thus not solving the problem?
Might very well be: while-no-input does not by-itself let processes run.
I don't know what the CEDET code does, so I just mentioned
while-no-input as something related, but indeed it might not solve
our problem.
>> Then please go ahead and install your change.
> Thanks, rev 115123. Closing.
Great, thank you,
Stefan
bug archived.
Request was from
Debbugs Internal Request <help-debbugs <at> gnu.org>
to
internal_control <at> debbugs.gnu.org.
(Sun, 15 Dec 2013 12:24:03 GMT)
Full text and
rfc822 format available.
This bug report was last modified 10 years and 146 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.