Old/unused display bits

Old/unused display bits

[Dmitry Antipov]

[-- Attachment #1: Type: text/plain, Size: 109 bytes --]

IIUC these bits are either the leftovers from old
code or has no real users; can we get rid of them?

Dmitry

[-- Attachment #2: old_preempt_drop.patch --]
[-- Type: text/plain, Size: 7780 bytes --]

=== modified file 'src/dispnew.c'
--- src/dispnew.c	2013-01-23 20:07:28 +0000
+++ src/dispnew.c	2013-01-25 10:49:31 +0000
@@ -107,12 +107,6 @@
 static void adjust_frame_glyphs_for_window_redisplay (struct frame *);
 static void adjust_frame_glyphs_for_frame_redisplay (struct frame *);
 
-\f
-/* Redisplay preemption timers.  */
-
-static EMACS_TIME preemption_period;
-static EMACS_TIME preemption_next_check;
-
 /* True upon entry to redisplay means do not assume anything about
    current contents of actual terminal frame; clear and redraw it.  */
 
@@ -122,9 +116,8 @@
 
 bool display_completed;
 
-Lisp_Object Qdisplay_table, Qredisplay_dont_pause;
+Lisp_Object Qdisplay_table;
 
-\f
 /* The currently selected frame.  In a single-frame version, this
    variable always equals the_only_frame.  */
 
@@ -3078,23 +3071,10 @@
   bool paused_p;
   struct window *root_window = XWINDOW (f->root_window);
 
-  if (redisplay_dont_pause)
-    force_p = 1;
-  else if (NILP (Vredisplay_preemption_period))
-    force_p = 1;
-  else if (!force_p && NUMBERP (Vredisplay_preemption_period))
+  if (!force_p && detect_input_pending_ignore_squeezables ())
     {
-      double p = XFLOATINT (Vredisplay_preemption_period);
-
-      if (detect_input_pending_ignore_squeezables ())
-	{
-	  paused_p = 1;
-	  goto do_pause;
-	}
-
-      preemption_period = EMACS_TIME_FROM_DOUBLE (p);
-      preemption_next_check = add_emacs_time (current_emacs_time (),
-					      preemption_period);
+      paused_p = 1;
+      goto do_pause;
     }
 
   if (FRAME_WINDOW_P (f))
@@ -3230,18 +3210,6 @@
       /* Record that this is not a frame-based redisplay.  */
       set_frame_matrix_frame (NULL);
 
-      if (redisplay_dont_pause)
-	force_p = 1;
-      else if (NILP (Vredisplay_preemption_period))
-	force_p = 1;
-      else if (!force_p && NUMBERP (Vredisplay_preemption_period))
-	{
-	  double p = XFLOATINT (Vredisplay_preemption_period);
-	  preemption_period = EMACS_TIME_FROM_DOUBLE (p);
-	  preemption_next_check = add_emacs_time (current_emacs_time (),
-						  preemption_period);
-	}
-
       /* Update W.  */
       update_begin (f);
       update_window (w, force_p);
@@ -3394,9 +3362,7 @@
 {
   struct glyph_matrix *desired_matrix = w->desired_matrix;
   bool paused_p;
-#if !PERIODIC_PREEMPTION_CHECKING
   int preempt_count = baud_rate / 2400 + 1;
-#endif
   struct redisplay_interface *rif = FRAME_RIF (XFRAME (WINDOW_FRAME (w)));
 #ifdef GLYPH_DEBUG
   /* Check that W's frame doesn't have glyph matrices.  */
@@ -3404,10 +3370,8 @@
 #endif
 
   /* Check pending input the first time so that we can quickly return.  */
-#if !PERIODIC_PREEMPTION_CHECKING
   if (!force_p)
     detect_input_pending_ignore_squeezables ();
-#endif
 
   /* If forced to complete the update, or if no input is pending, do
      the update.  */
@@ -3418,9 +3382,7 @@
       struct glyph_row *header_line_row;
       int yb;
       bool changed_p = 0, mouse_face_overwritten_p = 0;
-#if ! PERIODIC_PREEMPTION_CHECKING
       int n_updated = 0;
-#endif
 
       rif->update_window_begin_hook (w);
       yb = window_text_bottom_y (w);
@@ -3484,22 +3446,8 @@
 	       detect_input_pending.  If it's done too often,
 	       scrolling large windows with repeated scroll-up
 	       commands will too quickly pause redisplay.  */
-#if PERIODIC_PREEMPTION_CHECKING
-	    if (!force_p)
-	      {
-		EMACS_TIME tm = current_emacs_time ();
-		if (EMACS_TIME_LT (preemption_next_check, tm))
-		  {
-		    preemption_next_check = add_emacs_time (tm,
-							    preemption_period);
-		    if (detect_input_pending_ignore_squeezables ())
-		      break;
-		  }
-	      }
-#else
 	    if (!force_p && ++n_updated % preempt_count == 0)
 	      detect_input_pending_ignore_squeezables ();
-#endif
 	    changed_p |= update_window_line (w, vpos,
 					     &mouse_face_overwritten_p);
 
@@ -4531,13 +4479,11 @@
   if (preempt_count <= 0)
     preempt_count = 1;
 
-#if !PERIODIC_PREEMPTION_CHECKING
   if (!force_p && detect_input_pending_ignore_squeezables ())
     {
       pause_p = 1;
       goto do_pause;
     }
-#endif
 
   /* If we cannot insert/delete lines, it's no use trying it.  */
   if (!FRAME_LINE_INS_DEL_OK (f))
@@ -4578,21 +4524,8 @@
 		}
 	    }
 
-#if PERIODIC_PREEMPTION_CHECKING
-	  if (!force_p)
-	    {
-	      EMACS_TIME tm = current_emacs_time ();
-	      if (EMACS_TIME_LT (preemption_next_check, tm))
-		{
-		  preemption_next_check = add_emacs_time (tm, preemption_period);
-		  if (detect_input_pending_ignore_squeezables ())
-		    break;
-		}
-	    }
-#else
 	  if (!force_p && (i - 1) % preempt_count == 0)
 	    detect_input_pending_ignore_squeezables ();
-#endif
 
 	  update_frame_line (f, i);
 	}
