[binutils-gdb] Be lazy about refreshing the windows in tui_show_frame_info (PR tui/13378)

[Patrick Palka <ppalka at sourceware dot org>]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=b5fca6d7284b2cd2e90efc556eb6a29a1b177476

commit b5fca6d7284b2cd2e90efc556eb6a29a1b177476
Author: Patrick Palka <patrick@parcs.ath.cx>
Date:   Fri Jun 26 20:38:30 2015 -0400

    Be lazy about refreshing the windows in tui_show_frame_info (PR tui/13378)
    
    tui_show_frame_info is responsible for updating the visible windows
    following a change in frame information (that being the currently
    selected frame, PC, line number, etc).  Currently it always redraws and
    refreshes each window even if frame information has not changed.  This
    behavior is inefficient and helps contribute to the occassional
    flickering of the TUI as described in the mentioned PR.
    
    This patch makes tui_show_frame_info refresh the windows only if frame
    information has changed.  Determining whether frame information has
    changed is done indirectly by determining whether the locator has
    changed.  This approach is convenient and yet sensible because the
    locator contains all the relevant info we need to check anyway: the
    current PC, the line number, the name of the executable and the name of
    the current function.  Probably only the PC is really necessary to
    check, but it doesn't hurt to check every field.
    
    Effectively, with this patch, consecutive calls to select_frame with the
    same frame/PC no longer cause TUI's frame information to be updated
    multiple times.
    
    gdb/ChangeLog:
    
    	PR tui/13378
    	* tui/tui-stack.c (tui_set_locator_info): Change prototype to
    	return an int instead of void.  Return whether the locator
    	window has changed.
    	(tui_show_frame_info): If the locator info has not changed, then
    	bail out early to avoid refreshing the windows.

Diff:
---
 gdb/ChangeLog       |  9 +++++++
 gdb/tui/tui-stack.c | 71 +++++++++++++++++++++++++++++++++++++++++------------
 2 files changed, 64 insertions(+), 16 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 5d5e829..2bef30b 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,14 @@
 2015-06-30  Patrick Palka  <patrick@parcs.ath.cx>
 
+	PR tui/13378
+	* tui/tui-stack.c (tui_set_locator_info): Change prototype to
+	return an int instead of void.  Return whether the locator
+	window has changed.
+	(tui_show_frame_info): If the locator info has not changed, then
+	bail out early to avoid refreshing the windows.
+
+2015-06-30  Patrick Palka  <patrick@parcs.ath.cx>
+
 	* tui/tui-stack.c (tui_set_locator_info): Explicitly pass
 	LOCATOR_WIN to tui_alloc_content.
 
diff --git a/gdb/tui/tui-stack.c b/gdb/tui/tui-stack.c
index b17d303..65d18fe 100644
--- a/gdb/tui/tui-stack.c
+++ b/gdb/tui/tui-stack.c
@@ -48,10 +48,10 @@ static char *tui_get_function_from_frame (struct frame_info *fi);
 static void tui_set_locator_fullname (const char *fullname);
 
 /* Update the locator, with the provided arguments.  */
-static void tui_set_locator_info (struct gdbarch *gdbarch,
-				  const char *fullname,
-				  const char *procname,
-                                  int lineno, CORE_ADDR addr);
+static int tui_set_locator_info (struct gdbarch *gdbarch,
+				 const char *fullname,
+				 const char *procname,
+                                 int lineno, CORE_ADDR addr);
 
 static void tui_update_command (char *, int);
 
@@ -292,8 +292,12 @@ tui_set_locator_fullname (const char *fullname)
   strcat_to_buf (element->full_name, MAX_LOCATOR_ELEMENT_LEN, fullname);
 }
 
