Message ID | 1434688566-2549-1-git-send-email-patrick@parcs.ath.cx |
---|---|
State | New, archived |
Headers |
Received: (qmail 59619 invoked by alias); 19 Jun 2015 04:36:16 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 59610 invoked by uid 89); 19 Jun 2015 04:36:16 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.7 required=5.0 tests=AWL, BAYES_00, KAM_LAZY_DOMAIN_SECURITY, RCVD_IN_DNSWL_LOW autolearn=no version=3.3.2 X-HELO: mail-qk0-f178.google.com Received: from mail-qk0-f178.google.com (HELO mail-qk0-f178.google.com) (209.85.220.178) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Fri, 19 Jun 2015 04:36:15 +0000 Received: by qkhu186 with SMTP id u186so55443279qkh.0 for <gdb-patches@sourceware.org>; Thu, 18 Jun 2015 21:36:13 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=5v+JY3WhMxqPZ8v/l2bIAfy4GADP+AYViZ3HnueM7hk=; b=Zs9ZqbR+Tu48DNyvS+BfAi89Ijw6AHcWHzsQ3CjiTeERf62rFs9aOalNIZJE1O13aZ tY+AUnQyXozXh3XUDdjUdD7n5Rtu+M+OqUtoc5Do8AxhDtq264EMCL8cwO644p5kw4sZ 90roy4dt67K37Tz/6y6DG3wedKh074/WPZKBBOGscddFs0uWMM5k1ACcql+tJ3NNhFj7 ypMQOTpMCDjWU+Jgi3fgJiFUWhIz3lXh+h5LGPEBIyeMYn+bEzJWS+82MBUohapu8h1I gUqsyq3M5R3as4HJiDecQJG/KYQx66UTK19Wa3fr6G4nuMeorDet0+I498+yVKQTss3/ PN5g== X-Gm-Message-State: ALoCoQndX9DjOgQ6eHUzQezM+xC4DP6lFOgcwSIWAbPeQNMZp7QZRHNDnSi6CdPpO7JY/OK8Aee6 X-Received: by 10.55.33.203 with SMTP id f72mr15355697qki.52.1434688572915; Thu, 18 Jun 2015 21:36:12 -0700 (PDT) Received: from localhost.localdomain (ool-4353acd8.dyn.optonline.net. [67.83.172.216]) by mx.google.com with ESMTPSA id z189sm5037010qhd.44.2015.06.18.21.36.11 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Thu, 18 Jun 2015 21:36:12 -0700 (PDT) From: Patrick Palka <patrick@parcs.ath.cx> To: gdb-patches@sourceware.org Cc: Patrick Palka <patrick@parcs.ath.cx> Subject: [PATCH] Fix TUI flicker resulting from frequent frame changes (PR tui/13378) Date: Fri, 19 Jun 2015 00:36:06 -0400 Message-Id: <1434688566-2549-1-git-send-email-patrick@parcs.ath.cx> |
Commit Message
Patrick Palka
June 19, 2015, 4:36 a.m. UTC
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(+)
Comments
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 >
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.
On 06/19/2015 05:36 AM, Patrick Palka 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. ] That can't be right. Whenever GDB stops for any (thread) event, it'll select the current frame. So try putting a conditional breakpoint (whose condition fails) on a tight loop, and then continuing, like: (gdb) b inside_loop if 0 (gdb) c& I see lots of TUI flicker doing this. E.g.: (top-gdb) bt #0 tui_refresh_win (win_info=0xdbb3e0 <exec_info>) at /home/pedro/gdb/mygit/build/../src/gdb/tui/tui-wingeneral.c:38 #1 0x000000000052fcfa in tui_show_exec_info_content (win_info=0x1a2e0e0) at /home/pedro/gdb/mygit/build/../src/gdb/tui/tui-winsource.c:569 #2 0x000000000052fd8a in tui_update_exec_info (win_info=0x1a2e0e0) at /home/pedro/gdb/mygit/build/../src/gdb/tui/tui-winsource.c:598 #3 0x000000000052bc62 in tui_show_frame_info (fi=0x1dbc2a0) at /home/pedro/gdb/mygit/build/../src/gdb/tui/tui-stack.c:429 #4 0x0000000000526656 in tui_selected_frame_level_changed_hook (level=0) at /home/pedro/gdb/mygit/build/../src/gdb/tui/tui-hooks.c:158 #5 0x000000000076a5b9 in select_frame (fi=0x1dbc2a0) at /home/pedro/gdb/mygit/build/../src/gdb/frame.c:1580 #6 0x00000000005ac80d in bpstat_check_breakpoint_conditions (bs=0x3d42a30, ptid=...) at /home/pedro/gdb/mygit/build/../src/gdb/breakpoint.c:5414 #7 0x00000000005acc05 in bpstat_stop_status (aspace=0x1034a40, bp_addr=4196369, ptid=..., ws=0x7fffffffd210) at /home/pedro/gdb/mygit/build/../src/gdb/breakpoint.c:5616 #8 0x000000000062e6bc in handle_signal_stop (ecs=0x7fffffffd1f0) at /home/pedro/gdb/mygit/build/../src/gdb/infrun.c:4529 #9 0x000000000062dbce in handle_inferior_event_1 (ecs=0x7fffffffd1f0) at /home/pedro/gdb/mygit/build/../src/gdb/infrun.c:4195 #10 0x000000000062dc70 in handle_inferior_event (ecs=0x7fffffffd1f0) at /home/pedro/gdb/mygit/build/../src/gdb/infrun.c:4220 #11 0x000000000062bfb9 in fetch_inferior_event (client_data=0x0) at /home/pedro/gdb/mygit/build/../src/gdb/infrun.c:3328 How about we instead find some more higher level place to refresh? I'm thinking that maybe whenever we display the prompt might be a good place (before_prompt observer). With both the prompt and normal_stop, we cover every case that needs a refresh, I think. If we don't want that to always refresh at the prompt, we could still have the tui_selected_frame_level_changed_hook hook, but instead of having that cause an immediate refresh, it'd just set a flag to indicate that the next time we display the prompt, we should refresh everything. That way, multiple frame changes until we reach the prompt cause only one refresh. And if no frame changes, the flag is not set, so no refresh is done at the prompt. But if a simpler unconditional refresh at the prompt works, I'd go for that. Thanks, Pedro Alves > > [ 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); > } > > /* Observer for the command_error notification. */ >
On Fri, Jun 26, 2015 at 9:36 AM, Pedro Alves <palves@redhat.com> wrote: > On 06/19/2015 05:36 AM, Patrick Palka 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. ] > > That can't be right. Whenever GDB stops for any (thread) event, it'll > select the current frame. So try putting a conditional breakpoint (whose > condition fails) on a tight loop, and then continuing, like: > > (gdb) b inside_loop if 0 > (gdb) c& > > I see lots of TUI flicker doing this. E.g.: > > (top-gdb) bt > #0 tui_refresh_win (win_info=0xdbb3e0 <exec_info>) at /home/pedro/gdb/mygit/build/../src/gdb/tui/tui-wingeneral.c:38 > #1 0x000000000052fcfa in tui_show_exec_info_content (win_info=0x1a2e0e0) at /home/pedro/gdb/mygit/build/../src/gdb/tui/tui-winsource.c:569 > #2 0x000000000052fd8a in tui_update_exec_info (win_info=0x1a2e0e0) at /home/pedro/gdb/mygit/build/../src/gdb/tui/tui-winsource.c:598 > #3 0x000000000052bc62 in tui_show_frame_info (fi=0x1dbc2a0) at /home/pedro/gdb/mygit/build/../src/gdb/tui/tui-stack.c:429 > #4 0x0000000000526656 in tui_selected_frame_level_changed_hook (level=0) at /home/pedro/gdb/mygit/build/../src/gdb/tui/tui-hooks.c:158 > #5 0x000000000076a5b9 in select_frame (fi=0x1dbc2a0) at /home/pedro/gdb/mygit/build/../src/gdb/frame.c:1580 > #6 0x00000000005ac80d in bpstat_check_breakpoint_conditions (bs=0x3d42a30, ptid=...) at /home/pedro/gdb/mygit/build/../src/gdb/breakpoint.c:5414 > #7 0x00000000005acc05 in bpstat_stop_status (aspace=0x1034a40, bp_addr=4196369, ptid=..., ws=0x7fffffffd210) at /home/pedro/gdb/mygit/build/../src/gdb/breakpoint.c:5616 > #8 0x000000000062e6bc in handle_signal_stop (ecs=0x7fffffffd1f0) at /home/pedro/gdb/mygit/build/../src/gdb/infrun.c:4529 > #9 0x000000000062dbce in handle_inferior_event_1 (ecs=0x7fffffffd1f0) at /home/pedro/gdb/mygit/build/../src/gdb/infrun.c:4195 > #10 0x000000000062dc70 in handle_inferior_event (ecs=0x7fffffffd1f0) at /home/pedro/gdb/mygit/build/../src/gdb/infrun.c:4220 > #11 0x000000000062bfb9 in fetch_inferior_event (client_data=0x0) at /home/pedro/gdb/mygit/build/../src/gdb/infrun.c:3328 I see.. I did not catch that.. > > How about we instead find some more higher level place > to refresh? I'm thinking that maybe whenever we display the prompt > might be a good place (before_prompt observer). With both the prompt > and normal_stop, we cover every case that needs a refresh, I think. I can imagine a problem with this. It seems that when the screen gets refreshed following a frame change, any scrolling that the user did in the source/asm windows would get undone because the screen gets re-centered on the currently executing line. You can see this by doing "frame 0", scrolling the window a bit and doing "frame 0" again: the scrolling gets undone. So by naively refreshing the source/asm windows before each prompt, we would undo scrolling for benign commands such as "print 1 + 2", "bt", I think... This could be fixed by being smarter about refreshing, by only refreshing the screen in the before_prompt observer if the frame information/PC has changed. > > If we don't want that to always refresh at the prompt, we could still > have the tui_selected_frame_level_changed_hook hook, but instead > of having that cause an immediate refresh, it'd just set a > flag to indicate that the next time we display the prompt, we should > refresh everything. That way, multiple frame changes until we > reach the prompt cause only one refresh. And if no frame changes, > the flag is not set, so no refresh is done at the prompt. > But if a simpler unconditional refresh at the prompt works, I'd > go for that. Setting a flag inside the hook would not be perfect anyway since it seems that many times select_frame() is called on the already-selected frame. It seems it would be better to only refresh the screen if the frame/PC actually changes as mentioned above. This can be checked in the observer itself -- no need for the hook, right? > > Thanks, > Pedro Alves > > >> >> [ 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); >> } >> >> /* Observer for the command_error notification. */ >> > >
On 06/26/2015 03:09 PM, Patrick Palka wrote: > On Fri, Jun 26, 2015 at 9:36 AM, Pedro Alves <palves@redhat.com> wrote: >> How about we instead find some more higher level place >> to refresh? I'm thinking that maybe whenever we display the prompt >> might be a good place (before_prompt observer). With both the prompt >> and normal_stop, we cover every case that needs a refresh, I think. > > I can imagine a problem with this. It seems that when the screen gets > refreshed following a frame change, any scrolling that the user did in > the source/asm windows would get undone because the screen gets > re-centered on the currently executing line. You can see this by > doing "frame 0", scrolling the window a bit and doing "frame 0" again: > the scrolling gets undone. So by naively refreshing the source/asm > windows before each prompt, we would undo scrolling for benign > commands such as "print 1 + 2", "bt", I think... Indeed. > This could be fixed > by being smarter about refreshing, by only refreshing the screen in > the before_prompt observer if the frame information/PC has changed. ... > It seems it would be better to only refresh the screen if the > frame/PC actually changes as mentioned above. This can be checked in > the observer itself -- no need for the hook, right? Yes, I think so. Thanks, Pedro Alves
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); } /* Observer for the command_error notification. */