@@ -4698,9 +4631,7 @@
 	}
     }
 
-#if !PERIODIC_PREEMPTION_CHECKING
  do_pause:
-#endif
 
   clear_desired_matrices (f);
   return pause_p;
@@ -5893,33 +5824,19 @@
 
 DEFUN ("redisplay", Fredisplay, Sredisplay, 0, 1, 0,
        doc: /* Perform redisplay.
-Optional arg FORCE, if non-nil, prevents redisplay from being
-preempted by arriving input, even if `redisplay-dont-pause' is nil.
-If `redisplay-dont-pause' is non-nil (the default), redisplay is never
-preempted by arriving input, so FORCE does nothing.
+Optional arg FORCE does nothing and used for backward compatibility.
 
 Return t if redisplay was performed, nil if redisplay was preempted
 immediately by pending input.  */)
   (Lisp_Object force)
 {
-  ptrdiff_t count;
-
   swallow_events (1);
-  if ((detect_input_pending_run_timers (1)
-       && NILP (force) && !redisplay_dont_pause)
-      || !NILP (Vexecuting_kbd_macro))
+  if (detect_input_pending_run_timers (1) || !NILP (Vexecuting_kbd_macro))
     return Qnil;
-
-  count = SPECPDL_INDEX ();
-  if (!NILP (force) && !redisplay_dont_pause)
-    specbind (Qredisplay_dont_pause, Qt);
   redisplay_preserve_echo_area (2);
-  unbind_to (count, Qnil);
   return Qt;
 }
 
-
-\f
 /***********************************************************************
 			 Other Lisp Functions
  ***********************************************************************/
@@ -6335,7 +6252,6 @@
   staticpro (&frame_and_buffer_state);
 
   DEFSYM (Qdisplay_table, "display-table");
-  DEFSYM (Qredisplay_dont_pause, "redisplay-dont-pause");
 
   DEFVAR_INT ("baud-rate", baud_rate,
 	      doc: /* The output baud rate of the terminal.
@@ -6406,19 +6322,6 @@
 See `buffer-display-table' for more information.  */);
   Vstandard_display_table = Qnil;
 
