[RFC] avoid gc_in_progress in redisplay

[RFC] avoid gc_in_progress in redisplay

[Dmitry Antipov]

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

IIUC if we call to unblock_input _after_ clearing gc_in_progress in Fgarbage_collect,
we can avoid gc_in_progress == true cases in redisplay routines; thus, some things
may be simplified. Comments?

Dmitry

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

=== modified file 'src/alloc.c'
--- src/alloc.c	2012-10-18 18:21:55 +0000
+++ src/alloc.c	2012-10-19 05:08:46 +0000
@@ -5287,12 +5287,12 @@
   dump_zombies ();
 #endif
 
+  check_cons_list ();
+
+  gc_in_progress = 0;
+
   unblock_input ();
 
-  check_cons_list ();
-
-  gc_in_progress = 0;
-
   consing_since_gc = 0;
   if (gc_cons_threshold < GC_DEFAULT_THRESHOLD / 10)
     gc_cons_threshold = GC_DEFAULT_THRESHOLD / 10;

=== modified file 'src/frame.h'
--- src/frame.h	2012-10-07 22:31:58 +0000
+++ src/frame.h	2012-10-19 05:14:55 +0000
@@ -943,6 +943,21 @@
 	&& (frame_var = XCAR (list_var), 1));	\
        list_var = XCDR (list_var))
 
+/* Reflect mouse movement when a complete frame update is performed.  */
+
+#define FRAME_MOUSE_UPDATE(frame)				\
+  do {								\
+    Mouse_HLInfo *hlinfo = MOUSE_HL_INFO (frame);		\
+    if (frame == hlinfo->mouse_face_mouse_frame)		\
+      {							\
+	block_input ();						\
+	if (hlinfo->mouse_face_mouse_frame)			\
+	  note_mouse_highlight (hlinfo->mouse_face_mouse_frame,	\
+				hlinfo->mouse_face_mouse_x,     \
+				hlinfo->mouse_face_mouse_y);    \
+	unblock_input ();					\
+      }							\
+  } while (0)
 
 extern Lisp_Object Qframep, Qframe_live_p;
 extern Lisp_Object Qtty, Qtty_type;

=== modified file 'src/lisp.h'
--- src/lisp.h	2012-10-16 02:22:25 +0000
+++ src/lisp.h	2012-10-19 05:11:06 +0000
@@ -1609,10 +1609,6 @@
   int mouse_face_face_id;
   Lisp_Object mouse_face_overlay;
 
-  /* 1 if a mouse motion event came and we didn't handle it right away because
-     gc was in progress.  */
-  int mouse_face_deferred_gc;
-
   /* FRAME and X, Y position of mouse when last checked for
      highlighting.  X and Y can be negative or out of range for the frame.  */
   struct frame *mouse_face_mouse_frame;

=== modified file 'src/msdos.c'
--- src/msdos.c	2012-09-23 08:44:20 +0000
+++ src/msdos.c	2012-10-19 05:15:38 +0000
@@ -1275,7 +1275,6 @@
       hlinfo->mouse_face_beg_row = hlinfo->mouse_face_beg_col = -1;
       hlinfo->mouse_face_end_row = hlinfo->mouse_face_end_col = -1;
       hlinfo->mouse_face_window = Qnil;
-      hlinfo->mouse_face_deferred_gc = 0;
       hlinfo->mouse_face_mouse_frame = NULL;
     }
 
