[binutils-gdb] Replace TUI's select_frame hook (PR tui/13378)

[Patrick Palka <ppalka at sourceware dot org>]

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

commit 0986c744dfecb8177de90020646090e9ed23cfe7
Author: Patrick Palka <patrick@parcs.ath.cx>
Date:   Tue Jun 30 13:56:49 2015 -0400

    Replace TUI's select_frame hook (PR tui/13378)
    
    The select_frame hook is used by TUI to update TUI's frame and register
    information following changes to the selected frame.  The problem with
    this hook is that it gets called after every single frame change, even
    if the frame change is only temporary or internal.  This is the primary
    cause of flickering and slowdown when running the inferior under TUI
    with conditional breakpoints set.  Internal GDB events are the source of
    many calls to select_frame and these internal events are triggered
    frequently, especially when a few conditional breakpoints are set.
    
    This patch removes the select_frame hook altogether and instead makes
    the frame and register information get updated in two key places (using
    observers): after an inferior stops, and right before displaying a
    prompt.  The latter hook covers the case when frame information must be
    updated following a call to "up", "down" or "frame", and the former
    covers the case when frame and register information must be updated
    after a call to "continue", "step", etc. or after the inferior stops in
    async execution mode.  Together these hooks should cover all the cases
    when frame information ought to be refreshed (and when the relevant
    windows ought to be subsequently updated).
    
    The print_frame_info_listing hook is also effectively obsolete now, but
    it still must be set while the TUI is active because its caller
    print_frame_info will otherwise assume that the CLI is active, and will
    print the frame informaion accordingly.  So this patch also sets the
    print_frame_info_listing hook to a dummy callback, in lieu of outright
    removing it yet.
    
    Effectively, with this patch, frame/PC changes that do not immediately
    precede an inferior-stop event or a prompt display event no longer cause
    TUI's frame and register information to be updated.
    
    And as a result of this change and of the previous change to
    tui_show_frame_info, the TUI is much more disciplined about updating the
    screen, and so the flicker as described in the PR is totally gone.
    
    gdb/ChangeLog:
    
    	PR tui/13378
    	* frame.c (select_frame): Remove reference to
    	deprecated_selected_frame_level_changed_hook.
    	* frame.h (deprecated_selected_frame_level_changed_hook): Remove
    	declaration.
    	* stack.c (deprecated_selected_frame_level_changed_hook):
    	Likewise.
    	* tui/tui-hooks.c (tui_selected_frame_level_changed_hook):
    	Rename to ...
    	(tui_refresh_frame_and_register_information): ... this.  Bail
    	out if there is no stack.  Don't update register information
    	unless registers_too_p is true.
    	(tui_print_frame_info_listing_hook): Rename to ...
    	(tui_dummy_print_frame_info_listing_hook): ... this.
    	(tui_before_prompt): New function.
    	(tui_normal_stop): New function.
    	(tui_before_prompt_observer): New observer.
    	(tui_normal_stop_observer): New observer.
    	(tui_install_hooks): Set
    	deprecated_print_frame_info_listing_hook to
    	tui_dummy_print_frame_info_listing_hook.  Register
    	tui_before_prompt_observer to call tui_before_prompt and
    	tui_normal_stop_observer to call tui_normal_stop.  Remove
    	reference to deprecated_selected_frame_level_changed_hook.
    	(tui_remove_hooks): Detach and unset tui_before_prompt_observer
    	and tui_normal_stop_observer.  Remove reference to
    	deprecated_selected_frame_level_changed_hook.

Diff:
---
 gdb/ChangeLog       | 30 +++++++++++++++++++++++
 gdb/frame.c         |  2 --
 gdb/frame.h         |  2 --
 gdb/stack.c         |  2 --
 gdb/tui/tui-hooks.c | 70 +++++++++++++++++++++++++++++++++++++++--------------
 5 files changed, 82 insertions(+), 24 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 2bef30b..2dc32d8 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,6 +1,36 @@
 2015-06-30  Patrick Palka  <patrick@parcs.ath.cx>
 
 	PR tui/13378
