[RFA v2 09/17] Change command stats reporting to use class

Tom Tromey
2016-10-13T21:14:43Z

This removes make_command_stats_cleanup in favor of an RAII class.
The patch is reasonably straightforward, but keeping the same
semantics without excessive reindentation required splitting
captured_main in two.

2016-09-26 Tom Tromey <tom@tromey.com>

* maint.h (scoped_command_stats): New class.
(make_command_stats_cleanup): Don't declare.
* maint.c (struct cmd_stats): Remove.
(~scoped_command_stats): Rename from report_command_stats. Now a
destructor.
(scoped_command_stats): Rename from make_command_stats_cleanup.
Now a constructor.
* main.c (captured_main_1): New function. Use
scoped_command_stats.
(captured_main): Call captured_main_1.
* event-top.c (command_handler): Use scoped_command_stats.
---
gdb/ChangeLog | 14 +++++++
gdb/event-top.c | 5 +--
gdb/main.c | 25 +++++++-----
gdb/maint.c | 118 ++++++++++++++++++--------------------------------------
gdb/maint.h | 39 +++++++++++++++++--
5 files changed, 104 insertions(+), 97 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 7c072b5..f476299 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,19 @@
2016-09-26 Tom Tromey <tom@tromey.com>

+ * maint.h (scoped_command_stats): New class.
+ (make_command_stats_cleanup): Don't declare.
+ * maint.c (struct cmd_stats): Remove.
+ (~scoped_command_stats): Rename from report_command_stats. Now a
+ destructor.
+ (scoped_command_stats): Rename from make_command_stats_cleanup.
+ Now a constructor.
+ * main.c (captured_main_1): New function. Use
+ scoped_command_stats.
+ (captured_main): Call captured_main_1.
+ * event-top.c (command_handler): Use scoped_command_stats.
+
+2016-09-26 Tom Tromey <tom@tromey.com>
+
* mi/mi-main.c (mi_cmd_data_read_memory): Use gdb::unique_ptr.
Remove some cleanups.

diff --git a/gdb/event-top.c b/gdb/event-top.c
index 9b0ccbc..c452501 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -562,13 +562,12 @@ void
command_handler (char *command)
{
struct ui *ui = current_ui;
- struct cleanup *stat_chain;
char *c;

if (ui->instream == ui->stdin_stream)
reinitialize_more_filter ();

- stat_chain = make_command_stats_cleanup (1);
+ scoped_command_stats stat_reporter (1);

/* Do not execute commented lines. */
for (c = command; *c == ' ' || *c == '\t'; c++)
@@ -580,8 +579,6 @@ command_handler (char *command)
/* Do any commands attached to breakpoint we stopped at. */
bpstat_do_actions ();
}
-
- do_cleanups (stat_chain);
}

/* Append RL, an input line returned by readline or one of its
diff --git a/gdb/main.c b/gdb/main.c
index 2ea9466..420b4d3 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -441,12 +441,12 @@ typedef struct cmdarg {
/* Define type VEC (cmdarg_s). */
DEF_VEC_O (cmdarg_s);

-static int
-captured_main (void *data)
+static void
+captured_main_1 (struct captured_main_args *context)
{
- struct captured_main_args *context = (struct captured_main_args *) data;
int argc = context->argc;
char **argv = context->argv;
+
static int quiet = 0;
static int set_args = 0;
static int inhibit_home_gdbinit = 0;
@@ -486,14 +486,14 @@ captured_main (void *data)
int save_auto_load;
struct objfile *objfile;

- struct cleanup *pre_stat_chain;
+ struct cleanup *chain;

#ifdef HAVE_SBRK
- /* Set this before calling make_command_stats_cleanup. */
+ /* Set this before constructing scoped_command_stats. */
lim_at_start = (char *) sbrk (0);
#endif

- pre_stat_chain = make_command_stats_cleanup (0);
+ scoped_command_stats stat_reporter (0);

#if defined (HAVE_SETLOCALE) && defined (HAVE_LC_MESSAGES)
setlocale (LC_MESSAGES, "");
@@ -510,7 +510,7 @@ captured_main (void *data)
notice_open_fds ();
save_original_signals_state ();

- make_cleanup (VEC_cleanup (cmdarg_s), &cmdarg_vec);
+ chain = make_cleanup (VEC_cleanup (cmdarg_s), &cmdarg_vec);
dirsize = 1;
dirarg = (char **) xmalloc (dirsize * sizeof (*dirarg));
ndir = 0;
@@ -1139,8 +1139,15 @@ captured_main (void *data)
quit_force (NULL, 0);
}