@@ -1295,21 +1294,10 @@
 static void
 IT_frame_up_to_date (struct frame *f)
 {
-  Mouse_HLInfo *hlinfo = MOUSE_HL_INFO (f);
   Lisp_Object new_cursor, frame_desired_cursor;
   struct window *sw;
 
-  if (hlinfo->mouse_face_deferred_gc
-      || (f && f == hlinfo->mouse_face_mouse_frame))
-    {
-      block_input ();
-      if (hlinfo->mouse_face_mouse_frame)
-	note_mouse_highlight (hlinfo->mouse_face_mouse_frame,
-			      hlinfo->mouse_face_mouse_x,
-			      hlinfo->mouse_face_mouse_y);
-      hlinfo->mouse_face_deferred_gc = 0;
-      unblock_input ();
-    }
+  FRAME_MOUSE_UPDATE (f);
 
   /* Set the cursor type to whatever they wanted.  In a minibuffer
      window, we want the cursor to appear only if we are reading input
@@ -1849,7 +1837,6 @@
 	    FRAME_BACKGROUND_PIXEL (SELECTED_FRAME ()) = colors[1];
 	}
       the_only_display_info.mouse_highlight.mouse_face_mouse_frame = NULL;
-      the_only_display_info.mouse_highlight.mouse_face_deferred_gc = 0;
       the_only_display_info.mouse_highlight.mouse_face_beg_row =
 	the_only_display_info.mouse_highlight.mouse_face_beg_col = -1;
       the_only_display_info.mouse_highlight.mouse_face_end_row =

=== modified file 'src/w32term.c'
--- src/w32term.c	2012-10-08 13:46:03 +0000
+++ src/w32term.c	2012-10-19 05:15:56 +0000
@@ -723,21 +723,7 @@
 w32_frame_up_to_date (struct frame *f)
 {
   if (FRAME_W32_P (f))
-    {
-      Mouse_HLInfo *hlinfo = MOUSE_HL_INFO (f);
-
-      if (hlinfo->mouse_face_deferred_gc
-	  || f == hlinfo->mouse_face_mouse_frame)
-	{
-	  block_input ();
-	  if (hlinfo->mouse_face_mouse_frame)
-	    note_mouse_highlight (hlinfo->mouse_face_mouse_frame,
-				  hlinfo->mouse_face_mouse_x,
-				  hlinfo->mouse_face_mouse_y);
-	  hlinfo->mouse_face_deferred_gc = 0;
-	  unblock_input ();
-	}
-    }
+    FRAME_MOUSE_UPDATE (f);
 }
 
 
@@ -5984,7 +5970,6 @@
       hlinfo->mouse_face_end_row
 	= hlinfo->mouse_face_end_col = -1;
       hlinfo->mouse_face_window = Qnil;
-      hlinfo->mouse_face_deferred_gc = 0;
       hlinfo->mouse_face_mouse_frame = 0;
     }
 

=== modified file 'src/xdisp.c'
--- src/xdisp.c	2012-10-17 13:30:56 +0000
+++ src/xdisp.c	2012-10-19 05:10:52 +0000
@@ -9605,7 +9605,8 @@
       do_pending_window_change (0);
       echo_area_display (1);
       do_pending_window_change (0);
-      if (FRAME_TERMINAL (f)->frame_up_to_date_hook != 0 && ! gc_in_progress)
+      eassert (!gc_in_progress);
+      if (FRAME_TERMINAL (f)->frame_up_to_date_hook)
 	(*FRAME_TERMINAL (f)->frame_up_to_date_hook) (f);
     }
 }
@@ -9702,7 +9703,8 @@
       do_pending_window_change (0);
       echo_area_display (1);
       do_pending_window_change (0);
-      if (FRAME_TERMINAL (f)->frame_up_to_date_hook != 0 && ! gc_in_progress)
+      eassert (!gc_in_progress);
+      if (FRAME_TERMINAL (f)->frame_up_to_date_hook)
 	(*FRAME_TERMINAL (f)->frame_up_to_date_hook) (f);
     }
 }
@@ -27622,6 +27624,8 @@
   Lisp_Object pointer = Qnil;  /* Takes precedence over cursor!  */
   struct buffer *b;
 
+  eassert (!gc_in_progress);
+
   /* When a menu is active, don't highlight because this looks odd.  */
 #if defined (USE_X_TOOLKIT) || defined (USE_GTK) || defined (HAVE_NS) || defined (MSDOS)
   if (popup_activated ())
@@ -27640,12 +27644,6 @@
   if (hlinfo->mouse_face_defer)
     return;
 
-  if (gc_in_progress)
-    {
-      hlinfo->mouse_face_deferred_gc = 1;
-      return;
-    }
-
   /* Which window is that in?  */
   window = window_from_coordinates (f, x, y, &part, 1);
 

=== modified file 'src/xterm.c'
--- src/xterm.c	2012-10-01 06:36:54 +0000
+++ src/xterm.c	2012-10-19 05:16:09 +0000
@@ -669,21 +669,7 @@
 XTframe_up_to_date (struct frame *f)
 {
   if (FRAME_X_P (f))
-    {
-      Mouse_HLInfo *hlinfo = MOUSE_HL_INFO (f);
-
-      if (hlinfo->mouse_face_deferred_gc
-	  || f == hlinfo->mouse_face_mouse_frame)
-	{
-	  block_input ();
-	  if (hlinfo->mouse_face_mouse_frame)
-	    note_mouse_highlight (hlinfo->mouse_face_mouse_frame,
-				  hlinfo->mouse_face_mouse_x,
-				  hlinfo->mouse_face_mouse_y);
-	  hlinfo->mouse_face_deferred_gc = 0;
-	  unblock_input ();
-	}
-    }
+    FRAME_MOUSE_UPDATE (f);
 }
 
 
@@ -9512,7 +9498,6 @@
       hlinfo->mouse_face_end_row
 	= hlinfo->mouse_face_end_col = -1;
       hlinfo->mouse_face_window = Qnil;
-      hlinfo->mouse_face_deferred_gc = 0;
       hlinfo->mouse_face_mouse_frame = 0;
     }
 
@@ -10163,7 +10148,6 @@
   dpyinfo->bitmaps_last = 0;
   dpyinfo->scratch_cursor_gc = 0;
   hlinfo->mouse_face_mouse_frame = 0;
-  hlinfo->mouse_face_deferred_gc = 0;
   hlinfo->mouse_face_beg_row = hlinfo->mouse_face_beg_col = -1;
   hlinfo->mouse_face_end_row = hlinfo->mouse_face_end_col = -1;
   hlinfo->mouse_face_face_id = DEFAULT_FACE_ID;

Re: [RFC] avoid gc_in_progress in redisplay

[Eli Zaretskii]

> Date: Fri, 19 Oct 2012 09:27:29 +0400
> From: Dmitry Antipov <dmantipov@yandex.ru>
> 
> IIUC if we call to unblock_input _after_ clearing gc_in_progress in Fgarbage_collect,
> we can avoid gc_in_progress == true cases in redisplay routines; thus, some things
> may be simplified. Comments?

I think this is a red herring: mouse event cannot possibly come in
while GC is in progress, since we have switched to SYNC_INPUT.  Any
mouse event that is detected during GC will just set a flag, which
will be handled either as part of QUIT or when Emacs becomes idle.
And I don't believe GC can QUIT, can it?

Like I wrote when we switched all platforms to SYNC_INPUT: there's lot
of code all over the place that still tries to cater to asynchronous
input events (mouse highlight being one of them) that can no longer
happen.  We need to remove that code and delete the corresponding data
structure members which support that code.

But I wouldn't recommend doing that during the feature freeze.