+	* frame.c (select_frame): Remove reference to
+	deprecated_selected_frame_level_changed_hook.
+	* frame.h (deprecated_selected_frame_level_changed_hook): Remove
+	declaration.
+	* stack.c (deprecated_selected_frame_level_changed_hook):
+	Likewise.
+	* tui/tui-hooks.c (tui_selected_frame_level_changed_hook):
+	Rename to ...
+	(tui_refresh_frame_and_register_information): ... this.  Bail
+	out if there is no stack.  Don't update register information
+	unless registers_too_p is true.
+	(tui_print_frame_info_listing_hook): Rename to ...
+	(tui_dummy_print_frame_info_listing_hook): ... this.
+	(tui_before_prompt): New function.
+	(tui_normal_stop): New function.
+	(tui_before_prompt_observer): New observer.
+	(tui_normal_stop_observer): New observer.
+	(tui_install_hooks): Set
+	deprecated_print_frame_info_listing_hook to
+	tui_dummy_print_frame_info_listing_hook.  Register
+	tui_before_prompt_observer to call tui_before_prompt and
+	tui_normal_stop_observer to call tui_normal_stop.  Remove
+	reference to deprecated_selected_frame_level_changed_hook.
+	(tui_remove_hooks): Detach and unset tui_before_prompt_observer
+	and tui_normal_stop_observer.  Remove reference to
+	deprecated_selected_frame_level_changed_hook.
+
+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.
diff --git a/gdb/frame.c b/gdb/frame.c
index b3cbf23..da5bfb9 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -1576,8 +1576,6 @@ select_frame (struct frame_info *fi)
   selected_frame = fi;
   /* NOTE: cagney/2002-05-04: FI can be NULL.  This occurs when the
      frame is being invalidated.  */
-  if (deprecated_selected_frame_level_changed_hook)
-    deprecated_selected_frame_level_changed_hook (frame_relative_level (fi));
 
   /* FIXME: kseitz/2002-08-28: It would be nice to call
      selected_frame_level_changed_event() right here, but due to limitations
diff --git a/gdb/frame.h b/gdb/frame.h
index 53a93e0..be64c57 100644
--- a/gdb/frame.h
+++ b/gdb/frame.h
@@ -763,8 +763,6 @@ extern void args_info (char *, int);
 
 extern void locals_info (char *, int);
 
-extern void (*deprecated_selected_frame_level_changed_hook) (int);
-
 extern void return_command (char *, int);
 
 /* Set FRAME's unwinder temporarily, so that we can call a sniffer.
diff --git a/gdb/stack.c b/gdb/stack.c
index eea575a..39803d9 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -52,8 +52,6 @@
 #include "symfile.h"
 #include "extension.h"
 
-void (*deprecated_selected_frame_level_changed_hook) (int);
-
 /* The possible choices of "set print frame-arguments", and the value
    of this setting.  */
 
diff --git a/gdb/tui/tui-hooks.c b/gdb/tui/tui-hooks.c
index 8d84551..5987209 100644
--- a/gdb/tui/tui-hooks.c
+++ b/gdb/tui/tui-hooks.c
@@ -119,18 +119,19 @@ tui_about_to_proceed (void)
   tui_target_has_run = 1;
 }
 
-/* The selected frame has changed.  This is happens after a target
-   stop or when the user explicitly changes the frame
-   (up/down/thread/...).  */
+/* Refresh TUI's frame and register information.  This is a hook intended to be
+   used to update the screen after potential frame and register changes.
+
+   REGISTERS_TOO_P controls whether to refresh our register information.  */
+
 static void
