[Mac-emacs-users] Slow Redisplay Bug Fix

[Andrew C. <ak...@cs...>]

Hi Everyone,

This change improves the patch emacs-21.1-2-mac.patch.gz for building
Emacs 21.1 on Mac OS X.

Without it, the entire menu tree is rebuilt every time a frame is
redisplayed.  This is unnecessary since Emacs rebuilds the menu tree
when the menu bar is clicked anyway.

This (performance) bug does not cause anything to be displayed
incorrectly (afterall it is doing more than that is necessary).
However it causes input to be very sluggish when each key-press
requires a redisplay, e.g., when one is using a quail input method for
typing Chinese or Japanese.

Please apply the following patch to src/macmenu.c.

Andrew.
-----
--- emacs-21.1.30/src/macmenu.c	Sat Oct 27 10:28:58 2001
+++ emacs/src/macmenu.c	Fri Dec 14 14:33:12 2001
@@ -1318,6 +1318,11 @@
 
   XSETFRAME (Vmenu_updating_frame, f);
 
+  if (! menubar_widget)
+    deep_p = 1;
+  else if (pending_menu_activation && !deep_p)
+    deep_p = 1;
+
   wv = xmalloc_widget_value ();
   wv->name = "menubar";
   wv->value = 0;
@@ -1325,116 +1330,155 @@
   wv->button_type = BUTTON_TYPE_NONE;
   first_wv = wv;
 
