Bug 13378 - gdbtui updates source window for non-stopping conditional breakpoints
Summary: gdbtui updates source window for non-stopping conditional breakpoints
Status: RESOLVED FIXED
Alias: None
Product: gdb
Classification: Unclassified
Component: tui (show other bugs)
Version: HEAD
: P2 normal
Target Milestone: 7.10
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-11-02 16:12 UTC by dje
Modified: 2015-07-01 16:21 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description dje 2011-11-02 16:12:48 UTC
gdbtui updates the source window for non-stopping conditional breakpoints.
This unnecessarily slows things down.

Repro:

#include <stdlib.h>

void foo (int x) { }
void bar (int x) { }

int
main ()
{
  int i;

  for (i = 0; i < 1000; ++i)
    {
      if (i % 2 == 0)
	{
	  foo (i);
	  bar (i);
	}
      else
	{
	  bar (i);
	  foo (i);
	}
      sleep (1);
    }

  return 0;
}

bash$ gdbtui a.out
(gdb) b foo if x == 42
(gdb) b bar if x == 52
(gdb) run

and watch the cursor bounce back and forth between foo and bar.
Comment 1 Tom Tromey 2011-11-03 20:22:41 UTC
I haven't tried recently, but in the past I saw this with
gdb in emacs, using annotations, too.  The emacs modeline would
flicker between running/stopped as gdb evaluated breakpoint
conditions and then continued on.
Comment 2 Sourceware Commits 2015-06-30 17:59:00 UTC
The master branch has been updated by Patrick Palka <ppalka@sourceware.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.
Comment 3 Sourceware Commits 2015-06-30 17:59:05 UTC
The master branch has been updated by Patrick Palka <ppalka@sourceware.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.
Comment 4 patrick 2015-06-30 19:35:12 UTC
Should be fixed!
Comment 5 Sourceware Commits 2015-07-01 16:21:09 UTC
The master branch has been updated by Patrick Palka <ppalka@sourceware.org>:

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

commit bbcbf914a6ad801bfa7d4e56150217a1d53e07af
Author: Patrick Palka <patrick@parcs.ath.cx>
Date:   Wed Jul 1 08:02:09 2015 -0400

    TUI: Make sure to update registers if frame information has changed
    
    When I replaced TUI's frame_changed hook to fix PR tui/13378 I assumed
    that there's no reason to refresh register information following a call
    to "up", "down" or "frame".  This assumption was made to fix the problem
    of refreshing frame information twice following a sync-execution normal
    stop (once in tui_normal_stop and then in tui_before_prompt) -- the
    second refresh removing any highlights made by the first.
    
    I was wrong about that -- GDB's snapshot of register information is
    per-frame, and when the frame changes, registers do too (most
    prominently the %rip and %rsp registers).  So e.g. GDB 7.8 would
    highlight such register changes after invoking "up", "down" or "frame",
    and current GDB does not.
    
    To fix this regression, this patch adds another (sufficient) condition
    for refreshing register information: in
    tui_refresh_frame_and_register_information, always refresh register
    information if frame information has changed.  This makes register
    information get refreshed following a call to "up", "down" or "frame"
    while still avoiding the "double refresh" issue following a normal stop.
    
    This condition may seem to obsolete the existing registers_too_p
    parameter, but it does not: following a normal stop, it is possible that
    registers may have changed while frame information had not.  We could be
    on the exact same PC with different register values.  The new condition
    would not catch such a case, but the registers_too_p condition will.  So
    both conditions seem necessary (and either one is sufficient).
    
    gdb/ChangeLog:
    
    	* tui/tui-hooks.c (tui_refresh_frame_and_register_information):
    	Update commentary.  Always refresh the registers when frame
    	information has changed.
    	* tui/tui-stack.c (tui_show_frame_info): Update commentary.
    	Change return type to int.  Return 1 if frame information has
    	changed, 1 otherwise.
    	(tui_before_prompt): Update commentary.
    	* tui/tui-stack.h (tui_show_frame_info): Change return type to
    	int.