-tui_selected_frame_level_changed_hook (int level)
+tui_refresh_frame_and_register_information (int registers_too_p)
 {
   struct frame_info *fi;
   CORE_ADDR pc;
   struct cleanup *old_chain;
 
-  /* Negative level means that the selected frame was cleared.  */
-  if (level < 0)
+  if (!has_stack_frames ())
     return;
 
   old_chain = make_cleanup_restore_target_terminal ();
@@ -158,7 +159,7 @@ tui_selected_frame_level_changed_hook (int level)
   tui_show_frame_info (fi);
 
   /* Refresh the register window if it's visible.  */
-  if (tui_is_window_visible (DATA_WIN))
+  if (tui_is_window_visible (DATA_WIN) && registers_too_p)
     {
       tui_refreshing_registers = 1;
       tui_check_data_values (fi);
@@ -168,15 +169,15 @@ tui_selected_frame_level_changed_hook (int level)
   do_cleanups (old_chain);
 }
 
-/* Called from print_frame_info to list the line we stopped in.  */
+/* Dummy callback for deprecated_print_frame_info_listing_hook which is called
+   from print_frame_info.  */
+
 static void
-tui_print_frame_info_listing_hook (struct symtab *s,
-				   int line,
-                                   int stopline, 
-				   int noerror)
+tui_dummy_print_frame_info_listing_hook (struct symtab *s,
+					 int line,
+					 int stopline, 
+					 int noerror)
 {
-  select_source_symtab (s);
-  tui_show_frame_info (get_selected_frame (NULL));
 }
 
 /* Perform all necessary cleanups regarding our module's inferior data
@@ -191,21 +192,47 @@ tui_inferior_exit (struct inferior *inf)
   tui_display_main ();
 }
 
+/* Observer for the before_prompt notification.  */
+
+static void
+tui_before_prompt (const char *current_gdb_prompt)
+{
+  /* This refresh is intended to catch changes to the selected frame following
+     a call to "up", "down" or "frame".  As such we don't necessarily want to
+     refresh registers here as they could not have changed.  Registers will be
+     refreshed after a normal stop or by our tui_register_changed_hook.  */
+  tui_refresh_frame_and_register_information (/*registers_too_p=*/0);
+}
+
+/* Observer for the normal_stop notification.  */
+
+static void
+tui_normal_stop (struct bpstats *bs, int print_frame)
+{
+  /* This refresh is intended to catch changes to the selected frame and to
+     registers following a normal stop.  */
+  tui_refresh_frame_and_register_information (/*registers_too_p=*/1);
+}
+
 /* Observers created when installing TUI hooks.  */
 static struct observer *tui_bp_created_observer;
 static struct observer *tui_bp_deleted_observer;
 static struct observer *tui_bp_modified_observer;
 static struct observer *tui_inferior_exit_observer;
 static struct observer *tui_about_to_proceed_observer;
+static struct observer *tui_before_prompt_observer;
+static struct observer *tui_normal_stop_observer;
 
 /* Install the TUI specific hooks.  */
 void
 tui_install_hooks (void)
 {
-  deprecated_selected_frame_level_changed_hook
-    = tui_selected_frame_level_changed_hook;
+  /* If this hook is not set to something then print_frame_info will
+     assume that the CLI, not the TUI, is active, and will print the frame info
+     for us in such a way that we are not prepared to handle.  This hook is
+     otherwise effectively obsolete.  */
   deprecated_print_frame_info_listing_hook
-    = tui_print_frame_info_listing_hook;
+    = tui_dummy_print_frame_info_listing_hook;
 
   /* Install the event hooks.  */
   tui_bp_created_observer
@@ -218,6 +245,10 @@ tui_install_hooks (void)
     = observer_attach_inferior_exit (tui_inferior_exit);
   tui_about_to_proceed_observer
     = observer_attach_about_to_proceed (tui_about_to_proceed);
+  tui_before_prompt_observer
+    = observer_attach_before_prompt (tui_before_prompt);
+  tui_normal_stop_observer
+    = observer_attach_normal_stop (tui_normal_stop);
 
   deprecated_register_changed_hook = tui_register_changed_hook;
 }
@@ -226,7 +257,6 @@ tui_install_hooks (void)
 void
 tui_remove_hooks (void)
 {
-  deprecated_selected_frame_level_changed_hook = 0;
   deprecated_print_frame_info_listing_hook = 0;
   deprecated_query_hook = 0;
   deprecated_register_changed_hook = 0;
@@ -242,6 +272,10 @@ tui_remove_hooks (void)
   tui_inferior_exit_observer = NULL;
   observer_detach_about_to_proceed (tui_about_to_proceed_observer);
   tui_about_to_proceed_observer = NULL;
+  observer_detach_before_prompt (tui_before_prompt_observer);
+  tui_before_prompt_observer = NULL;
+  observer_detach_normal_stop (tui_normal_stop_observer);
+  tui_normal_stop_observer = NULL;
 }
 
 void _initialize_tui_hooks (void);