[Bug tui/13378] New: gdbtui updates source window for non-stopping conditional breakpoints

[Bug tui/13378] New: gdbtui updates source window for non-stopping conditional breakpoints

[dje at google dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=13378

             Bug #: 13378
           Summary: gdbtui updates source window for non-stopping
                    conditional breakpoints
           Product: gdb
           Version: HEAD
            Status: NEW
          Severity: normal
          Priority: P2
         Component: tui
        AssignedTo: unassigned@sourceware.org
        ReportedBy: dje@google.com
    Classification: Unclassified


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.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug tui/13378] gdbtui updates source window for non-stopping conditional breakpoints

[tromey at redhat dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=13378

Tom Tromey <tromey at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |tromey at redhat dot com

--- Comment #1 from Tom Tromey <tromey at redhat dot com> 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.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug tui/13378] gdbtui updates source window for non-stopping conditional breakpoints

[cvs-commit at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=13378

--- Comment #3 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
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.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug tui/13378] gdbtui updates source window for non-stopping conditional breakpoints

[cvs-commit at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=13378

--- Comment #2 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
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.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug tui/13378] gdbtui updates source window for non-stopping conditional breakpoints

[patrick at parcs dot ath.cx]

https://sourceware.org/bugzilla/show_bug.cgi?id=13378

patrick at parcs dot ath.cx changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
                 CC|                            |patrick at parcs dot ath.cx
         Resolution|---                         |FIXED

--- Comment #4 from patrick at parcs dot ath.cx ---
Should be fixed!

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug tui/13378] gdbtui updates source window for non-stopping conditional breakpoints

[patrick at parcs dot ath.cx]

https://sourceware.org/bugzilla/show_bug.cgi?id=13378

patrick at parcs dot ath.cx changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |7.9

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug tui/13378] gdbtui updates source window for non-stopping conditional breakpoints

[palves at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=13378

Pedro Alves <palves at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |palves at redhat dot com
   Target Milestone|7.9                         |7.10

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug tui/13378] gdbtui updates source window for non-stopping conditional breakpoints

[cvs-commit at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=13378

--- Comment #5 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
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.

-- 
You are receiving this mail because:
You are on the CC list for the bug.