* bug#12447: 24.1.50; Stuck in garbage collection on OS X @ 2012-09-14 21:08 Harald Hanche-Olsen 2012-09-15 9:55 ` Jan Djärv 2012-09-16 9:15 ` Dmitry Gutov 0 siblings, 2 replies; 41+ messages in thread From: Harald Hanche-Olsen @ 2012-09-14 21:08 UTC (permalink / raw) To: 12447 This concerns emacs built with --with-ns from trunk on OS X. To start with the symptom: Emacs seems to freeze, and spends a lot of CPU time. Taking a sample with Activity monitor appears to indicate a very deeply nested mark_object calls; from this I conclude that the problem appears to happen in GC. How to trigger the symptom: I always got it by trying to send an email message with a large attachment. My mail client of choice is Mew, but I suspect that any activity triggering garbage collection will also trigger the bug. After bisecting, I come to the conclusion that this revision is to blame: revno: 109470 committer: Jan D. <jan.h.d@swipnet.se> branch nick: trunk timestamp: Mon 2012-08-06 18:09:02 +0200 message: * keyboard.c (timer_check_2): Add break so timer_check returns next timeout. That revision only adds a single line of code. I also find that the bug disappears if I patch the current tip of trunk (revision 110013) as follows, which undoes revision 109470: === modified file 'src/keyboard.c' --- src/keyboard.c 2012-09-13 02:21:28 +0000 +++ src/keyboard.c 2012-09-14 19:12:15 +0000 @@ -4484,7 +4484,6 @@ } nexttime = make_emacs_time (0, 0); - break; } else /* When we encounter a timer that is still waiting, I suppose the break was added for a good reason, so this is probably not the right thing. But it does cure the present problem. (This bug is also discussed on the emacs-devel list, in the thread titled "Emacs seems awfully unstable on OS X lately".) There seems to be a second bug, introduced later, that causes crashes (as opposed to hangs). I mention this because the second bug could interfere with attempts to work on this one. I will endeavour to track down the other bug as well. - Harald ^ permalink raw reply [flat|nested] 41+ messages in thread
* bug#12447: 24.1.50; Stuck in garbage collection on OS X 2012-09-14 21:08 bug#12447: 24.1.50; Stuck in garbage collection on OS X Harald Hanche-Olsen @ 2012-09-15 9:55 ` Jan Djärv 2012-09-15 11:07 ` Harald Hanche-Olsen 2012-09-16 9:15 ` Dmitry Gutov 1 sibling, 1 reply; 41+ messages in thread From: Jan Djärv @ 2012-09-15 9:55 UTC (permalink / raw) To: Harald Hanche-Olsen; +Cc: 12447 Hello. I think this is probably the same bug as in #12326. I.e. a timer gets added over and over again. Please consider how to fix the original problem as described in http://debbugs.gnu.org/cgi/bugreport.cgi?bug=12326#11 before removing the break statement. Jan D. 14 sep 2012 kl. 23:08 skrev Harald Hanche-Olsen <hanche@math.ntnu.no>: > This concerns emacs built with --with-ns from trunk on OS X. > > To start with the symptom: Emacs seems to freeze, and spends a lot of > CPU time. Taking a sample with Activity monitor appears to indicate a > very deeply nested mark_object calls; from this I conclude that the > problem appears to happen in GC. > > How to trigger the symptom: I always got it by trying to send an email > message with a large attachment. My mail client of choice is Mew, but > I suspect that any activity triggering garbage collection will also > trigger the bug. > > After bisecting, I come to the conclusion that this revision is to > blame: > > revno: 109470 > committer: Jan D. <jan.h.d@swipnet.se> > branch nick: trunk > timestamp: Mon 2012-08-06 18:09:02 +0200 > message: > * keyboard.c (timer_check_2): Add break so timer_check returns next timeout. > > That revision only adds a single line of code. I also find that the > bug disappears if I patch the current tip of trunk (revision 110013) > as follows, which undoes revision 109470: > > === modified file 'src/keyboard.c' > --- src/keyboard.c 2012-09-13 02:21:28 +0000 > +++ src/keyboard.c 2012-09-14 19:12:15 +0000 > @@ -4484,7 +4484,6 @@ > } > > nexttime = make_emacs_time (0, 0); > - break; > } > else > /* When we encounter a timer that is still waiting, > > > I suppose the break was added for a good reason, so this is probably > not the right thing. But it does cure the present problem. > > (This bug is also discussed on the emacs-devel list, in the thread > titled "Emacs seems awfully unstable on OS X lately".) > > There seems to be a second bug, introduced later, that causes crashes > (as opposed to hangs). I mention this because the second bug could > interfere with attempts to work on this one. I will endeavour to track > down the other bug as well. > > - Harald ^ permalink raw reply [flat|nested] 41+ messages in thread
* bug#12447: 24.1.50; Stuck in garbage collection on OS X 2012-09-15 9:55 ` Jan Djärv @ 2012-09-15 11:07 ` Harald Hanche-Olsen 2012-09-15 12:31 ` Eli Zaretskii 2012-09-15 18:59 ` Jan Djärv 0 siblings, 2 replies; 41+ messages in thread From: Harald Hanche-Olsen @ 2012-09-15 11:07 UTC (permalink / raw) To: jan.h.d; +Cc: 12447 [Jan Djärv <jan.h.d@swipnet.se> (2012-09-15 09:55:55 UTC)] > I think this is probably the same bug as in #12326. I.e. a timer gets added over and over again. > Please consider how to fix the original problem as described in http://debbugs.gnu.org/cgi/bugreport.cgi?bug=12326#11 before removing the break statement. Thanks for the pointer. There are certainly timers involved in the code that runs when I get the hangs; I'll review it and see if that is where the problem lies. However, I am not sure exactly what to look for. Or, more precisely, what is the undefined behaviour in js2-mode that you complain about it bug 12326? On a side note, I see that only a few timer functions are described in the elisp manual. It appears one has to read timer.el to find out how timers work at a lower level. Or did I miss something? - Harald ^ permalink raw reply [flat|nested] 41+ messages in thread
* bug#12447: 24.1.50; Stuck in garbage collection on OS X 2012-09-15 11:07 ` Harald Hanche-Olsen @ 2012-09-15 12:31 ` Eli Zaretskii 2012-09-15 13:19 ` Harald Hanche-Olsen 2012-09-15 18:59 ` Jan Djärv 1 sibling, 1 reply; 41+ messages in thread From: Eli Zaretskii @ 2012-09-15 12:31 UTC (permalink / raw) To: Harald Hanche-Olsen; +Cc: 12447 > Date: Sat, 15 Sep 2012 13:07:10 +0200 (CEST) > From: Harald Hanche-Olsen <hanche@math.ntnu.no> > Cc: 12447@debbugs.gnu.org > > It appears one has to read timer.el to find out how timers work at a lower level. Or did I miss something? No, the real story of how timers work is in process.c. See the function wait_reading_process_output there, which runs the timers via the call to timer_check around line 4444; time_check and its subroutines are in keyboard.c. timer.el is just the Lisp interface to that code, used to create, delete, and manage timer objects. ^ permalink raw reply [flat|nested] 41+ messages in thread
* bug#12447: 24.1.50; Stuck in garbage collection on OS X 2012-09-15 12:31 ` Eli Zaretskii @ 2012-09-15 13:19 ` Harald Hanche-Olsen 2012-09-15 13:56 ` Eli Zaretskii 0 siblings, 1 reply; 41+ messages in thread From: Harald Hanche-Olsen @ 2012-09-15 13:19 UTC (permalink / raw) To: eliz; +Cc: 12447 [Eli Zaretskii <eliz@gnu.org> (2012-09-15 12:31:50 UTC)] > No, the real story of how timers work is in process.c. See the > function wait_reading_process_output there, which runs the timers via > the call to timer_check around line 4444; time_check and its > subroutines are in keyboard.c. > > timer.el is just the Lisp interface to that code, used to create, > delete, and manage timer objects. Okay, I'll study it; but it seems ludicrous that you need to read C code in order to use timers from elisp without causing emacs to hang. I almost feel like filing a documentation bug report. But I'll try to gain a better understanding of the issue before considering that. - Harald ^ permalink raw reply [flat|nested] 41+ messages in thread
* bug#12447: 24.1.50; Stuck in garbage collection on OS X 2012-09-15 13:19 ` Harald Hanche-Olsen @ 2012-09-15 13:56 ` Eli Zaretskii 2012-09-15 14:23 ` Harald Hanche-Olsen 0 siblings, 1 reply; 41+ messages in thread From: Eli Zaretskii @ 2012-09-15 13:56 UTC (permalink / raw) To: Harald Hanche-Olsen; +Cc: 12447 > Date: Sat, 15 Sep 2012 15:19:09 +0200 (CEST) > Cc: jan.h.d@swipnet.se, 12447@debbugs.gnu.org > From: Harald Hanche-Olsen <hanche@math.ntnu.no> > > [Eli Zaretskii <eliz@gnu.org> (2012-09-15 12:31:50 UTC)] > > > No, the real story of how timers work is in process.c. See the > > function wait_reading_process_output there, which runs the timers via > > the call to timer_check around line 4444; time_check and its > > subroutines are in keyboard.c. > > > > timer.el is just the Lisp interface to that code, used to create, > > delete, and manage timer objects. > > Okay, I'll study it; but it seems ludicrous that you need to read C > code in order to use timers from elisp without causing emacs to hang. ??? AFAIU, you are trying to track a bug, not use the timers. You specifically asked about "how the timers work at a lower level". Using timers does not involve any reading of C code; just read the "Timers" and "Idle timers" nodes of the ELisp manual, and Bob's your uncle. No low-level knowledge is needed just for using timers. ^ permalink raw reply [flat|nested] 41+ messages in thread
* bug#12447: 24.1.50; Stuck in garbage collection on OS X 2012-09-15 13:56 ` Eli Zaretskii @ 2012-09-15 14:23 ` Harald Hanche-Olsen 2012-09-15 14:37 ` Eli Zaretskii 0 siblings, 1 reply; 41+ messages in thread From: Harald Hanche-Olsen @ 2012-09-15 14:23 UTC (permalink / raw) To: eliz; +Cc: 12447 [Eli Zaretskii <eliz@gnu.org> (2012-09-15 13:56:14 UTC)] > ??? AFAIU, you are trying to track a bug, not use the timers. You > specifically asked about "how the timers work at a lower level". > > Using timers does not involve any reading of C code; just read the > "Timers" and "Idle timers" nodes of the ELisp manual, and Bob's your > uncle. No low-level knowledge is needed just for using timers. Ah. I was forming my impression by looking at the code quoted in the discussion of bug#12326, which seems to be using lower level stuff. So I thought I had to understand that. But the timer code in mew seems to only use the interface described in the elisp manual. But yes, I am chasing a bug, not trying to use timers. Which seems to require understanding the underlying mechanism, as otherwise I have no idea what I need to be looking for. After all, the code works fine most of the time, but revision 109470 broke it for some strange reason. And this seems to imply that you do need to understand the lower levels in order to use timers safely. Which is bad, if true. For example: Is it okay to set a timer in a timer callback? Example code in the "Idle timers" node in the elisp manual seems to say so. Thanks for your patience with my many misunderstandings. I want to track this problem down, but I am still fumbling in the dark. - Harald ^ permalink raw reply [flat|nested] 41+ messages in thread
* bug#12447: 24.1.50; Stuck in garbage collection on OS X 2012-09-15 14:23 ` Harald Hanche-Olsen @ 2012-09-15 14:37 ` Eli Zaretskii 0 siblings, 0 replies; 41+ messages in thread From: Eli Zaretskii @ 2012-09-15 14:37 UTC (permalink / raw) To: Harald Hanche-Olsen; +Cc: 12447 > Date: Sat, 15 Sep 2012 16:23:53 +0200 (CEST) > Cc: jan.h.d@swipnet.se, 12447@debbugs.gnu.org > From: Harald Hanche-Olsen <hanche@math.ntnu.no> > > For example: Is it okay to set a timer in a timer callback? I don't see why not. Setting up a timer just creates a Lisp object and adds it to the list of timers. The timer will be run the next time the low-level mechanism, which is part of the Emacs exec loop, determines that it's ripe. > Thanks for your patience with my many misunderstandings. I want to > track this problem down, but I am still fumbling in the dark. You are welcome. ^ permalink raw reply [flat|nested] 41+ messages in thread
* bug#12447: 24.1.50; Stuck in garbage collection on OS X 2012-09-15 11:07 ` Harald Hanche-Olsen 2012-09-15 12:31 ` Eli Zaretskii @ 2012-09-15 18:59 ` Jan Djärv 1 sibling, 0 replies; 41+ messages in thread From: Jan Djärv @ 2012-09-15 18:59 UTC (permalink / raw) To: Harald Hanche-Olsen; +Cc: 12447 15 sep 2012 kl. 13:07 skrev Harald Hanche-Olsen <hanche@math.ntnu.no>: > [Jan Djärv <jan.h.d@swipnet.se> (2012-09-15 09:55:55 UTC)] > >> I think this is probably the same bug as in #12326. I.e. a timer gets added over and over again. >> Please consider how to fix the original problem as described in http://debbugs.gnu.org/cgi/bugreport.cgi?bug=12326#11 before removing the break statement. > > Thanks for the pointer. There are certainly timers involved in the code that runs when I get the hangs; I'll review it and see if that is where the problem lies. > > However, I am not sure exactly what to look for. Or, more precisely, what is the undefined behaviour in js2-mode that you complain about it bug 12326? > The undefined behaviour was before adding the break, timers could run at a time in the future that depended on other timers. The defined behaviour is with the break, timers run when they should. I suspect there is some bug in the C code when adding a timer from within a timer callback. > On a side note, I see that only a few timer functions are described in the elisp manual. It appears one has to read timer.el to find out how timers work at a lower level. Or did I miss something? As Eli has answered, the timers are handeled in C as a list sorted by time when the timers are to be run. I.e. the next timer to run is at the head of the list. Jan D. ^ permalink raw reply [flat|nested] 41+ messages in thread
* bug#12447: 24.1.50; Stuck in garbage collection on OS X 2012-09-14 21:08 bug#12447: 24.1.50; Stuck in garbage collection on OS X Harald Hanche-Olsen 2012-09-15 9:55 ` Jan Djärv @ 2012-09-16 9:15 ` Dmitry Gutov 2012-09-16 10:31 ` Eli Zaretskii 1 sibling, 1 reply; 41+ messages in thread From: Dmitry Gutov @ 2012-09-16 9:15 UTC (permalink / raw) To: eliz; +Cc: 12447, hanche Eli Zaretskii <eliz@gnu.org> writes: >> Date: Sat, 15 Sep 2012 16:23:53 +0200 (CEST) >> Cc: jan.h.d@swipnet.se, 12447@debbugs.gnu.org >> From: Harald Hanche-Olsen <hanche@math.ntnu.no> >> >> For example: Is it okay to set a timer in a timer callback? > > I don't see why not. Setting up a timer just creates a Lisp object > and adds it to the list of timers. The timer will be run the next > time the low-level mechanism, which is part of the Emacs exec loop, > determines that it's ripe. In js2-mode's case, the problem is that 'run-with-idle-timer' makes the created timer run now, not the "next time". Here's an example: (defvar counter 0) (defun foo () (message (format "foo %s" counter)) (incf counter) (run-with-idle-timer 1 nil #'foo)) (foo) I'd expect that either timer would fire once every second (as long as I'm not touching my keyboard), or at least stop firing when I do touch my keyboard (seeing as otherwise Emacs is idle), but instead I just see the timer firing many times a second, the counter runs in the message area, and Emacs doesn't respond to any commands. ^ permalink raw reply [flat|nested] 41+ messages in thread
* bug#12447: 24.1.50; Stuck in garbage collection on OS X 2012-09-16 9:15 ` Dmitry Gutov @ 2012-09-16 10:31 ` Eli Zaretskii 2012-09-16 10:44 ` Dmitry Gutov 0 siblings, 1 reply; 41+ messages in thread From: Eli Zaretskii @ 2012-09-16 10:31 UTC (permalink / raw) To: Dmitry Gutov; +Cc: 12447, hanche > Date: Sun, 16 Sep 2012 13:15:52 +0400 > From: Dmitry Gutov <dgutov@yandex.ru> > CC: hanche@math.ntnu.no, 12447@debbugs.gnu.org > > Eli Zaretskii <eliz@gnu.org> writes: > > >> Date: Sat, 15 Sep 2012 16:23:53 +0200 (CEST) > >> Cc: jan.h.d@swipnet.se, 12447@debbugs.gnu.org > >> From: Harald Hanche-Olsen <hanche@math.ntnu.no> > >> > >> For example: Is it okay to set a timer in a timer callback? > > > > I don't see why not. Setting up a timer just creates a Lisp object > > and adds it to the list of timers. The timer will be run the next > > time the low-level mechanism, which is part of the Emacs exec loop, > > determines that it's ripe. > > In js2-mode's case, the problem is that 'run-with-idle-timer' makes the > created timer run now, not the "next time". Here's an example: > > (defvar counter 0) > > (defun foo () > (message (format "foo %s" counter)) > (incf counter) > (run-with-idle-timer 1 nil #'foo)) > > (foo) The code above does not run the timer, it just schedules it to run after at least 1 sec of idleness time. > I'd expect that either timer would fire once every second (as long as > I'm not touching my keyboard), or at least stop firing when I do touch > my keyboard (seeing as otherwise Emacs is idle), but instead I just see > the timer firing many times a second, the counter runs in the message > area, and Emacs doesn't respond to any commands. Then there's a bug, because an idle timer should only fire when there's no other input. If there's keyboard input, Emacs should process it first. I don't think the bug is related to the fact that the timer handler re-schedules itself. That is something many timers do. There's something else at work here, and that something is most probably on the C level. ^ permalink raw reply [flat|nested] 41+ messages in thread
* bug#12447: 24.1.50; Stuck in garbage collection on OS X 2012-09-16 10:31 ` Eli Zaretskii @ 2012-09-16 10:44 ` Dmitry Gutov 2012-09-16 11:53 ` Eli Zaretskii 0 siblings, 1 reply; 41+ messages in thread From: Dmitry Gutov @ 2012-09-16 10:44 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 12447, hanche On 16.09.2012 14:31, Eli Zaretskii wrote: >> Date: Sun, 16 Sep 2012 13:15:52 +0400 >> From: Dmitry Gutov <dgutov@yandex.ru> >> CC: hanche@math.ntnu.no, 12447@debbugs.gnu.org >> >> Eli Zaretskii <eliz@gnu.org> writes: >> >> >> Date: Sat, 15 Sep 2012 16:23:53 +0200 (CEST) >> >> Cc: jan.h.d@swipnet.se, 12447@debbugs.gnu.org >> >> From: Harald Hanche-Olsen <hanche@math.ntnu.no> >> >> >> >> For example: Is it okay to set a timer in a timer callback? >> > >> > I don't see why not. Setting up a timer just creates a Lisp object >> > and adds it to the list of timers. The timer will be run the next >> > time the low-level mechanism, which is part of the Emacs exec loop, >> > determines that it's ripe. >> >> In js2-mode's case, the problem is that 'run-with-idle-timer' makes the >> created timer run now, not the "next time". Here's an example: >> >> (defvar counter 0) >> >> (defun foo () >> (message (format "foo %s" counter)) >> (incf counter) >> (run-with-idle-timer 1 nil #'foo)) >> >> (foo) > > The code above does not run the timer, it just schedules it to run > after at least 1 sec of idleness time. Yes. And 1 second later, Emacs effectively freezes (while still continuing to show the increasing counter in the message area). >> I'd expect that either timer would fire once every second (as long as >> I'm not touching my keyboard), or at least stop firing when I do touch >> my keyboard (seeing as otherwise Emacs is idle), but instead I just see >> the timer firing many times a second, the counter runs in the message >> area, and Emacs doesn't respond to any commands. > > Then there's a bug, because an idle timer should only fire when > there's no other input. If there's keyboard input, Emacs should > process it first. > > I don't think the bug is related to the fact that the timer handler > re-schedules itself. That is something many timers do. There's > something else at work here, and that something is most probably on > the C level. Like I wrote in 12326, AFAICT, the problem is that timer_check_2 doesn't at any point check that Emacs is still idle. When run-with-idle-timer calls (timer-activate-when-idle timer t), the new timer is added to the list, timer_check_2 reaches is and runs it immediately because 'timer_idleness_start_time' still has the same value. ^ permalink raw reply [flat|nested] 41+ messages in thread
* bug#12447: 24.1.50; Stuck in garbage collection on OS X 2012-09-16 10:44 ` Dmitry Gutov @ 2012-09-16 11:53 ` Eli Zaretskii 2012-09-16 12:07 ` Dmitry Gutov 0 siblings, 1 reply; 41+ messages in thread From: Eli Zaretskii @ 2012-09-16 11:53 UTC (permalink / raw) To: Dmitry Gutov; +Cc: 12447, hanche > Date: Sun, 16 Sep 2012 14:44:30 +0400 > From: Dmitry Gutov <dgutov@yandex.ru> > CC: hanche@math.ntnu.no, 12447@debbugs.gnu.org > > Like I wrote in 12326, AFAICT, the problem is that timer_check_2 doesn't > at any point check that Emacs is still idle. When run-with-idle-timer > calls (timer-activate-when-idle timer t), the new timer is added to the > list, timer_check_2 reaches is and runs it immediately because > 'timer_idleness_start_time' still has the same value. If that is the problem, then perhaps having timer_check_2 work on a copy of the list would solve the problem. Did you try that? ^ permalink raw reply [flat|nested] 41+ messages in thread
* bug#12447: 24.1.50; Stuck in garbage collection on OS X 2012-09-16 11:53 ` Eli Zaretskii @ 2012-09-16 12:07 ` Dmitry Gutov 2012-09-16 12:39 ` Eli Zaretskii 0 siblings, 1 reply; 41+ messages in thread From: Dmitry Gutov @ 2012-09-16 12:07 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 12447, hanche On 16.09.2012 15:53, Eli Zaretskii wrote: >> Date: Sun, 16 Sep 2012 14:44:30 +0400 >> From: Dmitry Gutov <dgutov@yandex.ru> >> CC: hanche@math.ntnu.no, 12447@debbugs.gnu.org >> >> Like I wrote in 12326, AFAICT, the problem is that timer_check_2 doesn't >> at any point check that Emacs is still idle. When run-with-idle-timer >> calls (timer-activate-when-idle timer t), the new timer is added to the >> list, timer_check_2 reaches is and runs it immediately because >> 'timer_idleness_start_time' still has the same value. > > If that is the problem, then perhaps having timer_check_2 work on a > copy of the list would solve the problem. Did you try that? I'm no C programmer, so I didn't try to fix it in C code. How would I make a copy of a list there? But no, it probably won't: the "guilty" commit made timer_check_2 actually return 0 after a timer fires (keeping true to the comment above it), so a local copy would serve no purpose. Assuming it would've worked, though, wouldn't it make the second argument to 'timer-activate-when-idle' useless? ^ permalink raw reply [flat|nested] 41+ messages in thread
* bug#12447: 24.1.50; Stuck in garbage collection on OS X 2012-09-16 12:07 ` Dmitry Gutov @ 2012-09-16 12:39 ` Eli Zaretskii 2012-09-16 13:25 ` Dmitry Gutov 0 siblings, 1 reply; 41+ messages in thread From: Eli Zaretskii @ 2012-09-16 12:39 UTC (permalink / raw) To: Dmitry Gutov; +Cc: 12447, hanche > Date: Sun, 16 Sep 2012 16:07:07 +0400 > From: Dmitry Gutov <dgutov@yandex.ru> > CC: hanche@math.ntnu.no, 12447@debbugs.gnu.org > > >> Like I wrote in 12326, AFAICT, the problem is that timer_check_2 doesn't > >> at any point check that Emacs is still idle. When run-with-idle-timer > >> calls (timer-activate-when-idle timer t), the new timer is added to the > >> list, timer_check_2 reaches is and runs it immediately because > >> 'timer_idleness_start_time' still has the same value. > > > > If that is the problem, then perhaps having timer_check_2 work on a > > copy of the list would solve the problem. Did you try that? > > I'm no C programmer, so I didn't try to fix it in C code. How would I > make a copy of a list there? Using the Fcopy_sequence function, I'd think. Use it at the beginning of the function to set the value of 'idle_timers', instead of this line: idle_timers = Vtimer_idle_list; > But no, it probably won't: the "guilty" commit made timer_check_2 > actually return 0 after a timer fires (keeping true to the comment above > it), so a local copy would serve no purpose. timer_check_2 indeed returns, but then timer_check will call it again, because it continues calling timer_check_2 in a loop, until there's no ripe timer. > Assuming it would've worked, though, wouldn't it make the second > argument to 'timer-activate-when-idle' useless? It isn't useless, it just means a slight delay in the "now" part of its doc string. ^ permalink raw reply [flat|nested] 41+ messages in thread
* bug#12447: 24.1.50; Stuck in garbage collection on OS X 2012-09-16 12:39 ` Eli Zaretskii @ 2012-09-16 13:25 ` Dmitry Gutov 2012-09-16 13:47 ` Eli Zaretskii 0 siblings, 1 reply; 41+ messages in thread From: Dmitry Gutov @ 2012-09-16 13:25 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 12447, hanche On 16.09.2012 16:39, Eli Zaretskii wrote: >> Date: Sun, 16 Sep 2012 16:07:07 +0400 >> From: Dmitry Gutov <dgutov@yandex.ru> >> CC: hanche@math.ntnu.no, 12447@debbugs.gnu.org >> >>>> Like I wrote in 12326, AFAICT, the problem is that timer_check_2 doesn't >>>> at any point check that Emacs is still idle. When run-with-idle-timer >>>> calls (timer-activate-when-idle timer t), the new timer is added to the >>>> list, timer_check_2 reaches is and runs it immediately because >>>> 'timer_idleness_start_time' still has the same value. >>> >>> If that is the problem, then perhaps having timer_check_2 work on a >>> copy of the list would solve the problem. Did you try that? >> >> I'm no C programmer, so I didn't try to fix it in C code. How would I >> make a copy of a list there? > > Using the Fcopy_sequence function, I'd think. Use it at the beginning > of the function to set the value of 'idle_timers', instead of this > line: > > idle_timers = Vtimer_idle_list; Done that, recompiled, no difference in the example a sent previously. Exactly because, I think, of the control flow you describe below: >> But no, it probably won't: the "guilty" commit made timer_check_2 >> actually return 0 after a timer fires (keeping true to the comment above >> it), so a local copy would serve no purpose. > > timer_check_2 indeed returns, but then timer_check will call it again, > because it continues calling timer_check_2 in a loop, until there's no > ripe timer. Each time timer_check_2 is called, a new copy would be made from the idle timers list, so the newly created timer would be reached during the same call to 'timer_check'. ^ permalink raw reply [flat|nested] 41+ messages in thread
* bug#12447: 24.1.50; Stuck in garbage collection on OS X 2012-09-16 13:25 ` Dmitry Gutov @ 2012-09-16 13:47 ` Eli Zaretskii 2012-09-16 14:25 ` Dmitry Gutov 0 siblings, 1 reply; 41+ messages in thread From: Eli Zaretskii @ 2012-09-16 13:47 UTC (permalink / raw) To: Dmitry Gutov; +Cc: 12447, hanche > Date: Sun, 16 Sep 2012 17:25:35 +0400 > From: Dmitry Gutov <dgutov@yandex.ru> > CC: hanche@math.ntnu.no, 12447@debbugs.gnu.org > > > idle_timers = Vtimer_idle_list; > > Done that, recompiled, no difference in the example a sent previously. > Exactly because, I think, of the control flow you describe below: > > >> But no, it probably won't: the "guilty" commit made timer_check_2 > >> actually return 0 after a timer fires (keeping true to the comment above > >> it), so a local copy would serve no purpose. > > > > timer_check_2 indeed returns, but then timer_check will call it again, > > because it continues calling timer_check_2 in a loop, until there's no > > ripe timer. > > Each time timer_check_2 is called, a new copy would be made from the > idle timers list, so the newly created timer would be reached during the > same call to 'timer_check'. But the first time timer_check_2 encounters an un-ripe timer, or gets to the end of the list of timers, it returns a non-zero 'nexttime' value, which should break the loop in timer_check. Why isn't this happening? Perhaps the problem is that the value of 'difference' is not initialized: while (CONSP (timers) || CONSP (idle_timers)) { Lisp_Object timer = Qnil, idle_timer = Qnil; EMACS_TIME timer_time, idle_timer_time; EMACS_TIME difference; <<<<<<<<<<<<<<<<<<<<<<<<<<<< and then never set to any specific value, until here: else /* When we encounter a timer that is still waiting, return the amount of time to wait before it is ripe. */ { UNGCPRO; return difference; } which causes us return garbage, potentially zero, to timer_check. ^ permalink raw reply [flat|nested] 41+ messages in thread
* bug#12447: 24.1.50; Stuck in garbage collection on OS X 2012-09-16 13:47 ` Eli Zaretskii @ 2012-09-16 14:25 ` Dmitry Gutov 2012-09-16 14:54 ` Eli Zaretskii 0 siblings, 1 reply; 41+ messages in thread From: Dmitry Gutov @ 2012-09-16 14:25 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 12447, hanche On 16.09.2012 17:47, Eli Zaretskii wrote: >> Date: Sun, 16 Sep 2012 17:25:35 +0400 >> From: Dmitry Gutov <dgutov@yandex.ru> >> CC: hanche@math.ntnu.no, 12447@debbugs.gnu.org >> >>> idle_timers = Vtimer_idle_list; >> >> Done that, recompiled, no difference in the example a sent previously. >> Exactly because, I think, of the control flow you describe below: >> >>>> But no, it probably won't: the "guilty" commit made timer_check_2 >>>> actually return 0 after a timer fires (keeping true to the comment above >>>> it), so a local copy would serve no purpose. >>> >>> timer_check_2 indeed returns, but then timer_check will call it again, >>> because it continues calling timer_check_2 in a loop, until there's no >>> ripe timer. >> >> Each time timer_check_2 is called, a new copy would be made from the >> idle timers list, so the newly created timer would be reached during the >> same call to 'timer_check'. > > But the first time timer_check_2 encounters an un-ripe timer, or gets > to the end of the list of timers, it returns a non-zero 'nexttime' > value, which should break the loop in timer_check. Why isn't this > happening? Maybe because the new timer is at the top of the list, and it's considered ripe? > Perhaps the problem is that the value of 'difference' is not > initialized: > > while (CONSP (timers) || CONSP (idle_timers)) > { > Lisp_Object timer = Qnil, idle_timer = Qnil; > EMACS_TIME timer_time, idle_timer_time; > EMACS_TIME difference; <<<<<<<<<<<<<<<<<<<<<<<<<<<< > > and then never set to any specific value, until here: > > else > /* When we encounter a timer that is still waiting, > return the amount of time to wait before it is ripe. */ > { > UNGCPRO; > return difference; > } > > which causes us return garbage, potentially zero, to timer_check. It's assigned to, though. When we encounter a timer that's not yet ripe. ^ permalink raw reply [flat|nested] 41+ messages in thread
* bug#12447: 24.1.50; Stuck in garbage collection on OS X 2012-09-16 14:25 ` Dmitry Gutov @ 2012-09-16 14:54 ` Eli Zaretskii 2012-09-16 15:56 ` Dmitry Gutov 0 siblings, 1 reply; 41+ messages in thread From: Eli Zaretskii @ 2012-09-16 14:54 UTC (permalink / raw) To: Dmitry Gutov; +Cc: 12447, hanche > Date: Sun, 16 Sep 2012 18:25:50 +0400 > From: Dmitry Gutov <dgutov@yandex.ru> > CC: hanche@math.ntnu.no, 12447@debbugs.gnu.org > > > Perhaps the problem is that the value of 'difference' is not > > initialized: > > > > while (CONSP (timers) || CONSP (idle_timers)) > > { > > Lisp_Object timer = Qnil, idle_timer = Qnil; > > EMACS_TIME timer_time, idle_timer_time; > > EMACS_TIME difference; <<<<<<<<<<<<<<<<<<<<<<<<<<<< > > > > and then never set to any specific value, until here: > > > > else > > /* When we encounter a timer that is still waiting, > > return the amount of time to wait before it is ripe. */ > > { > > UNGCPRO; > > return difference; > > } > > > > which causes us return garbage, potentially zero, to timer_check. > > It's assigned to, though. When we encounter a timer that's not yet ripe. What if all of them are ripe? ^ permalink raw reply [flat|nested] 41+ messages in thread
* bug#12447: 24.1.50; Stuck in garbage collection on OS X 2012-09-16 14:54 ` Eli Zaretskii @ 2012-09-16 15:56 ` Dmitry Gutov 2012-09-18 15:05 ` Eli Zaretskii 0 siblings, 1 reply; 41+ messages in thread From: Dmitry Gutov @ 2012-09-16 15:56 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 12447, hanche On 16.09.2012 18:54, Eli Zaretskii wrote: >> Date: Sun, 16 Sep 2012 18:25:50 +0400 >> From: Dmitry Gutov <dgutov@yandex.ru> >> CC: hanche@math.ntnu.no, 12447@debbugs.gnu.org >> >>> Perhaps the problem is that the value of 'difference' is not >>> initialized: >>> >>> while (CONSP (timers) || CONSP (idle_timers)) >>> { >>> Lisp_Object timer = Qnil, idle_timer = Qnil; >>> EMACS_TIME timer_time, idle_timer_time; >>> EMACS_TIME difference; <<<<<<<<<<<<<<<<<<<<<<<<<<<< >>> >>> and then never set to any specific value, until here: >>> >>> else >>> /* When we encounter a timer that is still waiting, >>> return the amount of time to wait before it is ripe. */ >>> { >>> UNGCPRO; >>> return difference; >>> } >>> >>> which causes us return garbage, potentially zero, to timer_check. >> >> It's assigned to, though. When we encounter a timer that's not yet ripe. > > What if all of them are ripe? I don't see the problem. The first timer is ripe? Fire it and return 'nexttime'. Otherwise, return 'difference', which now has been assigned a value. If we've reached the end of the list, again return 'nexttime', which is initialized with invalid_emacs_time () at the beginning of timer_check_2. Anyway, I think the immediate problem is that the newly created timer can be considered ripe. ^ permalink raw reply [flat|nested] 41+ messages in thread
* bug#12447: 24.1.50; Stuck in garbage collection on OS X 2012-09-16 15:56 ` Dmitry Gutov @ 2012-09-18 15:05 ` Eli Zaretskii 2012-09-18 17:29 ` Jan Djärv 2012-09-19 0:27 ` Dmitry Gutov 0 siblings, 2 replies; 41+ messages in thread From: Eli Zaretskii @ 2012-09-18 15:05 UTC (permalink / raw) To: Dmitry Gutov, Jan Djärv; +Cc: 12447, hanche > Date: Sun, 16 Sep 2012 19:56:11 +0400 > From: Dmitry Gutov <dgutov@yandex.ru> > Cc: 12447@debbugs.gnu.org, hanche@math.ntnu.no > > On 16.09.2012 18:54, Eli Zaretskii wrote: > >> Date: Sun, 16 Sep 2012 18:25:50 +0400 > >> From: Dmitry Gutov <dgutov@yandex.ru> > >> CC: hanche@math.ntnu.no, 12447@debbugs.gnu.org > >> > >>> Perhaps the problem is that the value of 'difference' is not > >>> initialized: > >>> > >>> while (CONSP (timers) || CONSP (idle_timers)) > >>> { > >>> Lisp_Object timer = Qnil, idle_timer = Qnil; > >>> EMACS_TIME timer_time, idle_timer_time; > >>> EMACS_TIME difference; <<<<<<<<<<<<<<<<<<<<<<<<<<<< > >>> > >>> and then never set to any specific value, until here: > >>> > >>> else > >>> /* When we encounter a timer that is still waiting, > >>> return the amount of time to wait before it is ripe. */ > >>> { > >>> UNGCPRO; > >>> return difference; > >>> } > >>> > >>> which causes us return garbage, potentially zero, to timer_check. > >> > >> It's assigned to, though. When we encounter a timer that's not yet ripe. > > > > What if all of them are ripe? > > I don't see the problem. The first timer is ripe? Fire it and return > 'nexttime'. Otherwise, return 'difference', which now has been assigned > a value. > If we've reached the end of the list, again return 'nexttime', which is > initialized with invalid_emacs_time () at the beginning of timer_check_2. > > Anyway, I think the immediate problem is that the newly created timer > can be considered ripe. The patch below makes your simplified recipe, viz.: (defvar counter 0) (defun foo () (message (format "foo %s" counter)) (setq counter (1+ counter)) (run-with-idle-timer 1 nil #'foo)) (foo) "work" without locking up Emacs. "Work" in the sense that the timer is run and increments the counter, but keyboard input is still accepted, and causes 1-sec break in the idle timer invocation. What does NOT happen is the once-per-second invocation of the idle timer: as long as there's no other input, the idle timer runs much more frequently. But I think this is expected, since the call to run-with-idle-timer above explicitly asks to be run immediately. Can you see if these changes also make js2-mode work as expected? Jan, can you test whether this patch still keeps your two-timers recipe working? If it does, I think I should commit the changes below, because they avoid locking up Emacs by a timer that repeatedly reinvokes itself. === modified file 'src/keyboard.c' --- src/keyboard.c 2012-09-16 21:43:55 +0000 +++ src/keyboard.c 2012-09-18 14:58:24 +0000 @@ -4334,25 +4334,18 @@ decode_timer (Lisp_Object timer, EMACS_T should be done. */ static EMACS_TIME -timer_check_2 (void) +timer_check_2 (Lisp_Object timers, Lisp_Object idle_timers) { EMACS_TIME nexttime; EMACS_TIME now; EMACS_TIME idleness_now; - Lisp_Object timers, idle_timers, chosen_timer; - struct gcpro gcpro1, gcpro2, gcpro3; + Lisp_Object chosen_timer; + struct gcpro gcpro1; nexttime = invalid_emacs_time (); - /* Always consider the ordinary timers. */ - timers = Vtimer_list; - /* Consider the idle timers only if Emacs is idle. */ - if (EMACS_TIME_VALID_P (timer_idleness_start_time)) - idle_timers = Vtimer_idle_list; - else - idle_timers = Qnil; chosen_timer = Qnil; - GCPRO3 (timers, idle_timers, chosen_timer); + GCPRO1 (chosen_timer); /* First run the code that was delayed. */ while (CONSP (pending_funcalls)) @@ -4501,13 +4494,30 @@ EMACS_TIME timer_check (void) { EMACS_TIME nexttime; + Lisp_Object timers, idle_timers; + struct gcpro gcpro1, gcpro2; + + /* We use copies of the timers' lists to allow a timer to add itself + again, without locking up Emacs if the newly added timer is + already ripe when added. */ + + /* Always consider the ordinary timers. */ + timers = Fcopy_sequence (Vtimer_list); + /* Consider the idle timers only if Emacs is idle. */ + if (EMACS_TIME_VALID_P (timer_idleness_start_time)) + idle_timers = Fcopy_sequence (Vtimer_idle_list); + else + idle_timers = Qnil; + + GCPRO2 (timers, idle_timers); do { - nexttime = timer_check_2 (); + nexttime = timer_check_2 (timers, idle_timers); } while (EMACS_SECS (nexttime) == 0 && EMACS_NSECS (nexttime) == 0); + UNGCPRO; return nexttime; } ^ permalink raw reply [flat|nested] 41+ messages in thread
* bug#12447: 24.1.50; Stuck in garbage collection on OS X 2012-09-18 15:05 ` Eli Zaretskii @ 2012-09-18 17:29 ` Jan Djärv 2012-09-18 20:08 ` Eli Zaretskii 2012-09-19 0:27 ` Dmitry Gutov 1 sibling, 1 reply; 41+ messages in thread From: Jan Djärv @ 2012-09-18 17:29 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 12447, hanche, Dmitry Gutov Hello. 18 sep 2012 kl. 17:05 skrev Eli Zaretskii <eliz@gnu.org>: > The patch below makes your simplified recipe, viz.: > > (defvar counter 0) > > (defun foo () > (message (format "foo %s" counter)) > (setq counter (1+ counter)) > (run-with-idle-timer 1 nil #'foo)) > (foo) > > "work" without locking up Emacs. "Work" in the sense that the timer > is run and increments the counter, but keyboard input is still > accepted, and causes 1-sec break in the idle timer invocation. What > does NOT happen is the once-per-second invocation of the idle timer: > as long as there's no other input, the idle timer runs much more > frequently. But I think this is expected, since the call to > run-with-idle-timer above explicitly asks to be run immediately. > > Can you see if these changes also make js2-mode work as expected? > > Jan, can you test whether this patch still keeps your two-timers > recipe working? If it does, I think I should commit the changes > below, because they avoid locking up Emacs by a timer that repeatedly > reinvokes itself. It does. Jan D. ^ permalink raw reply [flat|nested] 41+ messages in thread
* bug#12447: 24.1.50; Stuck in garbage collection on OS X 2012-09-18 17:29 ` Jan Djärv @ 2012-09-18 20:08 ` Eli Zaretskii 0 siblings, 0 replies; 41+ messages in thread From: Eli Zaretskii @ 2012-09-18 20:08 UTC (permalink / raw) To: Jan Djärv; +Cc: 12447, hanche, dgutov > From: Jan Djärv <jan.h.d@swipnet.se> > Date: Tue, 18 Sep 2012 19:29:28 +0200 > Cc: Dmitry Gutov <dgutov@yandex.ru>, > 12447@debbugs.gnu.org, > hanche@math.ntnu.no > > > Jan, can you test whether this patch still keeps your two-timers > > recipe working? If it does, I think I should commit the changes > > below, because they avoid locking up Emacs by a timer that repeatedly > > reinvokes itself. > > > It does. Thanks. Assuming js2-mode also works, I will wait for a few days to hear possible objections and comments, and commit then. ^ permalink raw reply [flat|nested] 41+ messages in thread
* bug#12447: 24.1.50; Stuck in garbage collection on OS X 2012-09-18 15:05 ` Eli Zaretskii 2012-09-18 17:29 ` Jan Djärv @ 2012-09-19 0:27 ` Dmitry Gutov 2012-09-19 2:54 ` Eli Zaretskii 1 sibling, 1 reply; 41+ messages in thread From: Dmitry Gutov @ 2012-09-19 0:27 UTC (permalink / raw) To: Eli Zaretskii; +Cc: hanche, 12447 On 18.09.2012 19:05, Eli Zaretskii wrote: > The patch below makes your simplified recipe, viz.: > > (defvar counter 0) > > (defun foo () > (message (format "foo %s" counter)) > (setq counter (1+ counter)) > (run-with-idle-timer 1 nil #'foo)) > (foo) > > "work" without locking up Emacs. "Work" in the sense that the timer > is run and increments the counter, but keyboard input is still > accepted, and causes 1-sec break in the idle timer invocation. What > does NOT happen is the once-per-second invocation of the idle timer: > as long as there's no other input, the idle timer runs much more > frequently. But I think this is expected, since the call to > run-with-idle-timer above explicitly asks to be run immediately. I think this behavior makes sense, too. > Can you see if these changes also make js2-mode work as expected? They do, thank you. I'll keep the workaround, though, as it reportedly also fixes the long-standing OS X freeze bug. --Dmitry ^ permalink raw reply [flat|nested] 41+ messages in thread
* bug#12447: 24.1.50; Stuck in garbage collection on OS X 2012-09-19 0:27 ` Dmitry Gutov @ 2012-09-19 2:54 ` Eli Zaretskii 2012-09-19 7:41 ` Harald Hanche-Olsen 2012-09-19 9:54 ` Dmitry Gutov 0 siblings, 2 replies; 41+ messages in thread From: Eli Zaretskii @ 2012-09-19 2:54 UTC (permalink / raw) To: Dmitry Gutov; +Cc: hanche, 12447 > Date: Wed, 19 Sep 2012 04:27:07 +0400 > From: Dmitry Gutov <dgutov@yandex.ru> > CC: Jan Djärv <jan.h.d@swipnet.se>, > 12447@debbugs.gnu.org, hanche@math.ntnu.no > > > Can you see if these changes also make js2-mode work as expected? > > They do, thank you. Thanks for testing. > I'll keep the workaround, though, as it reportedly also fixes the > long-standing OS X freeze bug. What workaround is that? ^ permalink raw reply [flat|nested] 41+ messages in thread
* bug#12447: 24.1.50; Stuck in garbage collection on OS X 2012-09-19 2:54 ` Eli Zaretskii @ 2012-09-19 7:41 ` Harald Hanche-Olsen 2012-09-19 15:21 ` Eli Zaretskii 2012-09-19 9:54 ` Dmitry Gutov 1 sibling, 1 reply; 41+ messages in thread From: Harald Hanche-Olsen @ 2012-09-19 7:41 UTC (permalink / raw) To: eliz; +Cc: dgutov, 12447 [Eli Zaretskii <eliz@gnu.org> (2012-09-19 02:54:39 UTC)] > > Date: Wed, 19 Sep 2012 04:27:07 +0400 > > From: Dmitry Gutov <dgutov@yandex.ru> > > CC: Jan Djärv <jan.h.d@swipnet.se>, > > 12447@debbugs.gnu.org, hanche@math.ntnu.no > > > > I'll keep the workaround, though, as it reportedly also fixes the > > long-standing OS X freeze bug. > > What workaround is that? Perhaps he meant backing out of 109470. FWIW, I am now running with your patch applied, and 109470 reinstated, and sending messages with big attachments now works without a hitch. - Harald ^ permalink raw reply [flat|nested] 41+ messages in thread
* bug#12447: 24.1.50; Stuck in garbage collection on OS X 2012-09-19 7:41 ` Harald Hanche-Olsen @ 2012-09-19 15:21 ` Eli Zaretskii 0 siblings, 0 replies; 41+ messages in thread From: Eli Zaretskii @ 2012-09-19 15:21 UTC (permalink / raw) To: Harald Hanche-Olsen; +Cc: dgutov, 12447 > Date: Wed, 19 Sep 2012 09:41:10 +0200 (CEST) > Cc: dgutov@yandex.ru, jan.h.d@swipnet.se, 12447@debbugs.gnu.org > From: Harald Hanche-Olsen <hanche@math.ntnu.no> > > [Eli Zaretskii <eliz@gnu.org> (2012-09-19 02:54:39 UTC)] > > > > Date: Wed, 19 Sep 2012 04:27:07 +0400 > > > From: Dmitry Gutov <dgutov@yandex.ru> > > > CC: Jan Djärv <jan.h.d@swipnet.se>, > > > 12447@debbugs.gnu.org, hanche@math.ntnu.no > > > > > > I'll keep the workaround, though, as it reportedly also fixes the > > > long-standing OS X freeze bug. > > > > What workaround is that? > > Perhaps he meant backing out of 109470. I don't think it should be backed out. > FWIW, I am now running with your patch applied, and 109470 reinstated, > and sending messages with big attachments now works without a hitch. Thanks. ^ permalink raw reply [flat|nested] 41+ messages in thread
* bug#12447: 24.1.50; Stuck in garbage collection on OS X 2012-09-19 2:54 ` Eli Zaretskii 2012-09-19 7:41 ` Harald Hanche-Olsen @ 2012-09-19 9:54 ` Dmitry Gutov 2012-09-19 15:24 ` Eli Zaretskii 1 sibling, 1 reply; 41+ messages in thread From: Dmitry Gutov @ 2012-09-19 9:54 UTC (permalink / raw) To: Eli Zaretskii; +Cc: hanche, 12447 On 19.09.2012 6:54, Eli Zaretskii wrote: >> I'll keep the workaround, though, as it reportedly also fixes the >> long-standing OS X freeze bug. > > What workaround is that? The one I posted in #12326: calling timer-activate-when-idle with nil DONT-WAIT argument. https://github.com/mooz/js2-mode/commit/b02f4a0d72d0e2087038fe891e2580c4505415ef ^ permalink raw reply [flat|nested] 41+ messages in thread
* bug#12447: 24.1.50; Stuck in garbage collection on OS X 2012-09-19 9:54 ` Dmitry Gutov @ 2012-09-19 15:24 ` Eli Zaretskii 2012-09-19 16:21 ` Dmitry Gutov 0 siblings, 1 reply; 41+ messages in thread From: Eli Zaretskii @ 2012-09-19 15:24 UTC (permalink / raw) To: Dmitry Gutov; +Cc: hanche, 12447 > Date: Wed, 19 Sep 2012 13:54:39 +0400 > From: Dmitry Gutov <dgutov@yandex.ru> > CC: jan.h.d@swipnet.se, 12447@debbugs.gnu.org, hanche@math.ntnu.no > > On 19.09.2012 6:54, Eli Zaretskii wrote: > >> I'll keep the workaround, though, as it reportedly also fixes the > >> long-standing OS X freeze bug. > > > > What workaround is that? > > The one I posted in #12326: calling timer-activate-when-idle with nil > DONT-WAIT argument. I don't think that's a workaround. I think that's what js2 should do, if (AFAIU) it wants the timer handler be invoked once, after Emacs has been idle for more than 1 sec. > https://github.com/mooz/js2-mode/commit/b02f4a0d72d0e2087038fe891e2580c4505415ef Btw, in this line: (timer-activate-when-idle timer nil))) I think you can lose the last argument, since it is optional anyway. ^ permalink raw reply [flat|nested] 41+ messages in thread
* bug#12447: 24.1.50; Stuck in garbage collection on OS X 2012-09-19 15:24 ` Eli Zaretskii @ 2012-09-19 16:21 ` Dmitry Gutov 2012-09-19 16:38 ` Eli Zaretskii 0 siblings, 1 reply; 41+ messages in thread From: Dmitry Gutov @ 2012-09-19 16:21 UTC (permalink / raw) To: Eli Zaretskii; +Cc: hanche, 12447 On 19.09.2012 19:24, Eli Zaretskii wrote: >> Date: Wed, 19 Sep 2012 13:54:39 +0400 >> From: Dmitry Gutov <dgutov@yandex.ru> >> CC: jan.h.d@swipnet.se, 12447@debbugs.gnu.org, hanche@math.ntnu.no >> >> On 19.09.2012 6:54, Eli Zaretskii wrote: >>>> I'll keep the workaround, though, as it reportedly also fixes the >>>> long-standing OS X freeze bug. >>> >>> What workaround is that? >> >> The one I posted in #12326: calling timer-activate-when-idle with nil >> DONT-WAIT argument. > > I don't think that's a workaround. I think that's what js2 should do, > if (AFAIU) it wants the timer handler be invoked once, after Emacs has > been idle for more than 1 sec. Maybe, but run-with-idle-timer calls it with DONT-WAIT t, and js2-mode-reset-timer is only supposed to be called in response to some user action. So with your patch, it works effectively the same either way. By the way, here's what run-with-idle-timer docstring says: "Perform an action the next time Emacs is idle for SECS seconds." Shouldn't this mean that it should also pass DONT-WAIT nil? Below that, the docstring mentions what will happen if Emacs has been idle for N seconds (N < SECS), but doesn't clarify that if N => SECS, it will fire immediately. >> https://github.com/mooz/js2-mode/commit/b02f4a0d72d0e2087038fe891e2580c4505415ef > > Btw, in this line: > > (timer-activate-when-idle timer nil))) > > I think you can lose the last argument, since it is optional anyway. Sure, I wrote it that way just to be explicit. ^ permalink raw reply [flat|nested] 41+ messages in thread
* bug#12447: 24.1.50; Stuck in garbage collection on OS X 2012-09-19 16:21 ` Dmitry Gutov @ 2012-09-19 16:38 ` Eli Zaretskii 2012-09-19 18:29 ` Dmitry Gutov 2012-09-20 4:04 ` Chong Yidong 0 siblings, 2 replies; 41+ messages in thread From: Eli Zaretskii @ 2012-09-19 16:38 UTC (permalink / raw) To: Dmitry Gutov; +Cc: hanche, 12447 > Date: Wed, 19 Sep 2012 20:21:32 +0400 > From: Dmitry Gutov <dgutov@yandex.ru> > CC: jan.h.d@swipnet.se, 12447@debbugs.gnu.org, hanche@math.ntnu.no > > By the way, here's what run-with-idle-timer docstring says: > "Perform an action the next time Emacs is idle for SECS seconds." > > Shouldn't this mean that it should also pass DONT-WAIT nil? No, it just means no one considered the possibility that an idle timer will re-invoke itself like that. IOW, the doc string is inaccurate. ^ permalink raw reply [flat|nested] 41+ messages in thread
* bug#12447: 24.1.50; Stuck in garbage collection on OS X 2012-09-19 16:38 ` Eli Zaretskii @ 2012-09-19 18:29 ` Dmitry Gutov 2012-09-20 4:04 ` Chong Yidong 1 sibling, 0 replies; 41+ messages in thread From: Dmitry Gutov @ 2012-09-19 18:29 UTC (permalink / raw) To: Eli Zaretskii; +Cc: hanche, 12447 On 19.09.2012 20:38, Eli Zaretskii wrote: >> Date: Wed, 19 Sep 2012 20:21:32 +0400 >> From: Dmitry Gutov <dgutov@yandex.ru> >> CC: jan.h.d@swipnet.se, 12447@debbugs.gnu.org, hanche@math.ntnu.no >> >> By the way, here's what run-with-idle-timer docstring says: >> "Perform an action the next time Emacs is idle for SECS seconds." >> >> Shouldn't this mean that it should also pass DONT-WAIT nil? > > No, it just means no one considered the possibility that an idle timer > will re-invoke itself like that. IOW, the doc string is inaccurate. Okay. Could you fix it? ^ permalink raw reply [flat|nested] 41+ messages in thread
* bug#12447: 24.1.50; Stuck in garbage collection on OS X 2012-09-19 16:38 ` Eli Zaretskii 2012-09-19 18:29 ` Dmitry Gutov @ 2012-09-20 4:04 ` Chong Yidong 2012-09-20 16:01 ` Eli Zaretskii 1 sibling, 1 reply; 41+ messages in thread From: Chong Yidong @ 2012-09-20 4:04 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 12447, hanche, Dmitry Gutov Eli Zaretskii <eliz@gnu.org> writes: >> Date: Wed, 19 Sep 2012 20:21:32 +0400 >> From: Dmitry Gutov <dgutov@yandex.ru> >> CC: jan.h.d@swipnet.se, 12447@debbugs.gnu.org, hanche@math.ntnu.no >> >> By the way, here's what run-with-idle-timer docstring says: >> "Perform an action the next time Emacs is idle for SECS seconds." >> >> Shouldn't this mean that it should also pass DONT-WAIT nil? > > No, it just means no one considered the possibility that an idle timer > will re-invoke itself like that. IOW, the doc string is inaccurate. I'm not 100% sure this is merely a doc string problem. In the face of ambiguity, we should try to choose the behavior that is least likely to lead to infloops in user code. When `run-with-idle-timer' is called from an idle timer, we could interpret it to mean "run the function the next time Emacs becomes idle for SECS seconds, not including the current period of idleness". Such behavior seems quite reasonable. And it is of course easy to implement, by simply making `run-with-idle-timer' check if it is running while idle and, if so, giving a nil second arg to `timer-activate-by-idle'. ^ permalink raw reply [flat|nested] 41+ messages in thread
* bug#12447: 24.1.50; Stuck in garbage collection on OS X 2012-09-20 4:04 ` Chong Yidong @ 2012-09-20 16:01 ` Eli Zaretskii 2012-09-21 3:31 ` Chong Yidong 0 siblings, 1 reply; 41+ messages in thread From: Eli Zaretskii @ 2012-09-20 16:01 UTC (permalink / raw) To: Chong Yidong; +Cc: 12447, hanche, dgutov > From: Chong Yidong <cyd@gnu.org> > Cc: Dmitry Gutov <dgutov@yandex.ru>, hanche@math.ntnu.no, 12447@debbugs.gnu.org > Date: Thu, 20 Sep 2012 12:04:51 +0800 > > Eli Zaretskii <eliz@gnu.org> writes: > > >> Date: Wed, 19 Sep 2012 20:21:32 +0400 > >> From: Dmitry Gutov <dgutov@yandex.ru> > >> CC: jan.h.d@swipnet.se, 12447@debbugs.gnu.org, hanche@math.ntnu.no > >> > >> By the way, here's what run-with-idle-timer docstring says: > >> "Perform an action the next time Emacs is idle for SECS seconds." > >> > >> Shouldn't this mean that it should also pass DONT-WAIT nil? > > > > No, it just means no one considered the possibility that an idle timer > > will re-invoke itself like that. IOW, the doc string is inaccurate. > > I'm not 100% sure this is merely a doc string problem. In the face of > ambiguity, we should try to choose the behavior that is least likely to > lead to infloops in user code. There's no infloop with my patch. Assuming that no one comes with a loop in a couple of days, I will install that, and this particular issue will be gone. In general, I think we should prefer solutions that allow Lisp programmers or users do dumb things (and sometimes get dumb results), without wedging Emacs. Restricting what Lisp can do because someone dumb could wedge Emacs runs a risk of punishing the innocent lot because of a guilty (or dumb) few. > When `run-with-idle-timer' is called from an idle timer, we could > interpret it to mean "run the function the next time Emacs becomes idle > for SECS seconds, not including the current period of idleness". I could think of some legitimate uses of the current behavior, though. For example, imagine an idle timer that gets run after 1 sec of idleness, does something quick and simple, and then calls itself or another time with a larger timeout value, at which time it will do something different, perhaps more complex. I see no reason to forcibly prevent such use cases. It is also in line with Emacs behavior since about forever. The possibility of an infloop was a side effect of another bugfix. With that possibility hopefully taken care of, we have no reason to change old behavior, which evidently at least one external package relies upon. ^ permalink raw reply [flat|nested] 41+ messages in thread
* bug#12447: 24.1.50; Stuck in garbage collection on OS X 2012-09-20 16:01 ` Eli Zaretskii @ 2012-09-21 3:31 ` Chong Yidong 2012-09-21 7:14 ` Eli Zaretskii 0 siblings, 1 reply; 41+ messages in thread From: Chong Yidong @ 2012-09-21 3:31 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 12447, hanche, dgutov Eli Zaretskii <eliz@gnu.org> writes: >> When `run-with-idle-timer' is called from an idle timer, we could >> interpret it to mean "run the function the next time Emacs becomes idle >> for SECS seconds, not including the current period of idleness". > > I could think of some legitimate uses of the current behavior, though. > For example, imagine an idle timer that gets run after 1 sec of > idleness, does something quick and simple, and then calls itself or > another time with a larger timeout value, at which time it will do > something different, perhaps more complex. I see no reason to > forcibly prevent such use cases. It would still be possible to get the old behavior by calling lower-level timer functions used by `run-with-idle-timer'. > It is also in line with Emacs behavior since about forever. The > possibility of an infloop was a side effect of another bugfix. With > that possibility hopefully taken care of, we have no reason to change > old behavior, which evidently at least one external package relies > upon. What is the package that relies on the old behavior? ^ permalink raw reply [flat|nested] 41+ messages in thread
* bug#12447: 24.1.50; Stuck in garbage collection on OS X 2012-09-21 3:31 ` Chong Yidong @ 2012-09-21 7:14 ` Eli Zaretskii 2012-09-21 9:09 ` Chong Yidong 0 siblings, 1 reply; 41+ messages in thread From: Eli Zaretskii @ 2012-09-21 7:14 UTC (permalink / raw) To: Chong Yidong; +Cc: 12447, hanche, dgutov > From: Chong Yidong <cyd@gnu.org> > Cc: dgutov@yandex.ru, hanche@math.ntnu.no, 12447@debbugs.gnu.org > Date: Fri, 21 Sep 2012 11:31:51 +0800 > > What is the package that relies on the old behavior? js2-mode, see bug #12326 (merged with this one). ^ permalink raw reply [flat|nested] 41+ messages in thread
* bug#12447: 24.1.50; Stuck in garbage collection on OS X 2012-09-21 7:14 ` Eli Zaretskii @ 2012-09-21 9:09 ` Chong Yidong 2012-09-21 9:54 ` Eli Zaretskii 2012-09-21 10:49 ` Dmitry Gutov 0 siblings, 2 replies; 41+ messages in thread From: Chong Yidong @ 2012-09-21 9:09 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 12447, hanche, dgutov Eli Zaretskii <eliz@gnu.org> writes: >> What is the package that relies on the old behavior? > > js2-mode, see bug #12326 (merged with this one). I'm confused. You just said that js2-mode needed a code change: >> The one I posted in #12326: calling timer-activate-when-idle with nil >> DONT-WAIT argument. > > I don't think that's a workaround. I think that's what js2 should do, > if (AFAIU) it wants the timer handler be invoked once, after Emacs has > been idle for more than 1 sec. If I understand correctly, js2-mode (prior to the workaround) assumed the new behavior: it called run-with-idle-timer from inside the idle timer, with the same delay, with the intention of scheduling for the next period of idleness. ^ permalink raw reply [flat|nested] 41+ messages in thread
* bug#12447: 24.1.50; Stuck in garbage collection on OS X 2012-09-21 9:09 ` Chong Yidong @ 2012-09-21 9:54 ` Eli Zaretskii 2012-09-21 14:26 ` Chong Yidong 2012-09-21 10:49 ` Dmitry Gutov 1 sibling, 1 reply; 41+ messages in thread From: Eli Zaretskii @ 2012-09-21 9:54 UTC (permalink / raw) To: Chong Yidong; +Cc: 12447, hanche, dgutov > From: Chong Yidong <cyd@gnu.org> > Cc: dgutov@yandex.ru, hanche@math.ntnu.no, 12447@debbugs.gnu.org > Date: Fri, 21 Sep 2012 17:09:54 +0800 > > Eli Zaretskii <eliz@gnu.org> writes: > > >> What is the package that relies on the old behavior? > > > > js2-mode, see bug #12326 (merged with this one). > > I'm confused. You just said that js2-mode needed a code change: > > >> The one I posted in #12326: calling timer-activate-when-idle with nil > >> DONT-WAIT argument. > > > > I don't think that's a workaround. I think that's what js2 should do, > > if (AFAIU) it wants the timer handler be invoked once, after Emacs has > > been idle for more than 1 sec. > > If I understand correctly, js2-mode (prior to the workaround) assumed > the new behavior: it called run-with-idle-timer from inside the idle > timer, with the same delay, with the intention of scheduling for the > next period of idleness. I have no idea what js2-mode wants to do. What I wrote above was based on my limited reading of the code fragments posted to that bug. ^ permalink raw reply [flat|nested] 41+ messages in thread
* bug#12447: 24.1.50; Stuck in garbage collection on OS X 2012-09-21 9:54 ` Eli Zaretskii @ 2012-09-21 14:26 ` Chong Yidong 2012-09-22 13:18 ` Eli Zaretskii 0 siblings, 1 reply; 41+ messages in thread From: Chong Yidong @ 2012-09-21 14:26 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 12447, hanche, dgutov Eli Zaretskii <eliz@gnu.org> writes: >> If I understand correctly, js2-mode (prior to the workaround) assumed >> the new behavior: it called run-with-idle-timer from inside the idle >> timer, with the same delay, with the intention of scheduling for the >> next period of idleness. > > I have no idea what js2-mode wants to do. What I wrote above was > based on my limited reading of the code fragments posted to that bug. OK, I guess your approach is fine. Please go ahead and commit your patch when you're ready, and amend the docstring of run-with-idle-timer as Dmitry suggested. Thanks. ^ permalink raw reply [flat|nested] 41+ messages in thread
* bug#12447: 24.1.50; Stuck in garbage collection on OS X 2012-09-21 14:26 ` Chong Yidong @ 2012-09-22 13:18 ` Eli Zaretskii 0 siblings, 0 replies; 41+ messages in thread From: Eli Zaretskii @ 2012-09-22 13:18 UTC (permalink / raw) To: Chong Yidong; +Cc: hanche, 12447-done, dgutov > From: Chong Yidong <cyd@gnu.org> > Cc: dgutov@yandex.ru, hanche@math.ntnu.no, 12447@debbugs.gnu.org > Date: Fri, 21 Sep 2012 22:26:18 +0800 > > Eli Zaretskii <eliz@gnu.org> writes: > > >> If I understand correctly, js2-mode (prior to the workaround) assumed > >> the new behavior: it called run-with-idle-timer from inside the idle > >> timer, with the same delay, with the intention of scheduling for the > >> next period of idleness. > > > > I have no idea what js2-mode wants to do. What I wrote above was > > based on my limited reading of the code fragments posted to that bug. > > OK, I guess your approach is fine. Please go ahead and commit your > patch when you're ready, and amend the docstring of run-with-idle-timer > as Dmitry suggested. Done in trunk revision 110138. I'm therefore closing this bug. ^ permalink raw reply [flat|nested] 41+ messages in thread
* bug#12447: 24.1.50; Stuck in garbage collection on OS X 2012-09-21 9:09 ` Chong Yidong 2012-09-21 9:54 ` Eli Zaretskii @ 2012-09-21 10:49 ` Dmitry Gutov 1 sibling, 0 replies; 41+ messages in thread From: Dmitry Gutov @ 2012-09-21 10:49 UTC (permalink / raw) To: Chong Yidong; +Cc: 12447, hanche On 21.09.2012 13:09, Chong Yidong wrote: > Eli Zaretskii <eliz@gnu.org> writes: > >>> What is the package that relies on the old behavior? >> >> js2-mode, see bug #12326 (merged with this one). > > I'm confused. You just said that js2-mode needed a code change: > >>> The one I posted in #12326: calling timer-activate-when-idle with nil >>> DONT-WAIT argument. >> >> I don't think that's a workaround. I think that's what js2 should do, >> if (AFAIU) it wants the timer handler be invoked once, after Emacs has >> been idle for more than 1 sec. > > If I understand correctly, js2-mode (prior to the workaround) assumed > the new behavior: it called run-with-idle-timer from inside the idle > timer, with the same delay, with the intention of scheduling for the > next period of idleness. Like I mentioned, the new behavior makes more sense for it, but it worked almost the same with the old behavior until the infloop problem. ^ permalink raw reply [flat|nested] 41+ messages in thread
end of thread, other threads:[~2012-09-22 13:18 UTC | newest] Thread overview: 41+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-09-14 21:08 bug#12447: 24.1.50; Stuck in garbage collection on OS X Harald Hanche-Olsen 2012-09-15 9:55 ` Jan Djärv 2012-09-15 11:07 ` Harald Hanche-Olsen 2012-09-15 12:31 ` Eli Zaretskii 2012-09-15 13:19 ` Harald Hanche-Olsen 2012-09-15 13:56 ` Eli Zaretskii 2012-09-15 14:23 ` Harald Hanche-Olsen 2012-09-15 14:37 ` Eli Zaretskii 2012-09-15 18:59 ` Jan Djärv 2012-09-16 9:15 ` Dmitry Gutov 2012-09-16 10:31 ` Eli Zaretskii 2012-09-16 10:44 ` Dmitry Gutov 2012-09-16 11:53 ` Eli Zaretskii 2012-09-16 12:07 ` Dmitry Gutov 2012-09-16 12:39 ` Eli Zaretskii 2012-09-16 13:25 ` Dmitry Gutov 2012-09-16 13:47 ` Eli Zaretskii 2012-09-16 14:25 ` Dmitry Gutov 2012-09-16 14:54 ` Eli Zaretskii 2012-09-16 15:56 ` Dmitry Gutov 2012-09-18 15:05 ` Eli Zaretskii 2012-09-18 17:29 ` Jan Djärv 2012-09-18 20:08 ` Eli Zaretskii 2012-09-19 0:27 ` Dmitry Gutov 2012-09-19 2:54 ` Eli Zaretskii 2012-09-19 7:41 ` Harald Hanche-Olsen 2012-09-19 15:21 ` Eli Zaretskii 2012-09-19 9:54 ` Dmitry Gutov 2012-09-19 15:24 ` Eli Zaretskii 2012-09-19 16:21 ` Dmitry Gutov 2012-09-19 16:38 ` Eli Zaretskii 2012-09-19 18:29 ` Dmitry Gutov 2012-09-20 4:04 ` Chong Yidong 2012-09-20 16:01 ` Eli Zaretskii 2012-09-21 3:31 ` Chong Yidong 2012-09-21 7:14 ` Eli Zaretskii 2012-09-21 9:09 ` Chong Yidong 2012-09-21 9:54 ` Eli Zaretskii 2012-09-21 14:26 ` Chong Yidong 2012-09-22 13:18 ` Eli Zaretskii 2012-09-21 10:49 ` Dmitry Gutov
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).