- /* Show time and/or space usage. */
- do_cleanups (pre_stat_chain);
+ do_cleanups (chain);
+}
+
+static void
+captured_main (void *data)
+{
+ struct captured_main_args *context = (struct captured_main_args *) data;
+
+ captured_main_1 (context);

/* NOTE: cagney/1999-11-07: There is probably no reason for not
moving this loop and the code found in captured_command_loop()
diff --git a/gdb/maint.c b/gdb/maint.c
index d2c9346..06e6766 100644
--- a/gdb/maint.c
+++ b/gdb/maint.c
@@ -752,31 +752,6 @@ static int per_command_symtab;
static struct cmd_list_element *per_command_setlist;
static struct cmd_list_element *per_command_showlist;

-/* Records a run time and space usage to be used as a base for
- reporting elapsed time or change in space. */
-
-struct cmd_stats
-{
- /* Zero if the saved time is from the beginning of GDB execution.
- One if from the beginning of an individual command execution. */
- int msg_type;
- /* Track whether the stat was enabled at the start of the command
- so that we can avoid printing anything if it gets turned on by
- the current command. */
- int time_enabled : 1;
- int space_enabled : 1;
- int symtab_enabled : 1;
- long start_cpu_time;
- struct timeval start_wall_time;
- long start_space;
- /* Total number of symtabs (over all objfiles). */
- int start_nr_symtabs;
- /* A count of the compunits. */
- int start_nr_compunit_symtabs;
- /* Total number of blocks. */
- int start_nr_blocks;
-};
-
/* Set whether to display time statistics to NEW_VALUE
(non-zero means true). */

@@ -827,31 +802,38 @@ count_symtabs_and_blocks (int *nr_symtabs_ptr, int *nr_compunit_symtabs_ptr,
*nr_blocks_ptr = nr_blocks;
}

-/* As indicated by display_time and display_space, report GDB's elapsed time
- and space usage from the base time and space provided in ARG, which
- must be a pointer to a struct cmd_stat. This function is intended
- to be called as a cleanup. */
+/* As indicated by display_time and display_space, report GDB's
+ elapsed time and space usage from the base time and space recorded
+ in this object. */

-static void
-report_command_stats (void *arg)
+scoped_command_stats::~scoped_command_stats ()
{
- struct cmd_stats *start_stats = (struct cmd_stats *) arg;
- int msg_type = start_stats->msg_type;
+ /* Early exit if we're not reporting any stats. It can be expensive to
+ compute the pre-command values so don't collect them at all if we're
+ not reporting stats. Alas this doesn't work in the startup case because
+ we don't know yet whether we will be reporting the stats. For the
+ startup case collect the data anyway (it should be cheap at this point),
+ and leave it to the reporter to decide whether to print them. */
+ if (m_msg_type != 0
+ && !per_command_time
+ && !per_command_space
+ && !per_command_symtab)
+ return;

- if (start_stats->time_enabled && per_command_time)
+ if (this->m_time_enabled && per_command_time)
{
- long cmd_time = get_run_time () - start_stats->start_cpu_time;
+ long cmd_time = get_run_time () - this->m_start_cpu_time;
struct timeval now_wall_time, delta_wall_time, wait_time;

gettimeofday (&now_wall_time, NULL);
timeval_sub (&delta_wall_time,
- &now_wall_time, &start_stats->start_wall_time);
+ &now_wall_time, &this->m_start_wall_time);

/* Subtract time spend in prompt_for_continue from walltime. */
wait_time = get_prompt_for_continue_wait_time ();
timeval_sub (&delta_wall_time, &delta_wall_time, &wait_time);

- printf_unfiltered (msg_type == 0
+ printf_unfiltered (m_msg_type == 0
? _("Startup time: %ld.%06ld (cpu), %ld.%06ld (wall)\n")
: _("Command execution time: %ld.%06ld (cpu), %ld.%06ld (wall)\n"),
cmd_time / 1000000, cmd_time % 1000000,
@@ -859,15 +841,15 @@ report_command_stats (void *arg)
(long) delta_wall_time.tv_usec);
}

- if (start_stats->space_enabled && per_command_space)
+ if (this->m_space_enabled && per_command_space)
{
#ifdef HAVE_SBRK
char *lim = (char *) sbrk (0);

long space_now = lim - lim_at_start;
- long space_diff = space_now - start_stats->start_space;
+ long space_diff = space_now - this->m_start_space;

- printf_unfiltered (msg_type == 0
+ printf_unfiltered (m_msg_type == 0
? _("Space used: %ld (%s%ld during startup)\n")
: _("Space used: %ld (%s%ld for this command)\n"),
space_now,
@@ -876,7 +858,7 @@ report_command_stats (void *arg)
#endif
}

- if (start_stats->symtab_enabled && per_command_symtab)
+ if (this->m_symtab_enabled && per_command_symtab)
{
int nr_symtabs, nr_compunit_symtabs, nr_blocks;

@@ -885,54 +867,32 @@ report_command_stats (void *arg)
" #compunits: %d (+%d),"
" #blocks: %d (+%d)\n"),
nr_symtabs,
- nr_symtabs - start_stats->start_nr_symtabs,
+ nr_symtabs - this->m_start_nr_symtabs,
nr_compunit_symtabs,
(nr_compunit_symtabs
- - start_stats->start_nr_compunit_symtabs),
+ - this->m_start_nr_compunit_symtabs),
nr_blocks,
- nr_blocks - start_stats->start_nr_blocks);
+ nr_blocks - this->m_start_nr_blocks);
}
}

