From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24205 invoked by alias); 6 Jan 2015 18:37:18 -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 24191 invoked by uid 89); 6 Jan 2015 18:37:17 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.7 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham 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-SHA encrypted) ESMTPS; Tue, 06 Jan 2015 18:37:15 +0000 Received: by mail-ob0-f179.google.com with SMTP id va2so66994750obc.10 for ; Tue, 06 Jan 2015 10:37:13 -0800 (PST) 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:date :message-id:subject:from:to:cc:content-type; bh=r+wmqGWvL1ANAWx5EubdZMYRIq3YCdfvuzUncPKP0zU=; b=Mx99fAeZEjay5MAHJvFhl7oD44uBVzBoz0ilVlQrwF2iwWx5ApAB67rpd+Twq8rVLA A6EO5jOMu6n5GVfmx24gBEDeNXiy2Zaio98DDrdb/AmT4JSOItc46Ztoo4yXroH0L2fV +ht0w2nIQ2KvNEqAp1TKO0P0DWEpzrWUObzI8uJymRoUkXu+CgBlwcwDw4p90pVp9r2s deai4rQCqI2ZyoAbOHZZuX2nqC6J9QnEUyj0uq6up9McKZ1ZzEZpN5OH5TX+2+xic1Rt N6KKXxy2/eaEZMiyUkWSJQpD7i3qXxQHC/A3j8Olzpj9RhYhmWcIYWqrNQLRVolqb2Lo cMZQ== X-Gm-Message-State: ALoCoQmzgfPK6hVdrMa2n3TS8yrEIFpNL4RsuOvOomaE8TyNA3+pUmDVSjbSvE++RW2vcAZu6R6+ MIME-Version: 1.0 X-Received: by 10.182.27.207 with SMTP id v15mr58346040obg.21.1420569433253; Tue, 06 Jan 2015 10:37:13 -0800 (PST) Received: by 10.182.222.98 with HTTP; Tue, 6 Jan 2015 10:37:13 -0800 (PST) In-Reply-To: <83zj9v7urq.fsf@gnu.org> References: <83zj9v7urq.fsf@gnu.org> Date: Tue, 06 Jan 2015 18:37:00 -0000 Message-ID: Subject: Re: [PATCH] Speed up "gdb -tui" output From: Doug Evans To: Eli Zaretskii Cc: gdb-patches Content-Type: text/plain; charset=UTF-8 X-IsSubscribed: yes X-SW-Source: 2015-01/txt/msg00082.txt.bz2 On Tue, Jan 6, 2015 at 8:12 AM, Eli Zaretskii wrote: > In the TUI mode, we call wrefresh after outputting every single > character. Since fputs_* functions go through fputc_*, i.e. break > their writes into single characters as well, and tui_putc makes a > 1-character string out of every character and passes it to tui_puts, > every time GDB wants to display more than a few characters, the I/O > becomes very slow. > > I guess TUI relies on the curses library or the underlying stdio to do > the buffering, but the Windows build of ncurses doesn't buffer and > doesn't use stdio. > > The simple patch below speeds up GDB output in TUI mode several orders > of magnitude, especially on Windows XP (but there's a very prominent > speedup on Windows 7 as well). > > Any objections (ChangeLog entry will be added, of course)? > > > *** gdb/tui/tui-io.c~1 2015-01-03 11:12:52.187500000 +0200 > --- gdb/tui/tui-io.c 2015-01-06 17:59:55.098500000 +0200 > *************** tui_puts (const char *string) > *** 201,209 **** > TUI_CMD_WIN->detail.command_info.start_line > = TUI_CMD_WIN->detail.command_info.cur_line; > > ! /* We could defer the following. */ > ! wrefresh (w); > ! fflush (stdout); > } > > /* Readline callback. > --- 201,211 ---- > TUI_CMD_WIN->detail.command_info.start_line > = TUI_CMD_WIN->detail.command_info.cur_line; > > ! if (c == '\n') > ! { > ! wrefresh (w); > ! fflush (stdout); > ! } > } > > /* Readline callback. I don't know TUI well enough to know if there are any gotchas here. [I know curses well enough, but memory fades ...] This is ok with me if it doesn't break anything :-), but I think a comment explaining "why things are the way they are" is needed here. It's too bad the original author didn't elaborate on "We could defer the following." I can guess at was was meant. I suspect it was just an offhand wish-list item. Normally one lets curses do the refresh when we get to keyboard input unless something needs to go out before then. But tui_puts can't know which is which. I could invest more time seeing if there's an answer in the historical records. There's nothing here: https://sourceware.org/ml/gdb-patches/2001-07/msg00490.html and I hesitate to spend too much time on this. So while I'm ok with deleting the current comment, I think it would help future readers to replace it with a new one. Maybe something like: /* Since strings are generally output a character at a time (e.g., fputs_* break it up and send us a character at a time that's converted back to a string), blindly calling wrefresh here is really expensive on Windows. PR 12345 Assume that if the caller wants to output a string without a trailing newline that's not a prompt (curses will refresh the screen for us at keyboard input), then the caller knows what s/he is doing and will, if necessary, call wrefresh on his/her own. */ [Yeah, I think this is deserving of a bug report.] Having written all that, asking the caller to know to do wrefresh at the needed times could be onerous. One thing that occurs to me is that gdb does do things like: Reading symbols from foo ... (work work work) done. And if I do that on my standard monster benchmark (fortunately I keep a ready-to-use copy lying around for experiments like this :-)) I see a long pause before "done." is printed. And lo and behold, if I apply your patch I see this: bash$ gdb -tui (gdb) file foo At this point I've hit return but I don't see anything printed. pause pause pause and then finally I see all the output: Reading symbols from foo...done. mumble ... (gdb) So the patch is not ok, at least as is. Sorry! One could put it in an appropriate #ifdef, but maybe there's a better solution. Another possibility would be to do the string -> char -> string processing differently. String printing utilities could accumulate what they want to print and send the output in chunks instead of characters. Then tui_puts could get real strings instead of always getting { char, '\0' }, and maybe that would be enough. I don't have a preference on which way to go yet. I'm just tossing out ideas.