Re: alarm_signal_handler is called too frequently

[Jan D.]

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.