-  {
-    /* Make a widget-value tree representing the entire menu trees.  */
-
-    struct buffer *prev = current_buffer;
-    Lisp_Object buffer;
-    int specpdl_count = specpdl_ptr - specpdl;
-    int previous_menu_items_used = f->menu_bar_items_used;
-    Lisp_Object *previous_items
-      = (Lisp_Object *) alloca (previous_menu_items_used
-                                * sizeof (Lisp_Object));
-
-    /* If we are making a new widget, its contents are empty,
-       do always reinitialize them.  */
-    if (! menubar_widget)
-      previous_menu_items_used = 0;
-
-    buffer = XWINDOW (FRAME_SELECTED_WINDOW (f))->buffer;
-    specbind (Qinhibit_quit, Qt);
-    /* Don't let the debugger step into this code
-       because it is not reentrant.  */
-    specbind (Qdebug_on_next_call, Qnil);
-
-    record_unwind_protect (Fset_match_data, Fmatch_data (Qnil, Qnil));
-    if (NILP (Voverriding_local_map_menu_flag))
-      {
-	specbind (Qoverriding_terminal_local_map, Qnil);
-	specbind (Qoverriding_local_map, Qnil);
-      }
-
-    set_buffer_internal_1 (XBUFFER (buffer));
-
-    /* Run the Lucid hook.  */
-    call1 (Vrun_hooks, Qactivate_menubar_hook);
-    /* If it has changed current-menubar from previous value,
-       really recompute the menubar from the value.  */
-    if (! NILP (Vlucid_menu_bar_dirty_flag))
-      call0 (Qrecompute_lucid_menubar);
-    safe_run_hooks (Qmenu_bar_update_hook);
-    FRAME_MENU_BAR_ITEMS (f) = menu_bar_items (FRAME_MENU_BAR_ITEMS (f));
-
-    items = FRAME_MENU_BAR_ITEMS (f);
-
-    inhibit_garbage_collection ();
-
-    /* Save the frame's previous menu bar contents data.  */
-    bcopy (XVECTOR (f->menu_bar_vector)->contents, previous_items,
-	   previous_menu_items_used * sizeof (Lisp_Object));
-
-    /* Fill in the current menu bar contents.  */
-    menu_items = f->menu_bar_vector;
-    menu_items_allocated = XVECTOR (menu_items)->size;
-    init_menu_items ();
-    for (i = 0; i < XVECTOR (items)->size; i += 4)
-      {
-	Lisp_Object key, string, maps;
-
-	key = XVECTOR (items)->contents[i];
-	string = XVECTOR (items)->contents[i + 1];
-	maps = XVECTOR (items)->contents[i + 2];
-	if (NILP (string))
-	  break;
-
-	wv = single_submenu (key, string, maps);
-	if (prev_wv) 
-	  prev_wv->next = wv;
-	else
-	  first_wv->contents = wv;
-	/* Don't set wv->name here; GC during the loop might relocate it.  */
-	wv->enabled = 1;
-	wv->button_type = BUTTON_TYPE_NONE;
-	prev_wv = wv;
-      }
-
-    finish_menu_items ();
-
-    set_buffer_internal_1 (prev);
-    unbind_to (specpdl_count, Qnil);
-
-    /* If there has been no change in the Lisp-level contents
-       of the menu bar, skip redisplaying it.  Just exit.  */
-
-    for (i = 0; i < previous_menu_items_used; i++)
-    if (menu_items_used == i
-	|| (!EQ (previous_items[i], XVECTOR (menu_items)->contents[i])))
-      break;
-    if (i == menu_items_used && i == previous_menu_items_used && i != 0)
-      {
-	free_menubar_widget_value_tree (first_wv);
-	menu_items = Qnil;
-
-	return;
-      }
-
-    /* Now GC cannot happen during the lifetime of the widget_value,
-       so it's safe to store data from a Lisp_String.  */
-    wv = first_wv->contents;
-    for (i = 0; i < XVECTOR (items)->size; i += 4)
-      {
-	Lisp_Object string;
-	string = XVECTOR (items)->contents[i + 1];
-	if (NILP (string))
+  if (deep_p)
+    {
+      /* Make a widget-value tree representing the entire menu trees.  */
+
+      struct buffer *prev = current_buffer;
+      Lisp_Object buffer;
+      int specpdl_count = specpdl_ptr - specpdl;
+      int previous_menu_items_used = f->menu_bar_items_used;
+      Lisp_Object *previous_items
+	= (Lisp_Object *) alloca (previous_menu_items_used
+				  * sizeof (Lisp_Object));
+
+      /* If we are making a new widget, its contents are empty,
+	 do always reinitialize them.  */
+      if (! menubar_widget)
+	previous_menu_items_used = 0;
+      
+      buffer = XWINDOW (FRAME_SELECTED_WINDOW (f))->buffer;
+      specbind (Qinhibit_quit, Qt);
+      /* Don't let the debugger step into this code
+	 because it is not reentrant.  */
+      specbind (Qdebug_on_next_call, Qnil);
+
+      record_unwind_protect (Fset_match_data, Fmatch_data (Qnil, Qnil));
+      if (NILP (Voverriding_local_map_menu_flag))
+	{
+	  specbind (Qoverriding_terminal_local_map, Qnil);
+	  specbind (Qoverriding_local_map, Qnil);
+	}
+
+      set_buffer_internal_1 (XBUFFER (buffer));
+
+      /* Run the Lucid hook.  */
+      call1 (Vrun_hooks, Qactivate_menubar_hook);
+      /* If it has changed current-menubar from previous value,
+	 really recompute the menubar from the value.  */
+      if (! NILP (Vlucid_menu_bar_dirty_flag))
+	call0 (Qrecompute_lucid_menubar);
+      safe_run_hooks (Qmenu_bar_update_hook);
+      FRAME_MENU_BAR_ITEMS (f) = menu_bar_items (FRAME_MENU_BAR_ITEMS (f));
+
+      items = FRAME_MENU_BAR_ITEMS (f);
+
+      inhibit_garbage_collection ();
+
+      /* Save the frame's previous menu bar contents data.  */
+      if (previous_menu_items_used)
+	bcopy (XVECTOR (f->menu_bar_vector)->contents, previous_items,
+	       previous_menu_items_used * sizeof (Lisp_Object));
+
+      /* Fill in the current menu bar contents.  */
+      menu_items = f->menu_bar_vector;
+      menu_items_allocated = VECTORP (menu_items) ? ASIZE (menu_items) : 0;
+      init_menu_items ();
+      for (i = 0; i < XVECTOR (items)->size; i += 4)
+	{
+	  Lisp_Object key, string, maps;
+
+	  key = XVECTOR (items)->contents[i];
+	  string = XVECTOR (items)->contents[i + 1];
+	  maps = XVECTOR (items)->contents[i + 2];
+	  if (NILP (string))
+	    break;
+
+	  wv = single_submenu (key, string, maps);
+	  if (prev_wv) 
+	    prev_wv->next = wv;
+	  else
+	    first_wv->contents = wv;
+	  /* Don't set wv->name here; GC during the loop might relocate it.  */
+	  wv->enabled = 1;
+	  wv->button_type = BUTTON_TYPE_NONE;
+	  prev_wv = wv;
+	}
+
+      finish_menu_items ();
+
+      set_buffer_internal_1 (prev);
+      unbind_to (specpdl_count, Qnil);
+
+      /* If there has been no change in the Lisp-level contents
+	 of the menu bar, skip redisplaying it.  Just exit.  */
+
+      for (i = 0; i < previous_menu_items_used; i++)
+	if (menu_items_used == i
+	    || (!EQ (previous_items[i], XVECTOR (menu_items)->contents[i])))
 	  break;
-	wv->name = (char *) XSTRING (string)->data;
-	wv = wv->next;
-      }
-
-    f->menu_bar_vector = menu_items;
-    f->menu_bar_items_used = menu_items_used;
-    menu_items = Qnil;
-  }
+      if (i == menu_items_used && i == previous_menu_items_used && i != 0)
+	{
+	  free_menubar_widget_value_tree (first_wv);
+	  menu_items = Qnil;
+	  
+	  return;
+	}
+
+      /* Now GC cannot happen during the lifetime of the widget_value,
+	 so it's safe to store data from a Lisp_String.  */
+      wv = first_wv->contents;
+      for (i = 0; i < XVECTOR (items)->size; i += 4)
+	{
+	  Lisp_Object string;
+	  string = XVECTOR (items)->contents[i + 1];
+	  if (NILP (string))
+	    break;
+	  wv->name = (char *) XSTRING (string)->data;
+	  wv = wv->next;
+	}
+
+      f->menu_bar_vector = menu_items;
+      f->menu_bar_items_used = menu_items_used;
+      menu_items = Qnil;
+    }
+  else
+    {
+      /* Make a widget-value tree containing
+	 just the top level menu bar strings.  */
+
+      items = FRAME_MENU_BAR_ITEMS (f);
+      for (i = 0; i < XVECTOR (items)->size; i += 4)
+	{
+	  Lisp_Object string;
+
+	  string = XVECTOR (items)->contents[i + 1];
+	  if (NILP (string))
+	    break;
+
+	  wv = xmalloc_widget_value ();
+	  wv->name = (char *) XSTRING (string)->data;
+	  wv->value = 0;
+	  wv->enabled = 1;
+	  wv->button_type = BUTTON_TYPE_NONE;
+	  /* This prevents lwlib from assuming this
+	     menu item is really supposed to be empty.  */
+	  /* The EMACS_INT cast avoids a warning.
+	     This value just has to be different from small integers.  */
+	  wv->call_data = (void *) (EMACS_INT) (-1);
+
+	  if (prev_wv) 
+	    prev_wv->next = wv;
+	  else
+	    first_wv->contents = wv;
+	  prev_wv = wv;
+	}
+
+      /* Forget what we thought we knew about what is in the
+	 detailed contents of the menu bar menus.
+	 Changing the top level always destroys the contents.  */
+      f->menu_bar_items_used = 0;
+    }
 
   /* Create or update the menu bar widget.  */