-/* Update the locator, with the provided arguments.  */
-static void
+/* Update the locator, with the provided arguments.
+
+   Returns 1 if any of the locator's fields were actually changed,
+   and 0 otherwise.  */
+
+static int
 tui_set_locator_info (struct gdbarch *gdbarch,
 		      const char *fullname,
 		      const char *procname, 
@@ -302,21 +306,40 @@ tui_set_locator_info (struct gdbarch *gdbarch,
 {
   struct tui_gen_win_info *locator = tui_locator_win_info_ptr ();
   struct tui_locator_element *element;
+  int locator_changed_p = 0;
 
   /* Allocate the locator content if necessary.  */
   if (locator->content_size <= 0)
     {
       locator->content = tui_alloc_content (1, LOCATOR_WIN);
       locator->content_size = 1;
+      locator_changed_p = 1;
     }
 
+  if (procname == NULL)
+    procname = "";
+
+  if (fullname == NULL)
+    fullname = "";
+
   element = &locator->content[0]->which_element.locator;
+
+  locator_changed_p |= strncmp (element->proc_name, procname,
+				MAX_LOCATOR_ELEMENT_LEN) != 0;
+  locator_changed_p |= lineno != element->line_no;
+  locator_changed_p |= addr != element->addr;
+  locator_changed_p |= gdbarch != element->gdbarch;
+  locator_changed_p |= strncmp (element->full_name, fullname,
+				MAX_LOCATOR_ELEMENT_LEN) != 0;
+
   element->proc_name[0] = (char) 0;
   strcat_to_buf (element->proc_name, MAX_LOCATOR_ELEMENT_LEN, procname);
   element->line_no = lineno;
   element->addr = addr;
   element->gdbarch = gdbarch;
   tui_set_locator_fullname (fullname);
+
+  return locator_changed_p;
 }
 
 /* Update only the full_name portion of the locator.  */
@@ -327,11 +350,14 @@ tui_update_locator_fullname (const char *fullname)
   tui_show_locator_content ();
 }
 
-/* Function to print the frame information for the TUI.  */
+/* Function to print the frame information for the TUI.  The windows are
+   refreshed only if frame information has changed since the last refresh.  */
+
 void
 tui_show_frame_info (struct frame_info *fi)
 {
   struct tui_win_info *win_info;
+  int locator_changed_p;
   int i;
 
   if (fi)
@@ -349,15 +375,23 @@ tui_show_frame_info (struct frame_info *fi)
         && tui_source_is_displayed (symtab_to_fullname (sal.symtab));
 
       if (get_frame_pc_if_available (fi, &pc))
-	tui_set_locator_info (get_frame_arch (fi),
-			      (sal.symtab == 0
-			       ? "??" : symtab_to_fullname (sal.symtab)),
-			      tui_get_function_from_frame (fi),
-			      sal.line,
-			      pc);
+	locator_changed_p
+	  = tui_set_locator_info (get_frame_arch (fi),
+				  (sal.symtab == 0
+				   ? "??" : symtab_to_fullname (sal.symtab)),
+				  tui_get_function_from_frame (fi),
+				  sal.line,
+				  pc);
       else
-	tui_set_locator_info (get_frame_arch (fi),
-			      "??", _("<unavailable>"), sal.line, 0);
+	locator_changed_p
+	  = tui_set_locator_info (get_frame_arch (fi),
+				  "??", _("<unavailable>"), sal.line, 0);
+
+      /* If the locator information has not changed, then frame information has
+	 not changed.  If frame information has not changed, then the windows'
+	 contents will not change.  So don't bother refreshing the windows.  */
+      if (!locator_changed_p)
+	return;
 
       tui_show_locator_content ();
       start_line = 0;
@@ -431,7 +465,12 @@ tui_show_frame_info (struct frame_info *fi)
     }
   else
     {
-      tui_set_locator_info (NULL, NULL, NULL, 0, (CORE_ADDR) 0);
+      locator_changed_p
+	= tui_set_locator_info (NULL, NULL, NULL, 0, (CORE_ADDR) 0);
+
+      if (!locator_changed_p)
+	return;
+
       tui_show_locator_content ();
       for (i = 0; i < (tui_source_windows ())->count; i++)
 	{