Re: alarm_signal_handler is called too frequently

["Jan D." <jan.h.d@swipnet.se>]

YAMAMOTO Mitsuharu wrote:

> There may be some confusion between two kinds of timers: the OS-level
> alarm timer and the Emacs-level (cooperative?) timer.  The timer for
> Xt timeout events is the former, and the cursor blinking uses the
> latter.  The function timer_check is also for the latter.

Sorry, I misunderstood.  The timer_check is not run from popup_get_selection 
for popup menus because the argument do_timers is 0:

       if (do_timers && !XtAppPending (Xt_app_con))
         timer_check (1);

But for dialogs, do_timer is 1, so timers do get checked when a dialog is 
popped up.  However, idle timers are disabled because the event that popups the 
dialog makes Emacs non-idle.  But non-idle timers are run.  The check for 
INPUT_BLOCKED_P proposed by Richard in Feval does detect this:

   if (handling_signal || INPUT_BLOCKED_P)
     abort ();

I guess we just have to get rid of the call to timer_check in popup_get_selection.

> 
>>Since popups are within BLOCK/UNBLOCK__INPUT, the signal handler
>>just reschedules the alarm without running any timer code.
> 
> 
> Actually, the signal handler only sets the interval timer value
> (set_alarm) without calling schedule_atimer in this situation.  The
> new interval may become a small value, 1msec, by the following code in
> set_alarm.
> 
>       /* Don't set the interval to 0; this disables the timer.  */
>       if (EMACS_TIME_LE (atimers->expiration, now))
> 	{
> 	  EMACS_SET_SECS (time, 0);
> 	  EMACS_SET_USECS (time, 1000);
> 	}
> 
>       bzero (&it, sizeof it);
>       it.it_value = time;
>       setitimer (ITIMER_REAL, &it, 0);

By "reschedules the alarm" I meant that the code arranges for another SIGALRM 
to be delivered.  And yes, the time is 1 millisecond.

> 
> That's the reason why I did the following question:
> 
>   I think we don't have to call set_alarm when pending_atimers is
>   non-zero because do_pending_atimers is supposed to be called
>   eventually in such a case.  Is that correct?

You mean like this?

Index: atimer.c
*** atimer.c.~1.16.~    2004-07-19 12:53:25.000000000 +0200
--- atimer.c    2004-10-29 08:56:37.000000000 +0200
***************
*** 397,402 ****
--- 397,403 ----
         EMACS_GET_TIME (now);
       }

+   if (! (pending_atimers && interrupt_input_blocked))
       set_alarm ();
   }


That works and should save a couple of CPU cycles.

	Jan D.