-  DEFVAR_BOOL ("redisplay-dont-pause", redisplay_dont_pause,
-	       doc: /* Non-nil means display update isn't paused when input is detected.  */);
-  redisplay_dont_pause = 1;
-
-#if PERIODIC_PREEMPTION_CHECKING
-  DEFVAR_LISP ("redisplay-preemption-period", Vredisplay_preemption_period,
-	       doc: /* Period in seconds between checking for input during redisplay.
-This has an effect only if `redisplay-dont-pause' is nil; in that
-case, arriving input preempts redisplay until the input is processed.
-If the value is nil, redisplay is never preempted.  */);
-  Vredisplay_preemption_period = make_float (0.10);
-#endif
-
 #ifdef CANNOT_DUMP
   if (noninteractive)
 #endif

=== modified file 'src/xdisp.c'
--- src/xdisp.c	2013-01-24 05:41:28 +0000
+++ src/xdisp.c	2013-01-25 10:51:24 +0000
@@ -10697,14 +10697,9 @@
 	         needs to run hooks.  */
 	      && !NILP (Vrun_hooks))
 	    {
-	      /* Must update other windows.  Likewise as in other
-		 cases, don't let this update be interrupted by
-		 pending input.  */
-	      ptrdiff_t count = SPECPDL_INDEX ();
-	      specbind (Qredisplay_dont_pause, Qt);
+	      /* Must update other windows.  */
 	      windows_or_buffers_changed = 1;
 	      redisplay_internal ();
-	      unbind_to (count, Qnil);
 	    }
 	  else if (FRAME_WINDOW_P (f) && n == 0)
 	    {

Re: Old/unused display bits

[Eli Zaretskii]

> Date: Fri, 25 Jan 2013 16:22:09 +0400
> From: Dmitry Antipov <dmantipov@yandex.ru>
> CC: Eli Zaretskii <eliz@gnu.org>
> 
> IIUC these bits are either the leftovers from old
> code or has no real users; can we get rid of them?

Kim, is there a reason to keep this?

> === modified file 'src/dispnew.c'
> --- src/dispnew.c	2013-01-23 20:07:28 +0000
> +++ src/dispnew.c	2013-01-25 10:49:31 +0000
> @@ -107,12 +107,6 @@
>  static void adjust_frame_glyphs_for_window_redisplay (struct frame *);
>  static void adjust_frame_glyphs_for_frame_redisplay (struct frame *);
>  
> -\f
> -/* Redisplay preemption timers.  */
> -
> -static EMACS_TIME preemption_period;
> -static EMACS_TIME preemption_next_check;
> -
>  /* True upon entry to redisplay means do not assume anything about
>     current contents of actual terminal frame; clear and redraw it.  */
>  
> @@ -122,9 +116,8 @@
>  
>  bool display_completed;
>  
> -Lisp_Object Qdisplay_table, Qredisplay_dont_pause;
> +Lisp_Object Qdisplay_table;
>  
> -\f
>  /* The currently selected frame.  In a single-frame version, this
>     variable always equals the_only_frame.  */
>  
> @@ -3078,23 +3071,10 @@
>    bool paused_p;
>    struct window *root_window = XWINDOW (f->root_window);
>  
> -  if (redisplay_dont_pause)
> -    force_p = 1;
> -  else if (NILP (Vredisplay_preemption_period))
> -    force_p = 1;
> -  else if (!force_p && NUMBERP (Vredisplay_preemption_period))
> +  if (!force_p && detect_input_pending_ignore_squeezables ())
>      {
> -      double p = XFLOATINT (Vredisplay_preemption_period);
> -
> -      if (detect_input_pending_ignore_squeezables ())
> -	{
> -	  paused_p = 1;
> -	  goto do_pause;
> -	}
> -
> -      preemption_period = EMACS_TIME_FROM_DOUBLE (p);
> -      preemption_next_check = add_emacs_time (current_emacs_time (),
> -					      preemption_period);
> +      paused_p = 1;
> +      goto do_pause;
>      }
>  
>    if (FRAME_WINDOW_P (f))
> @@ -3230,18 +3210,6 @@
>        /* Record that this is not a frame-based redisplay.  */
>        set_frame_matrix_frame (NULL);
>  
> -      if (redisplay_dont_pause)
> -	force_p = 1;
> -      else if (NILP (Vredisplay_preemption_period))
> -	force_p = 1;
> -      else if (!force_p && NUMBERP (Vredisplay_preemption_period))
> -	{
> -	  double p = XFLOATINT (Vredisplay_preemption_period);
> -	  preemption_period = EMACS_TIME_FROM_DOUBLE (p);
> -	  preemption_next_check = add_emacs_time (current_emacs_time (),
> -						  preemption_period);
> -	}
> -
>        /* Update W.  */
>        update_begin (f);
>        update_window (w, force_p);
> @@ -3394,9 +3362,7 @@
>  {
>    struct glyph_matrix *desired_matrix = w->desired_matrix;
>    bool paused_p;
> -#if !PERIODIC_PREEMPTION_CHECKING
>    int preempt_count = baud_rate / 2400 + 1;
> -#endif
>    struct redisplay_interface *rif = FRAME_RIF (XFRAME (WINDOW_FRAME (w)));
>  #ifdef GLYPH_DEBUG
>    /* Check that W's frame doesn't have glyph matrices.  */
> @@ -3404,10 +3370,8 @@
>  #endif
>  
>    /* Check pending input the first time so that we can quickly return.  */
> -#if !PERIODIC_PREEMPTION_CHECKING
>    if (!force_p)
>      detect_input_pending_ignore_squeezables ();
> -#endif
>  
>    /* If forced to complete the update, or if no input is pending, do
>       the update.  */
> @@ -3418,9 +3382,7 @@
>        struct glyph_row *header_line_row;
>        int yb;
>        bool changed_p = 0, mouse_face_overwritten_p = 0;
> -#if ! PERIODIC_PREEMPTION_CHECKING
>        int n_updated = 0;
> -#endif
>  
>        rif->update_window_begin_hook (w);
>        yb = window_text_bottom_y (w);
> @@ -3484,22 +3446,8 @@
>  	       detect_input_pending.  If it's done too often,
>  	       scrolling large windows with repeated scroll-up
>  	       commands will too quickly pause redisplay.  */
> -#if PERIODIC_PREEMPTION_CHECKING
> -	    if (!force_p)
> -	      {
> -		EMACS_TIME tm = current_emacs_time ();
> -		if (EMACS_TIME_LT (preemption_next_check, tm))
> -		  {
> -		    preemption_next_check = add_emacs_time (tm,
> -							    preemption_period);
> -		    if (detect_input_pending_ignore_squeezables ())
> -		      break;
> -		  }
> -	      }
> -#else
>  	    if (!force_p && ++n_updated % preempt_count == 0)
>  	      detect_input_pending_ignore_squeezables ();
> -#endif
>  	    changed_p |= update_window_line (w, vpos,
>  					     &mouse_face_overwritten_p);
>  
> @@ -4531,13 +4479,11 @@
>    if (preempt_count <= 0)
>      preempt_count = 1;
>  
> -#if !PERIODIC_PREEMPTION_CHECKING
>    if (!force_p && detect_input_pending_ignore_squeezables ())
>      {
>        pause_p = 1;
>        goto do_pause;
>      }
> -#endif
>  
>    /* If we cannot insert/delete lines, it's no use trying it.  */
>    if (!FRAME_LINE_INS_DEL_OK (f))
> @@ -4578,21 +4524,8 @@
>  		}
>  	    }
>  
> -#if PERIODIC_PREEMPTION_CHECKING
> -	  if (!force_p)
> -	    {
> -	      EMACS_TIME tm = current_emacs_time ();
> -	      if (EMACS_TIME_LT (preemption_next_check, tm))
> -		{
> -		  preemption_next_check = add_emacs_time (tm, preemption_period);
> -		  if (detect_input_pending_ignore_squeezables ())
> -		    break;
> -		}
> -	    }
> -#else
>  	  if (!force_p && (i - 1) % preempt_count == 0)
>  	    detect_input_pending_ignore_squeezables ();
> -#endif
>  
>  	  update_frame_line (f, i);
>  	}
> @@ -4698,9 +4631,7 @@
>  	}
>      }
>  
> -#if !PERIODIC_PREEMPTION_CHECKING
>   do_pause:
> -#endif
>  
>    clear_desired_matrices (f);
>    return pause_p;
> @@ -5893,33 +5824,19 @@
>  
>  DEFUN ("redisplay", Fredisplay, Sredisplay, 0, 1, 0,
>         doc: /* Perform redisplay.
> -Optional arg FORCE, if non-nil, prevents redisplay from being
> -preempted by arriving input, even if `redisplay-dont-pause' is nil.
> -If `redisplay-dont-pause' is non-nil (the default), redisplay is never
> -preempted by arriving input, so FORCE does nothing.
> +Optional arg FORCE does nothing and used for backward compatibility.
>  
>  Return t if redisplay was performed, nil if redisplay was preempted
>  immediately by pending input.  */)
>    (Lisp_Object force)
>  {
> -  ptrdiff_t count;
> -
>    swallow_events (1);
> -  if ((detect_input_pending_run_timers (1)
> -       && NILP (force) && !redisplay_dont_pause)
> -      || !NILP (Vexecuting_kbd_macro))
> +  if (detect_input_pending_run_timers (1) || !NILP (Vexecuting_kbd_macro))
>      return Qnil;
> -
> -  count = SPECPDL_INDEX ();
> -  if (!NILP (force) && !redisplay_dont_pause)
> -    specbind (Qredisplay_dont_pause, Qt);
>    redisplay_preserve_echo_area (2);
> -  unbind_to (count, Qnil);
>    return Qt;
>  }
>  
> -
> -\f
>  /***********************************************************************
>  			 Other Lisp Functions
>   ***********************************************************************/
> @@ -6335,7 +6252,6 @@
>    staticpro (&frame_and_buffer_state);
>  
>    DEFSYM (Qdisplay_table, "display-table");
> -  DEFSYM (Qredisplay_dont_pause, "redisplay-dont-pause");
>  
>    DEFVAR_INT ("baud-rate", baud_rate,
>  	      doc: /* The output baud rate of the terminal.
> @@ -6406,19 +6322,6 @@
>  See `buffer-display-table' for more information.  */);
>    Vstandard_display_table = Qnil;
>  
> -  DEFVAR_BOOL ("redisplay-dont-pause", redisplay_dont_pause,
> -	       doc: /* Non-nil means display update isn't paused when input is detected.  */);
> -  redisplay_dont_pause = 1;
> -
> -#if PERIODIC_PREEMPTION_CHECKING
> -  DEFVAR_LISP ("redisplay-preemption-period", Vredisplay_preemption_period,
> -	       doc: /* Period in seconds between checking for input during redisplay.
> -This has an effect only if `redisplay-dont-pause' is nil; in that
> -case, arriving input preempts redisplay until the input is processed.
> -If the value is nil, redisplay is never preempted.  */);
> -  Vredisplay_preemption_period = make_float (0.10);
> -#endif
> -
>  #ifdef CANNOT_DUMP
>    if (noninteractive)
>  #endif
> 
> === modified file 'src/xdisp.c'
> --- src/xdisp.c	2013-01-24 05:41:28 +0000
> +++ src/xdisp.c	2013-01-25 10:51:24 +0000
> @@ -10697,14 +10697,9 @@
>  	         needs to run hooks.  */
>  	      && !NILP (Vrun_hooks))
>  	    {
> -	      /* Must update other windows.  Likewise as in other
> -		 cases, don't let this update be interrupted by
> -		 pending input.  */
> -	      ptrdiff_t count = SPECPDL_INDEX ();
> -	      specbind (Qredisplay_dont_pause, Qt);
> +	      /* Must update other windows.  */
>  	      windows_or_buffers_changed = 1;
>  	      redisplay_internal ();
> -	      unbind_to (count, Qnil);
>  	    }
>  	  else if (FRAME_WINDOW_P (f) && n == 0)
>  	    {
> 

Re: Old/unused display bits

[Kim Storm]

On 2013-01-25 14:48, Eli Zaretskii wrote:
>> Date: Fri, 25 Jan 2013 16:22:09 +0400
>> From: Dmitry Antipov <dmantipov@yandex.ru>
>> CC: Eli Zaretskii <eliz@gnu.org>
>>
>> IIUC these bits are either the leftovers from old
>> code or has no real users; can we get rid of them?
> Kim, is there a reason to keep this?

Hi Eli,

Has redisplay-dont-pause been removed entirely?

I think the default has changed - but the option is still there - or ?

Iff it has been remove, the mentioned bits are indeed superfluous.
I haven't studied the patch in details though.

Kim

Re: Old/unused display bits

[Eli Zaretskii]

> Date: Fri, 25 Jan 2013 16:08:55 +0100
> From: Kim Storm <storm@cua.dk>
> CC: Dmitry Antipov <dmantipov@yandex.ru>, emacs-devel@gnu.org
> 
> On 2013-01-25 14:48, Eli Zaretskii wrote:
> >> Date: Fri, 25 Jan 2013 16:22:09 +0400
> >> From: Dmitry Antipov <dmantipov@yandex.ru>
> >> CC: Eli Zaretskii <eliz@gnu.org>
> >>
> >> IIUC these bits are either the leftovers from old
> >> code or has no real users; can we get rid of them?
> > Kim, is there a reason to keep this?
> 
> Hi Eli,
> 
> Has redisplay-dont-pause been removed entirely?

No, it's still there.

> I think the default has changed - but the option is still there

Correct.

> Iff it has been remove, the mentioned bits are indeed superfluous.
> I haven't studied the patch in details though.

Can you say a few words about this code and what behavior it allows?
Some of it is conditionally compiled, so not all of it is available in
stock Emacs.

TIA

Re: Old/unused display bits

[Kim Storm]

On 2013-01-25 16:47, Eli Zaretskii wrote:
> Can you say a few words about this code and what behavior it allows? 
> Some of it is conditionally compiled, so not all of it is available in 
> stock Emacs. TIA 

I think the doc string for redisplay-preemption-period is the best 
source (look in dispnew.c sources for it).

I don't remember, but from the look of it, it seems related to running 
text-only emacs over very slow lines (2400 bps and up), and trying to 
make a less drastic version of redisplay-dont-pause, so that the screen 
is updated at least sometimes and to some extend even if input arrives 
at a steady rate without pause.

I'm not sure this is relevant anymore.

Kim

Re: Old/unused display bits

[Eli Zaretskii]

> Date: Fri, 25 Jan 2013 17:22:18 +0100
> From: Kim Storm <storm@cua.dk>
> CC: dmantipov@yandex.ru, emacs-devel@gnu.org
> 
> On 2013-01-25 16:47, Eli Zaretskii wrote:
> > Can you say a few words about this code and what behavior it allows? 
> > Some of it is conditionally compiled, so not all of it is available in 
> > stock Emacs. TIA 
> 
> I think the doc string for redisplay-preemption-period is the best 
> source (look in dispnew.c sources for it).

Thanks.

So I took a closer look at the patch, and it sounds like it is
removing 3 different, although related, features:

 . The redisplay-dont-pause variable and its effect on redisplay
   preemption due to arriving input;

 . The redisplay-preemption-period variable and its effect on
   redisplay preemption;

 . The PERIODIC_PREEMPTION_CHECKING compile-time option.

I don't think we should remove redisplay-dont-pause.  It was only
turned on by default in Emacs 24.1, and it's not yet clear whether the
non-nil is a "fits all" value.

I never used the other 2 options.  I don't think I have a slow line to
see them in action.  It is probably safe to say that
PERIODIC_PREEMPTION_CHECKING was not used by too many people in their
day to day work.

Stefan, your opinion?

Re: Old/unused display bits

[Stefan Monnier]

> I don't think we should remove redisplay-dont-pause.  It was only
> turned on by default in Emacs 24.1, and it's not yet clear whether the
> non-nil is a "fits all" value.

Agreed, it's too early to remove it.

> I never used the other 2 options.  I don't think I have a slow line to
> see them in action.  It is probably safe to say that
> PERIODIC_PREEMPTION_CHECKING was not used by too many people in their
> day to day work.

We I think it's OK to remove PERIODIC_PREEMPTION_CHECKING and
redisplay-preemption-period (which is only defined if you set
PERIODIC_PREEMPTION_CHECKING).
[ BTW, it seems that in the current code, Vredisplay_preemption_period
  is read but is never initialized, unless you compile with
  PERIODIC_PREEMPTION_CHECKING.  ]


        Stefan