Speed up "gdb -tui" output
Commit Message
Doug Evans writes:
> Seems like the main source of the problem would be gdb_stdout,
> so let's try to fix that first and go from there.
Here's a prototype.
I left fflush (stdout) in to be conservative.
Does this resolve the performance issues you're seeing well enough?
2015-01-07 Doug Evans <dje@google.com>
PR tui/17810
* tui/tui-command.c (tui_refresh_cmd_win): New function.
* tui/tui-command.c (tui_refresh_cmd_win): Declare.
* tui/tui-file.c: #include tui/tui-command.h.
(tui_file_fputs): Refresh command window if stream is not gdb_stdout.
(tui_file_flush): Refresh command window if stream is gdb_stdout or
gdb_stderr.
* tui/tui-io.c (tui_puts): Remove call to wrefresh, fflush.
(tui_readline_output): Call tui_refresh_cmd_win.
(print_filename): Ditto.
(tui_rl_display_match_list): Ditto.
Comments
> From: Doug Evans <dje@google.com>
> Date: Wed, 7 Jan 2015 13:59:06 -0800
>
> Doug Evans writes:
> > Seems like the main source of the problem would be gdb_stdout,
> > so let's try to fix that first and go from there.
>
> Here's a prototype.
Sorry for a long delay. I tried this (and Pedro's) patch today. They
both do the job, so one of them should IMO be committed, master and
branch.
> @@ -239,6 +243,12 @@ tui_file_flush (struct ui_file *file)
> case astring:
> break;
> case afile:
> + /* There is also gdb_stdlog, gdb_stdtarg, gdb_stdtargerr, but
> + tui_setup_io maps those to gdb_stderr. OTOH, do we need to make
> + this conditional? */
> + if (file == gdb_stdout
> + || file == gdb_stderr)
> + tui_refresh_cmd_win ();
I indeed think that the condition should be removed. I see no need
for it: there's no reason to make any stream displayed on TUI more
than line-buffered.
Thanks.
On Mon, Jan 19, 2015 at 9:55 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Doug Evans <dje@google.com>
>> Date: Wed, 7 Jan 2015 13:59:06 -0800
>>
>> Doug Evans writes:
>> > Seems like the main source of the problem would be gdb_stdout,
>> > so let's try to fix that first and go from there.
>>
>> Here's a prototype.
>
> Sorry for a long delay. I tried this (and Pedro's) patch today. They
> both do the job, so one of them should IMO be committed, master and
> branch.
>
>> @@ -239,6 +243,12 @@ tui_file_flush (struct ui_file *file)
>> case astring:
>> break;
>> case afile:
>> + /* There is also gdb_stdlog, gdb_stdtarg, gdb_stdtargerr, but
>> + tui_setup_io maps those to gdb_stderr. OTOH, do we need to make
>> + this conditional? */
>> + if (file == gdb_stdout
>> + || file == gdb_stderr)
>> + tui_refresh_cmd_win ();
>
> I indeed think that the condition should be removed. I see no need
> for it: there's no reason to make any stream displayed on TUI more
> than line-buffered.
I don't have a preference on which, with one condition.
If we're going to remove the fflush's let's do it as a separate patch.
> Date: Mon, 19 Jan 2015 10:32:48 -0800
> From: Doug Evans <xdje42@gmail.com>
> Cc: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
>
> On Mon, Jan 19, 2015 at 9:55 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> >> From: Doug Evans <dje@google.com>
> >> Date: Wed, 7 Jan 2015 13:59:06 -0800
> >>
> >> Doug Evans writes:
> >> > Seems like the main source of the problem would be gdb_stdout,
> >> > so let's try to fix that first and go from there.
> >>
> >> Here's a prototype.
> >
> > Sorry for a long delay. I tried this (and Pedro's) patch today. They
> > both do the job, so one of them should IMO be committed, master and
> > branch.
> >
> >> @@ -239,6 +243,12 @@ tui_file_flush (struct ui_file *file)
> >> case astring:
> >> break;
> >> case afile:
> >> + /* There is also gdb_stdlog, gdb_stdtarg, gdb_stdtargerr, but
> >> + tui_setup_io maps those to gdb_stderr. OTOH, do we need to make
> >> + this conditional? */
> >> + if (file == gdb_stdout
> >> + || file == gdb_stderr)
> >> + tui_refresh_cmd_win ();
> >
> > I indeed think that the condition should be removed. I see no need
> > for it: there's no reason to make any stream displayed on TUI more
> > than line-buffered.
>
> I don't have a preference on which, with one condition.
> If we're going to remove the fflush's let's do it as a separate patch.
Ping! Can one of these patches be committed, please, master and the
7.9 branch?
On 01/31/2015 09:59 AM, Eli Zaretskii wrote:
>> Date: Mon, 19 Jan 2015 10:32:48 -0800
>> From: Doug Evans <xdje42@gmail.com>
>> I don't have a preference on which, with one condition.
>> If we're going to remove the fflush's let's do it as a separate patch.
I'll do that.
> Ping! Can one of these patches be committed, please, master and the
> 7.9 branch?
Sorry for the radio silence; I was traveling last week.
I'm working on this now.
Thanks,
Pedro Alves
> Date: Tue, 03 Feb 2015 18:52:38 +0100
> From: Pedro Alves <palves@redhat.com>
> CC: gdb-patches@sourceware.org
>
> Sorry for the radio silence; I was traveling last week.
>
> I'm working on this now.
Thank you.
@@ -129,3 +129,18 @@ tui_dispatch_ctrl_char (unsigned int ch)
return c;
}
}
+
+/* Refresh the command window. */
+
+void
+tui_refresh_cmd_win (void)
+{
+ WINDOW *w = TUI_CMD_WIN->generic.handle;
+
+ wrefresh (w);
+
+ /* FIXME: It's not clear why this is here.
+ It was present in the original tui_puts code and is kept in order to
+ not introduce some subtle breakage. */
+ fflush (stdout);
+}
@@ -24,4 +24,6 @@
extern unsigned int tui_dispatch_ctrl_char (unsigned int);
+extern void tui_refresh_cmd_win (void);
+
#endif
@@ -20,7 +20,7 @@
#include "ui-file.h"
#include "tui/tui-file.h"
#include "tui/tui-io.h"
-
+#include "tui/tui-command.h"
#include "tui.h"
/* A ``struct ui_file'' that is compatible with all the legacy
@@ -179,6 +179,10 @@ tui_file_fputs (const char *linebuffer, struct ui_file *file)
else
{
tui_puts (linebuffer);
+ /* gdb_stdout is buffered, and the caller must gdb_flush it at
+ appropriate times. Other streams are not so buffered. */
+ if (file != gdb_stdout)
+ tui_refresh_cmd_win ();
}
}
@@ -239,6 +243,12 @@ tui_file_flush (struct ui_file *file)
case astring:
break;
case afile:
+ /* There is also gdb_stdlog, gdb_stdtarg, gdb_stdtargerr, but
+ tui_setup_io maps those to gdb_stderr. OTOH, do we need to make
+ this conditional? */
+ if (file == gdb_stdout
+ || file == gdb_stderr)
+ tui_refresh_cmd_win ();
fflush (stream->ts_filestream);
break;
}
@@ -158,7 +158,10 @@ tui_putc (char c)
tui_puts (buf);
}
-/* Print the string in the curses command window. */
+/* Print the string in the curses command window.
+ The output is buffered. It is up to the caller to refresh the screen
+ if necessary. */
+
void
tui_puts (const char *string)
{
@@ -187,10 +190,6 @@ tui_puts (const char *string)
TUI_CMD_WIN->detail.command_info.curch);
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.
@@ -316,6 +315,7 @@ tui_readline_output (int error, gdb_client_data data)
{
buf[size] = 0;
tui_puts (buf);
+ tui_refresh_cmd_win ();
}
}
#endif
@@ -367,6 +367,7 @@ print_filename (const char *to_print, const char *full_pathname)
{
PUTX (*s);
}
+ tui_refresh_cmd_win ();
return printed_len;
}
@@ -425,6 +426,7 @@ tui_rl_display_match_list (char **matches, int len, int max)
if (get_y_or_n () == 0)
{
tui_puts ("\n");
+ tui_refresh_cmd_win ();
return;
}
}