From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 61726 invoked by alias); 26 Jun 2015 14:10:13 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 61715 invoked by uid 89); 26 Jun 2015 14:10:13 -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-ob0-f179.google.com Received: from mail-ob0-f179.google.com (HELO mail-ob0-f179.google.com) (209.85.214.179) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Fri, 26 Jun 2015 14:10:06 +0000 Received: by obctg8 with SMTP id tg8so67446406obc.3 for ; Fri, 26 Jun 2015 07:10:05 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc:content-type; bh=RcYL4EPPeeZXkpzVMQ0UccjdJ9JwpY/kXJ7EYHlQToU=; b=Dw/3gHR1L92ifFmmI0MuxtFMyt5EF1VxsSBWdPghLbZXHMcV4pnDIlcUM2Rq0caG1O jzUrz4hXBB5B8cT79d8ySYpcepzMMb9kkvaoX+PIsXbfov03HWNw44dxPjXvgHLA47PD YqOOCGQXjJPZTsf1p5LZTHloPHy8YWhZvd6Oa+mZPcigh9pEMRwXqjxVTQ2dboWfDRxq q40fvmvMdoAItK07lcAMPlF+2hvfanzd3SNcw0tF/uUKngX1IGMb9kMV8fzp4IjJqWwa NMghPlF4mFha6rNg4BZSZrvp5GEncPciO5Tsh7rYPs6EFJOTSYVy1lv6S7DePMCSTUhH 4UJg== X-Gm-Message-State: ALoCoQmpHm78sKAadSL+V6BYrWDOxU9daP5vpUsPq4OObK4Y0iwDtj8ZdISHZN9bdp3+f57lmPH0 X-Received: by 10.182.102.129 with SMTP id fo1mr1679742obb.24.1435327804911; Fri, 26 Jun 2015 07:10:04 -0700 (PDT) MIME-Version: 1.0 Received: by 10.182.96.167 with HTTP; Fri, 26 Jun 2015 07:09:45 -0700 (PDT) In-Reply-To: <558D5562.5090106@redhat.com> References: <1434688566-2549-1-git-send-email-patrick@parcs.ath.cx> <558D5562.5090106@redhat.com> From: Patrick Palka Date: Fri, 26 Jun 2015 14:10:00 -0000 Message-ID: Subject: Re: [PATCH] Fix TUI flicker resulting from frequent frame changes (PR tui/13378) To: Pedro Alves Cc: "gdb-patches@sourceware.org" Content-Type: text/plain; charset=UTF-8 X-SW-Source: 2015-06/txt/msg00573.txt.bz2 On Fri, Jun 26, 2015 at 9:36 AM, Pedro Alves 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 ) 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. */ >> > >