Re: [PATCH] Fix TUI flicker resulting from frequent frame changes (PR tui/13378)

[Patrick Palka <patrick@parcs.ath.cx>]

On Fri, Jun 19, 2015 at 12:36 AM, Patrick Palka <patrick@parcs.ath.cx> wrote:
> This patch fixes the perceived flicker of the TUI screen (and subsequent
> slowdown) that most apparent when running an inferior while a
> conditional breakpoint is active.  The cause of the flicker is that each
> internal event GDB responds to is accompanied by a multitude of calls to
> select_frame() and each such call forces the TUI screen to be refreshed.
> We would like to not update the TUI screen after each such frame change.
>
> The fix for this issue is pretty straightforward: do not update the TUI
> screen when select_frame() gets called while synchronous execution of
> the inferior is enabled.  This works because synchronous execution
> remains enabled during the processing of internal events.  And since
> during synchronous execution the user has no control of the TUI anyway,
> it does not hurt to avoid updating the screen then.
>
> The select_frame hook is still undesirable and should be removed, but
> in the meantime this fix is seemingly an effective approximation of a
> more disciplined approach of updating the TUI screen.
>
> [ When the inferior is running while sync_execution is disabled, e.g.
>   via "continue&" it looks like GDB lacks access to frame information
>   thorughout -- and the hook never gets called -- so seemingly no
>   worries in that case.  ]
>
> [ up/down etc still work properl of course  ]
>
> [ I am not sure that the change in tui_on_sync_execution_done is
>   necessary/desirable.  It seems normal_stop already calls
>   select_frame (get_current_frame ()) after sync_execution is toggled
>   off.  ]
>
> gdb/ChangeLog:
>
>         * tui/tui-hooks.c (tui_selected_frame_level_changed_hook): Don't
>         update the screen while synchronous execution is active.
>         * tui/tui-interp.c (tui_on_sync_execution_done): Make sure that
>         TUI's frame information is refreshed.
> ---
>  gdb/tui/tui-hooks.c  | 8 ++++++++
>  gdb/tui/tui-interp.c | 5 +++++
>  2 files changed, 13 insertions(+)
>
> diff --git a/gdb/tui/tui-hooks.c b/gdb/tui/tui-hooks.c
> index 8d84551..ca8358d 100644
> --- a/gdb/tui/tui-hooks.c
> +++ b/gdb/tui/tui-hooks.c
> @@ -133,6 +133,14 @@ tui_selected_frame_level_changed_hook (int level)
>    if (level < 0)
>      return;
>
> +  /* Do not respond to frame changes occurring while synchronous execution is
> +     enabled.  Updating the screen in response to each such frame change just
> +     results in pointless flicker and slowdown.  Once synchronous execution is
> +     done this hook will get called again to ensure that our frame information
> +     is refreshed.  */
> +  if (sync_execution)
> +    return;
> +
>    old_chain = make_cleanup_restore_target_terminal ();
>    target_terminal_ours_for_output ();
>
> diff --git a/gdb/tui/tui-interp.c b/gdb/tui/tui-interp.c
> index 1a5639d..2477536 100644
> --- a/gdb/tui/tui-interp.c
> +++ b/gdb/tui/tui-interp.c
> @@ -107,6 +107,11 @@ tui_on_sync_execution_done (void)
>  {
>    if (!interp_quiet_p (tui_interp))
>      display_gdb_prompt (NULL);
> +
> +  /* Make sure our frame information is refreshed now that synchronous
> +     execution is done.  */
> +  if (tui_active)
> +    deprecated_selected_frame_level_changed_hook (0);

This condition ought to be if (tui_active && has_stack_frames ()) so
that we don't call the hook (and thus, try to lookup frame
information) after the inferior has exited (which would result in a
confusing "No stack." error).

>  }
>
>  /* Observer for the command_error notification.  */
> --
> 2.4.4.410.g43ed522.dirty
>