-/* Create a cleanup that reports time and space used since its creation.
- MSG_TYPE is zero for gdb startup, otherwise it is one(1) to report
- data for individual commands. */
-
-struct cleanup *
-make_command_stats_cleanup (int msg_type)
+scoped_command_stats::scoped_command_stats (int msg_type)
+: m_msg_type (msg_type)
{
- struct cmd_stats *new_stat;
-
- /* Early exit if we're not reporting any stats. It can be expensive to
- compute the pre-command values so don't collect them at all if we're
- not reporting stats. Alas this doesn't work in the startup case because
- we don't know yet whether we will be reporting the stats. For the
- startup case collect the data anyway (it should be cheap at this point),
- and leave it to the reporter to decide whether to print them. */
- if (msg_type != 0
- && !per_command_time
- && !per_command_space
- && !per_command_symtab)
- return make_cleanup (null_cleanup, 0);
-
- new_stat = XCNEW (struct cmd_stats);
-
- new_stat->msg_type = msg_type;
-
- if (msg_type == 0 || per_command_space)
+ if (m_msg_type == 0 || per_command_space)
{
#ifdef HAVE_SBRK
char *lim = (char *) sbrk (0);
- new_stat->start_space = lim - lim_at_start;
- new_stat->space_enabled = 1;
+ this->m_start_space = lim - lim_at_start;
+ this->m_space_enabled = 1;
#endif
}

if (msg_type == 0 || per_command_time)
{
- new_stat->start_cpu_time = get_run_time ();
- gettimeofday (&new_stat->start_wall_time, NULL);
- new_stat->time_enabled = 1;
+ this->m_start_cpu_time = get_run_time ();
+ gettimeofday (&this->m_start_wall_time, NULL);
+ this->m_time_enabled = 1;
}

if (msg_type == 0 || per_command_symtab)
@@ -940,16 +900,14 @@ make_command_stats_cleanup (int msg_type)
int nr_symtabs, nr_compunit_symtabs, nr_blocks;

count_symtabs_and_blocks (&nr_symtabs, &nr_compunit_symtabs, &nr_blocks);
- new_stat->start_nr_symtabs = nr_symtabs;
- new_stat->start_nr_compunit_symtabs = nr_compunit_symtabs;
- new_stat->start_nr_blocks = nr_blocks;
- new_stat->symtab_enabled = 1;
+ this->m_start_nr_symtabs = nr_symtabs;
+ this->m_start_nr_compunit_symtabs = nr_compunit_symtabs;
+ this->m_start_nr_blocks = nr_blocks;
+ this->m_symtab_enabled = 1;
}

/* Initalize timer to keep track of how long we waited for the user. */
reset_prompt_for_continue_wait_time ();
-
- return make_cleanup_dtor (report_command_stats, new_stat, xfree);
}

/* Handle unknown "mt set per-command" arguments.
diff --git a/gdb/maint.h b/gdb/maint.h
index 841e790..ffbf0cb 100644
--- a/gdb/maint.h
+++ b/gdb/maint.h
@@ -23,9 +23,40 @@ extern void set_per_command_time (int);

extern void set_per_command_space (int);

-/* Note: There's no set_per_command_symtab on purpose.
- Symtab stats aren't yet as useful for --statistics output. */
-
-extern struct cleanup *make_command_stats_cleanup (int);
+/* Records a run time and space usage to be used as a base for
+ reporting elapsed time or change in space. */
+
+class scoped_command_stats
+{
+ public:
+
+ scoped_command_stats (int msg_type);
+ ~scoped_command_stats ();
+
+ private:
+
+ // No need for these. They are intentionally not defined anywhere.
+ scoped_command_stats &operator= (const scoped_command_stats &);
+ scoped_command_stats (const scoped_command_stats &);
+
+ /* Zero if the saved time is from the beginning of GDB execution.
+ One if from the beginning of an individual command execution. */
+ int m_msg_type;
+ /* Track whether the stat was enabled at the start of the command
+ so that we can avoid printing anything if it gets turned on by
+ the current command. */
+ int m_time_enabled : 1;
+ int m_space_enabled : 1;
+ int m_symtab_enabled : 1;
+ long m_start_cpu_time;
+ struct timeval m_start_wall_time;
+ long m_start_space;
+ /* Total number of symtabs (over all objfiles). */
+ int m_start_nr_symtabs;
+ /* A count of the compunits. */
+ int m_start_nr_compunit_symtabs;
+ /* Total number of blocks. */
+ int m_start_nr_blocks;
+};

#endif /* MAINT_H */
--
2.7.4