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

[Patrick Palka <patrick at parcs dot ath dot cx>]

On Fri, Jun 19, 2015 at 1:13 PM, Patrick Palka <patrick@parcs.ath.cx> wrote:
> 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
>>

Ping.