* [PATCH] gdb: Use C++11 std::chrono
@ 2016-11-17 17:15 Pedro Alves
2016-11-17 22:15 ` Simon Marchi
0 siblings, 1 reply; 11+ messages in thread
From: Pedro Alves @ 2016-11-17 17:15 UTC (permalink / raw)
To: gdb-patches
This patch fixes a few problems with GDB's time handling.
#1 - It avoids problems with gnulib's C++ namespace support
On MinGW, the struct timeval that should be passed to gnulib's
gettimeofday replacement is incompatible with libiberty's
timeval_sub/timeval_add. That's because gnulib also replaces "struct
timeval" with its own definition, while libiberty expects the
system's.
E.g., in code like this:
gettimeofday (&prompt_ended, NULL);
timeval_sub (&prompt_delta, &prompt_ended, &prompt_started);
timeval_add (&prompt_for_continue_wait_time,
&prompt_for_continue_wait_time, &prompt_delta);
That's currently handled in gdb by not using gnulib's gettimeofday at
all (see common/gdb_sys_time.h), but that #undef hack won't work with
if/when we enable gnulib's C++ namespace support, because that mode
adds compile time warnings for uses of ::gettimeofday, which are hard
errors with -Werror.
#2 - But there's an elephant in the room: gettimeofday is not monotonic...
We're using it to:
a) check how long functions take, for performance analysis
b) compute when in the future to fire events in the event-loop
c) print debug timestamps
But that's exactly what gettimeofday is NOT meant for. Straight from
the man page:
~~~
The time returned by gettimeofday() is affected by
discontinuous jumps in the system time (e.g., if the system
administrator manually changes the system time). If you need a
monotonically increasing clock, see clock_gettime(2).
~~~
std::chrono (part of the C++11 standard library) has a monotonic clock
exactly for such purposes (std::chrono::steady_clock). This commit
switches to use that instead of gettimeofday, fixing all the issues
mentioned above.
std::chrono is a brilliant API IMO, designed to catch problems at
compile time, and to offload factor conversions (minutes <-> seconds
<-> microseconds, etc.) to the library, often at compile time.
It also supports the obvious <,>,-,+ operators, making code comparing
timestamps or computing time ranges more natural. I.e., libiberty's
timeval_add/timeval_sub are just +/- with std::chrono. Checking
whether a timer has elapsed is just using a natural "when > now"
check.
Another nice thing with std::chrono is that it's super easy to create
extra clocks that follow the same interface, and leverage all the
std::chrono::duration/ratio niceties. The patch adds a couple to
count user/system CPU time. See the new cpu_time_clock.h|c files.
The fact that the API is strongly typed and catches problems with
mismatching clocks/time_point at compile time is very nice. It's not
visible in the resulting patch, of course, but that helped write it.
[The best intro I know of to std::chrono is the proposal for the
standard itself:
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2008/n2661.htm
I highly recommend reading that, all the way up to the "threads"
chapter, at least. Should make understanding this patch much easier.]
gdb/ChangeLog:
* Makefile.in (SFILES): Add common/cpu-time-clock.c.
(HFILES_NO_SRCDIR): Add common/cpu-time-clock.h.
(COMMON_OBS): Add cpu-time-clock.o.
(cpu-time-clock.o): New rule.
* common/cpu-time-clock.c, common/cpu-time-clock.h: New files.
* defs.h (struct timeval, print_transfer_performance): Delete
declarations.
* event-loop.c (struct gdb_timer) <when>: Now a
std::chrono::steady_clock::time_point.
(create_timer): use std::chrono::steady_clock instead of
gettimeofday. Use new instead of malloc.
(delete_timer): Use delete instead of xfree.
(duration_cast_timeval): New.
(update_wait_timeout): Use std::chrono::steady_clock instead of
gettimeofday.
gdb/gdbserver/ChangeLog:
* debug.c: Include <chrono> instead of "gdb_sys_time.h".
(debug_vprintf): Use std::chrono::steady_clock instead of
gettimeofday.
* tracepoint.c: Include <chrono> instead of "gdb_sys_time.h".
(get_timestamp): Use std::chrono::steady_clock instead of
gettimeofday.
* maint.c: Include <chrono> instead of "gdb_sys_time.h", <time.h>
and "timeval-utils.h".
(scoped_command_stats::~scoped_command_stats)
(scoped_command_stats::scoped_command_stats): Use
std::chrono::steady_clock instead of gettimeofday. Use
user_cpu_time_clock instead of get_run_time.
* maint.h: Include "cpu-time-clock.h" and <chrono>.
(scoped_command_stats): <m_start_cpu_time>: Now a
user_cpu_time_clock::time_point.
<m_start_wall_time>: Now a std::chrono::steady_clock::time_point.
* mi/mi-main.c: Include "cpu-time-clock.h" and <chrono> instead of
"gdb_sys_time.h" and <sys/resource.h>.
(rusage): Delete.
(mi_execute_command): Use new instead of XNEW.
(mi_load_progress): Use std::chrono::steady_clock instead of
gettimeofday.
(timestamp): Rewrite in terms of std::chrono::steady_clock,
user_cpu_time_clock and system_cpu_time_clock.
(timeval_diff): Delete.
(print_diff): Adjust to use std::chrono::steady_clock,
user_cpu_time_clock and system_cpu_time_clock.
* mi/mi-parse.h: Include "cpu-time-clock.h" and <chrono> instead
of "gdb_sys_time.h".
(struct mi_timestamp): Change fields types to
std::chrono::steady_clock::time_point, user_cpu_time_clock::time
and system_cpu_time_clock::time_point, instead of struct timeval.
* symfile.c: Include <chrono> instead of <time.h> and
"gdb_sys_time.h".
(struct time_range): New.
(generic_load): Use std::chrono::steady_clock instead of
gettimeofday.
(print_transfer_performance): Replace timeval parameters with a
time_range parameter. Adjust.
* utils.c: Include <chrono> instead of "timeval-utils.h",
"gdb_sys_time.h", and <time.h>.
(prompt_for_continue_wait_time): Now a
std::chrono::steady_clock::duration.
(defaulted_query, prompt_for_continue): Use
std::chrono::steady_clock instead of
gettimeofday/timeval_sub/timeval_add.
(reset_prompt_for_continue_wait_time): Use
std::chrono::steady_clock::duration instead of struct timeval.
(get_prompt_for_continue_wait_time): Return a
std::chrono::steady_clock::duration instead of struct timeval.
(vfprintf_unfiltered): Use std::chrono::steady_clock instead of
gettimeofday. Use std::string.
* utils.h: Include <chrono>.
(get_prompt_for_continue_wait_time): Return a
std::chrono::steady_clock::duration instead of struct timeval.
---
gdb/Makefile.in | 9 ++++-
gdb/common/cpu-time-clock.c | 48 +++++++++++++++++++++++
gdb/common/cpu-time-clock.h | 52 +++++++++++++++++++++++++
gdb/defs.h | 14 -------
gdb/event-loop.c | 94 +++++++++++++++++++++------------------------
gdb/gdbserver/debug.c | 13 +++----
gdb/gdbserver/tracepoint.c | 10 ++---
gdb/maint.c | 31 +++++++--------
gdb/maint.h | 7 +++-
gdb/mi/mi-main.c | 75 ++++++++++--------------------------
gdb/mi/mi-parse.h | 12 +++---
gdb/symfile.c | 47 +++++++++++++++--------
gdb/utils.c | 57 +++++++++++----------------
gdb/utils.h | 3 +-
14 files changed, 262 insertions(+), 210 deletions(-)
create mode 100644 gdb/common/cpu-time-clock.c
create mode 100644 gdb/common/cpu-time-clock.h
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index c473472..e3e5ba8 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -895,6 +895,7 @@ SFILES = ada-exp.y ada-lang.c ada-typeprint.c ada-valprint.c ada-tasks.c \
common/errors.c common/common-debug.c common/common-exceptions.c \
common/btrace-common.c common/fileio.c common/common-regcache.c \
common/signals-state-save-restore.c common/new-op.c \
+ common/cpu-time-clock.c \
$(SUBDIR_GCC_COMPILE_SRCS)
LINTFILES = $(SFILES) $(YYFILES) $(CONFIG_SRCS) init.c
@@ -988,7 +989,7 @@ common/common-regcache.h fbsd-tdep.h nat/linux-personality.h \
common/fileio.h nat/x86-linux.h nat/x86-linux-dregs.h nat/amd64-linux-siginfo.h\
nat/linux-namespaces.h arch/arm.h common/gdb_sys_time.h arch/aarch64-insn.h \
tid-parse.h ser-event.h \
-common/signals-state-save-restore.h
+common/signals-state-save-restore.h common/cpu-time-clock.h
# Header files that already have srcdir in them, or which are in objdir.
@@ -1088,7 +1089,7 @@ COMMON_OBS = $(DEPFILES) $(CONFIG_OBS) $(YYOBJ) \
format.o registry.o btrace.o record-btrace.o waitstatus.o \
print-utils.o rsp-low.o errors.o common-debug.o debug.o \
common-exceptions.o btrace-common.o fileio.o \
- common-regcache.o new-op.o \
+ common-regcache.o new-op.o cpu-time-clock.o \
$(SUBDIR_GCC_COMPILE_OBS)
TSOBS = inflow.o
@@ -2298,6 +2299,10 @@ new-op.o: ${srcdir}/common/new-op.c
$(COMPILE) $(srcdir)/common/new-op.c
$(POSTCOMPILE)
+cpu-time-clock.o: ${srcdir}/common/cpu-time-clock.c
+ $(COMPILE) $(srcdir)/common/cpu-time-clock.c
+ $(POSTCOMPILE)
+
#
# gdb/target/ dependencies
#
diff --git a/gdb/common/cpu-time-clock.c b/gdb/common/cpu-time-clock.c
new file mode 100644
index 0000000..3c69532
--- /dev/null
+++ b/gdb/common/cpu-time-clock.c
@@ -0,0 +1,48 @@
+/* User/system CPU time clocks that follow the std::chrono interface.
+ Copyright (C) 2016 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#include "common-defs.h"
+#include "cpu-time-clock.h"
+#if defined HAVE_SYS_RESOURCE_H
+#include <sys/resource.h>
+#endif
+
+using namespace std::chrono;
+
+/* Count the total amount of time spent executing in user mode. */
+
+user_cpu_time_clock::time_point
+user_cpu_time_clock::now () noexcept
+{
+ return time_point (microseconds (get_run_time ()));
+}
+
+/* Count the total amount of time spent executing in kernel mode. */
+
+system_cpu_time_clock::time_point
+system_cpu_time_clock::now () noexcept
+{
+#ifdef HAVE_GETRUSAGE
+ struct rusage rusage;
+ getrusage (RUSAGE_SELF, &rusage);
+ return time_point (seconds (rusage.ru_stime.tv_sec)
+ + microseconds (rusage.ru_stime.tv_usec));
+#else
+ return time_point (duration::zero ());
+#endif
+}
diff --git a/gdb/common/cpu-time-clock.h b/gdb/common/cpu-time-clock.h
new file mode 100644
index 0000000..f982769
--- /dev/null
+++ b/gdb/common/cpu-time-clock.h
@@ -0,0 +1,52 @@
+/* User/system CPU time clocks that follow the std::chrono interface.
+ Copyright (C) 2016 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#ifndef CPU_TIME_CLOCK_H
+#define CPU_TIME_CLOCK_H
+
+#include <chrono>
+
+/* Count the total amount of time spent executing in user mode. */
+
+struct user_cpu_time_clock
+{
+ using duration = std::chrono::microseconds;
+ using rep = duration::rep;
+ using period = duration::period;
+ using time_point = std::chrono::time_point<user_cpu_time_clock>;
+
+ static constexpr bool is_steady = true;
+
+ static time_point now () noexcept;
+};
+
+/* Count the total amount of time spent executing in kernel mode. */
+
+struct system_cpu_time_clock
+{
+ using duration = std::chrono::microseconds;
+ using rep = duration::rep;
+ using period = duration::period;
+ using time_point = std::chrono::time_point<system_cpu_time_clock>;
+
+ static constexpr bool is_steady = true;
+
+ static time_point now () noexcept;
+};
+
+#endif
diff --git a/gdb/defs.h b/gdb/defs.h
index 6d0d2dd..3d21f62 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -303,20 +303,6 @@ extern void symbol_file_command (char *, int);
/* * Remote targets may wish to use this as their load function. */
extern void generic_load (const char *name, int from_tty);
-/* * Report on STREAM the performance of memory transfer operation,
- such as 'load'.
- @param DATA_COUNT is the number of bytes transferred.
- @param WRITE_COUNT is the number of separate write operations, or 0,
- if that information is not available.
- @param START_TIME is the time at which an operation was started.
- @param END_TIME is the time at which an operation ended. */
-struct timeval;
-extern void print_transfer_performance (struct ui_file *stream,
- unsigned long data_count,
- unsigned long write_count,
- const struct timeval *start_time,
- const struct timeval *end_time);
-
/* From top.c */
typedef void initialize_file_ftype (void);
diff --git a/gdb/event-loop.c b/gdb/event-loop.c
index f94a6fa..9e8cf66 100644
--- a/gdb/event-loop.c
+++ b/gdb/event-loop.c
@@ -212,7 +212,7 @@ gdb_notifier;
first occasion after WHEN. */
struct gdb_timer
{
- struct timeval when;
+ std::chrono::steady_clock::time_point when;
int timer_id;
struct gdb_timer *next;
timer_handler_func *proc; /* Function to call to do the work. */
@@ -1097,33 +1097,22 @@ delete_async_event_handler (async_event_handler **async_handler_ptr)
*async_handler_ptr = NULL;
}
-/* Create a timer that will expire in MILLISECONDS from now. When the
- timer is ready, PROC will be executed. At creation, the timer is
- aded to the timers queue. This queue is kept sorted in order of
- increasing timers. Return a handle to the timer struct. */
+/* Create a timer that will expire in MS milliseconds from now. When
+ the timer is ready, PROC will be executed. At creation, the timer
+ is added to the timers queue. This queue is kept sorted in order
+ of increasing timers. Return a handle to the timer struct. */
+
int
-create_timer (int milliseconds, timer_handler_func * proc,
+create_timer (int ms, timer_handler_func *proc,
gdb_client_data client_data)
{
+ using namespace std::chrono;
struct gdb_timer *timer_ptr, *timer_index, *prev_timer;
- struct timeval time_now, delta;
-
- /* Compute seconds. */
- delta.tv_sec = milliseconds / 1000;
- /* Compute microseconds. */
- delta.tv_usec = (milliseconds % 1000) * 1000;
- gettimeofday (&time_now, NULL);
+ steady_clock::time_point time_now = steady_clock::now ();
- timer_ptr = XNEW (struct gdb_timer);
- timer_ptr->when.tv_sec = time_now.tv_sec + delta.tv_sec;
- timer_ptr->when.tv_usec = time_now.tv_usec + delta.tv_usec;
- /* Carry? */
- if (timer_ptr->when.tv_usec >= 1000000)
- {
- timer_ptr->when.tv_sec += 1;
- timer_ptr->when.tv_usec -= 1000000;
- }
+ timer_ptr = new gdb_timer ();
+ timer_ptr->when = time_now + milliseconds (ms);
timer_ptr->proc = proc;
timer_ptr->client_data = client_data;
timer_list.num_timers++;
@@ -1136,11 +1125,7 @@ create_timer (int milliseconds, timer_handler_func * proc,
timer_index != NULL;
timer_index = timer_index->next)
{
- /* If the seconds field is greater or if it is the same, but the
- microsecond field is greater. */
- if ((timer_index->when.tv_sec > timer_ptr->when.tv_sec)
- || ((timer_index->when.tv_sec == timer_ptr->when.tv_sec)
- && (timer_index->when.tv_usec > timer_ptr->when.tv_usec)))
+ if (timer_index->when > timer_ptr->when)
break;
}
@@ -1194,47 +1179,56 @@ delete_timer (int id)
;
prev_timer->next = timer_ptr->next;
}
- xfree (timer_ptr);
+ delete timer_ptr;
gdb_notifier.timeout_valid = 0;
}
+/* Convert a std::chrono duration to a struct timeval. */
+
+template<typename Duration>
+static struct timeval
+duration_cast_timeval (const Duration &d)
+{
+ using namespace std::chrono;
+ seconds sec = duration_cast<seconds> (d);
+ microseconds msec = duration_cast<microseconds> (d - sec);
+
+ struct timeval tv;
+ tv.tv_sec = sec.count ();
+ tv.tv_usec = msec.count ();
+ return tv;
+}
+
/* Update the timeout for the select() or poll(). Returns true if the
timer has already expired, false otherwise. */
static int
update_wait_timeout (void)
{
- struct timeval time_now, delta;
-
if (timer_list.first_timer != NULL)
{
- gettimeofday (&time_now, NULL);
- delta.tv_sec = timer_list.first_timer->when.tv_sec - time_now.tv_sec;
- delta.tv_usec = timer_list.first_timer->when.tv_usec - time_now.tv_usec;
- /* Borrow? */
- if (delta.tv_usec < 0)
- {
- delta.tv_sec -= 1;
- delta.tv_usec += 1000000;
- }
+ using namespace std::chrono;
+ steady_clock::time_point time_now = steady_clock::now ();
+ struct timeval timeout;
- /* Cannot simply test if delta.tv_sec is negative because time_t
- might be unsigned. */
- if (timer_list.first_timer->when.tv_sec < time_now.tv_sec
- || (timer_list.first_timer->when.tv_sec == time_now.tv_sec
- && timer_list.first_timer->when.tv_usec < time_now.tv_usec))
+ if (timer_list.first_timer->when < time_now)
{
/* It expired already. */
- delta.tv_sec = 0;
- delta.tv_usec = 0;
+ timeout.tv_sec = 0;
+ timeout.tv_usec = 0;
+ }
+ else
+ {
+ steady_clock::duration d = timer_list.first_timer->when - time_now;
+ timeout = duration_cast_timeval (d);
}
/* Update the timeout for select/ poll. */
if (use_poll)
{
#ifdef HAVE_POLL
- gdb_notifier.poll_timeout = delta.tv_sec * 1000;
+ gdb_notifier.poll_timeout = timeout.tv_sec * 1000;
#else
internal_error (__FILE__, __LINE__,
_("use_poll without HAVE_POLL"));
@@ -1242,12 +1236,12 @@ update_wait_timeout (void)
}
else
{
- gdb_notifier.select_timeout.tv_sec = delta.tv_sec;
- gdb_notifier.select_timeout.tv_usec = delta.tv_usec;
+ gdb_notifier.select_timeout.tv_sec = timeout.tv_sec;
+ gdb_notifier.select_timeout.tv_usec = timeout.tv_usec;
}
gdb_notifier.timeout_valid = 1;
- if (delta.tv_sec == 0 && delta.tv_usec == 0)
+ if (timer_list.first_timer->when < time_now)
return 1;
}
else
diff --git a/gdb/gdbserver/debug.c b/gdb/gdbserver/debug.c
index 54f2665..dd987ed 100644
--- a/gdb/gdbserver/debug.c
+++ b/gdb/gdbserver/debug.c
@@ -17,7 +17,7 @@
along with this program. If not, see <http://www.gnu.org/licenses/>. */
#include "server.h"
-#include "gdb_sys_time.h"
+#include <chrono>
/* Enable miscellaneous debugging output. The name is historical - it
was originally used to debug LinuxThreads support. */
@@ -41,14 +41,13 @@ debug_vprintf (const char *format, va_list ap)
if (debug_timestamp && new_line)
{
- struct timeval tm;
+ using namespace std::chrono;
- gettimeofday (&tm, NULL);
+ steady_clock::time_point now = steady_clock::now ();
+ seconds s = duration_cast<seconds> (now.time_since_epoch ());
+ microseconds us = duration_cast<microseconds> (now.time_since_epoch ()) - s;
- /* If gettimeofday doesn't exist, and as a portability solution it has
- been replaced with, e.g., time, then it doesn't make sense to print
- the microseconds field. Is there a way to check for that? */
- fprintf (stderr, "%ld:%06ld ", (long) tm.tv_sec, (long) tm.tv_usec);
+ fprintf (stderr, "%ld:%06ld ", s.count (), us.count ());
}
#endif
diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c
index 7700ad1..1444a0d 100644
--- a/gdb/gdbserver/tracepoint.c
+++ b/gdb/gdbserver/tracepoint.c
@@ -25,7 +25,7 @@
#include <ctype.h>
#include <fcntl.h>
#include <unistd.h>
-#include "gdb_sys_time.h"
+#include <chrono>
#include <inttypes.h>
#include "ax.h"
#include "tdesc.h"
@@ -7410,12 +7410,10 @@ getauxval (unsigned long type)
static LONGEST
get_timestamp (void)
{
- struct timeval tv;
+ using namespace std::chrono;
- if (gettimeofday (&tv, 0) != 0)
- return -1;
- else
- return (LONGEST) tv.tv_sec * 1000000 + tv.tv_usec;
+ steady_clock::time_point now = steady_clock::now ();
+ return duration_cast<microseconds> (now.time_since_epoch ()).count ();
}
void
diff --git a/gdb/maint.c b/gdb/maint.c
index 738571c..2a8f067 100644
--- a/gdb/maint.c
+++ b/gdb/maint.c
@@ -24,8 +24,6 @@
#include "arch-utils.h"
#include <ctype.h>
#include <signal.h>
-#include "gdb_sys_time.h"
-#include <time.h>
#include "command.h"
#include "gdbcmd.h"
#include "symtab.h"
@@ -39,7 +37,6 @@
#include "objfiles.h"
#include "value.h"
#include "top.h"
-#include "timeval-utils.h"
#include "maint.h"
#include "selftest.h"
@@ -822,23 +819,21 @@ scoped_command_stats::~scoped_command_stats ()
if (m_time_enabled && per_command_time)
{
- long cmd_time = get_run_time () - m_start_cpu_time;
- struct timeval now_wall_time, delta_wall_time, wait_time;
+ using namespace std::chrono;
- gettimeofday (&now_wall_time, NULL);
- timeval_sub (&delta_wall_time,
- &now_wall_time, &m_start_wall_time);
+ user_cpu_time_clock::duration cmd_time
+ = user_cpu_time_clock::now () - m_start_cpu_time;
+ steady_clock::duration wall_time
+ = steady_clock::now () - 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);
+ wall_time -= get_prompt_for_continue_wait_time ();
printf_unfiltered (!m_msg_type
- ? _("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,
- (long) delta_wall_time.tv_sec,
- (long) delta_wall_time.tv_usec);
+ ? _("Startup time: %.6f (cpu), %.6f (wall)\n")
+ : _("Command execution time: %.6f (cpu), %.6f (wall)\n"),
+ duration<double> (cmd_time).count (),
+ duration<double> (wall_time).count ());
}
if (m_space_enabled && per_command_space)
@@ -892,8 +887,10 @@ scoped_command_stats::scoped_command_stats (bool msg_type)
if (msg_type == 0 || per_command_time)
{
- m_start_cpu_time = get_run_time ();
- gettimeofday (&m_start_wall_time, NULL);
+ using namespace std::chrono;
+
+ m_start_cpu_time = user_cpu_time_clock::now ();
+ m_start_wall_time = steady_clock::now ();
m_time_enabled = 1;
}
else
diff --git a/gdb/maint.h b/gdb/maint.h
index 8d03800..68132ae 100644
--- a/gdb/maint.h
+++ b/gdb/maint.h
@@ -19,6 +19,9 @@
#ifndef MAINT_H
#define MAINT_H
+#include "cpu-time-clock.h"
+#include <chrono>
+
extern void set_per_command_time (int);
extern void set_per_command_space (int);
@@ -48,8 +51,8 @@ class scoped_command_stats
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;
+ user_cpu_time_clock::time_point m_start_cpu_time;
+ std::chrono::steady_clock::time_point m_start_wall_time;
long m_start_space;
/* Total number of symtabs (over all objfiles). */
int m_start_nr_symtabs;
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 984a415..d1ce78c 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -56,15 +56,8 @@
#include "observer.h"
#include <ctype.h>
-#include "gdb_sys_time.h"
-
-#if defined HAVE_SYS_RESOURCE_H
-#include <sys/resource.h>
-#endif
-
-#ifdef HAVE_GETRUSAGE
-struct rusage rusage;
-#endif
+#include "cpu-time-clock.h"
+#include <chrono>
enum
{
@@ -2169,7 +2162,7 @@ mi_execute_command (const char *cmd, int from_tty)
if (do_timings)
{
- command->cmd_start = XNEW (struct mi_timestamp);
+ command->cmd_start = new mi_timestamp ();
timestamp (command->cmd_start);
}
@@ -2390,8 +2383,8 @@ mi_load_progress (const char *section_name,
unsigned long total_sent,
unsigned long grand_total)
{
- struct timeval time_now, delta, update_threshold;
- static struct timeval last_update;
+ using namespace std::chrono;
+ static steady_clock::time_point last_update;
static char *previous_sect_name = NULL;
int new_section;
struct ui_out *saved_uiout;
@@ -2416,18 +2409,9 @@ mi_load_progress (const char *section_name,
uiout = current_uiout;
- update_threshold.tv_sec = 0;
- update_threshold.tv_usec = 500000;
- gettimeofday (&time_now, NULL);
-
- delta.tv_usec = time_now.tv_usec - last_update.tv_usec;
- delta.tv_sec = time_now.tv_sec - last_update.tv_sec;
-
- if (delta.tv_usec < 0)
- {
- delta.tv_sec -= 1;
- delta.tv_usec += 1000000L;
- }
+ microseconds update_threshold (500000);
+ steady_clock::time_point time_now = steady_clock::now ();
+ steady_clock::duration delta = time_now - last_update;
new_section = (previous_sect_name ?
strcmp (previous_sect_name, section_name) : 1);
@@ -2451,13 +2435,11 @@ mi_load_progress (const char *section_name,
gdb_flush (mi->raw_stdout);
}
- if (delta.tv_sec >= update_threshold.tv_sec &&
- delta.tv_usec >= update_threshold.tv_usec)
+ if (delta > update_threshold)
{
struct cleanup *cleanup_tuple;
- last_update.tv_sec = time_now.tv_sec;
- last_update.tv_usec = time_now.tv_usec;
+ last_update = time_now;
if (current_token)
fputs_unfiltered (current_token, mi->raw_stdout);
fputs_unfiltered ("+download", mi->raw_stdout);
@@ -2480,23 +2462,10 @@ mi_load_progress (const char *section_name,
static void
timestamp (struct mi_timestamp *tv)
{
- gettimeofday (&tv->wallclock, NULL);
-#ifdef HAVE_GETRUSAGE
- getrusage (RUSAGE_SELF, &rusage);
- tv->utime.tv_sec = rusage.ru_utime.tv_sec;
- tv->utime.tv_usec = rusage.ru_utime.tv_usec;
- tv->stime.tv_sec = rusage.ru_stime.tv_sec;
- tv->stime.tv_usec = rusage.ru_stime.tv_usec;
-#else
- {
- long usec = get_run_time ();
-
- tv->utime.tv_sec = usec/1000000L;
- tv->utime.tv_usec = usec - 1000000L*tv->utime.tv_sec;
- tv->stime.tv_sec = 0;
- tv->stime.tv_usec = 0;
- }
-#endif
+ using namespace std::chrono;
+ tv->wallclock = steady_clock::now ();
+ tv->utime = user_cpu_time_clock::now ();
+ tv->stime = system_cpu_time_clock::now ();
}
static void
@@ -2517,23 +2486,19 @@ mi_print_timing_maybe (struct ui_file *file)
print_diff_now (file, current_command_ts);
}
-static long
-timeval_diff (struct timeval start, struct timeval end)
-{
- return ((end.tv_sec - start.tv_sec) * 1000000L)
- + (end.tv_usec - start.tv_usec);
-}
-
static void
print_diff (struct ui_file *file, struct mi_timestamp *start,
struct mi_timestamp *end)
{
+ using namespace std::chrono;
+ duration<double> wallclock = end->wallclock - start->wallclock;
+ duration<double> utime = end->utime - start->utime;
+ duration<double> stime = end->stime - start->stime;
+
fprintf_unfiltered
(file,
",time={wallclock=\"%0.5f\",user=\"%0.5f\",system=\"%0.5f\"}",
- timeval_diff (start->wallclock, end->wallclock) / 1000000.0,
- timeval_diff (start->utime, end->utime) / 1000000.0,
- timeval_diff (start->stime, end->stime) / 1000000.0);
+ wallclock.count (), utime.count (), stime.count ());
}
void
diff --git a/gdb/mi/mi-parse.h b/gdb/mi/mi-parse.h
index d5595b3..ac3c654 100644
--- a/gdb/mi/mi-parse.h
+++ b/gdb/mi/mi-parse.h
@@ -20,15 +20,17 @@
#ifndef MI_PARSE_H
#define MI_PARSE_H
-#include "gdb_sys_time.h"
+#include "cpu-time-clock.h"
+#include <chrono>
/* MI parser */
/* Timestamps for current command and last asynchronous command. */
-struct mi_timestamp {
- struct timeval wallclock;
- struct timeval utime;
- struct timeval stime;
+struct mi_timestamp
+{
+ std::chrono::steady_clock::time_point wallclock;
+ user_cpu_time_clock::time_point utime;
+ system_cpu_time_clock::time_point stime;
};
enum mi_command_type
diff --git a/gdb/symfile.c b/gdb/symfile.c
index f524f56..6e791d5 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -61,8 +61,7 @@
#include <fcntl.h>
#include <sys/stat.h>
#include <ctype.h>
-#include <time.h>
-#include "gdb_sys_time.h"
+#include <chrono>
#include "psymtab.h"
@@ -2066,11 +2065,23 @@ clear_memory_write_data (void *arg)
VEC_free (memory_write_request_s, vec);
}
+/* A time range. */
+
+struct time_range
+{
+ std::chrono::steady_clock::time_point start;
+ std::chrono::steady_clock::time_point end;
+};
+
+static void print_transfer_performance (struct ui_file *stream,
+ unsigned long data_count,
+ unsigned long write_count,
+ const time_range &time);
+
void
generic_load (const char *args, int from_tty)
{
bfd *loadfile_bfd;
- struct timeval start_time, end_time;
char *filename;
struct cleanup *old_cleanups = make_cleanup (null_cleanup, 0);
struct load_section_data cbdata;
@@ -2131,13 +2142,15 @@ generic_load (const char *args, int from_tty)
bfd_map_over_sections (loadfile_bfd, load_section_callback, &cbdata);
- gettimeofday (&start_time, NULL);
+ using namespace std::chrono;
+
+ steady_clock::time_point start_time = steady_clock::now ();
if (target_write_memory_blocks (cbdata.requests, flash_discard,
load_progress) != 0)
error (_("Load failed"));
- gettimeofday (&end_time, NULL);
+ steady_clock::time_point end_time = steady_clock::now ();
entry = bfd_get_start_address (loadfile_bfd);
entry = gdbarch_addr_bits_remove (target_gdbarch (), entry);
@@ -2160,32 +2173,32 @@ generic_load (const char *args, int from_tty)
print_transfer_performance (gdb_stdout, total_progress.data_count,
total_progress.write_count,
- &start_time, &end_time);
+ {start_time, end_time});
do_cleanups (old_cleanups);
}
-/* Report how fast the transfer went. */
+/* Report on STREAM the performance of a memory transfer operation,
+ such as 'load'. DATA_COUNT is the number of bytes transferred.
+ WRITE_COUNT is the number of separate write operations, or 0, if
+ that information is not available. TIME is the range of time in
+ which the operation lasted. */
-void
+static void
print_transfer_performance (struct ui_file *stream,
unsigned long data_count,
unsigned long write_count,
- const struct timeval *start_time,
- const struct timeval *end_time)
+ const time_range &time)
{
- ULONGEST time_count;
struct ui_out *uiout = current_uiout;
- /* Compute the elapsed time in milliseconds, as a tradeoff between
- accuracy and overflow. */
- time_count = (end_time->tv_sec - start_time->tv_sec) * 1000;
- time_count += (end_time->tv_usec - start_time->tv_usec) / 1000;
+ using namespace std::chrono;
+ milliseconds ms = duration_cast<milliseconds> (time.end - time.start);
ui_out_text (uiout, "Transfer rate: ");
- if (time_count > 0)
+ if (ms.count () > 0)
{
- unsigned long rate = ((ULONGEST) data_count * 1000) / time_count;
+ unsigned long rate = ((ULONGEST) data_count * 1000) / ms.count ();
if (ui_out_is_mi_like_p (uiout))
{
diff --git a/gdb/utils.c b/gdb/utils.c
index 751f099..73c07ec 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -37,7 +37,6 @@
#endif
#include <signal.h>
-#include "timeval-utils.h"
#include "gdbcmd.h"
#include "serial.h"
#include "bfd.h"
@@ -61,8 +60,7 @@
#include "readline/readline.h"
-#include "gdb_sys_time.h"
-#include <time.h>
+#include <chrono>
#include "gdb_usleep.h"
#include "interps.h"
@@ -98,7 +96,7 @@ static void set_width (void);
Modified in prompt_for_continue and defaulted_query.
Used in report_command_stats. */
-static struct timeval prompt_for_continue_wait_time;
+static std::chrono::steady_clock::duration prompt_for_continue_wait_time;
/* A flag indicating whether to timestamp debugging messages. */
@@ -1184,9 +1182,6 @@ defaulted_query (const char *ctlstr, const char defchar, va_list args)
int def_value;
char def_answer, not_def_answer;
char *y_string, *n_string, *question, *prompt;
- /* Used to add duration we waited for user to respond to
- prompt_for_continue_wait_time. */
- struct timeval prompt_started, prompt_ended, prompt_delta;
struct cleanup *old_chain;
/* Set up according to which answer is the default. */
@@ -1261,8 +1256,10 @@ defaulted_query (const char *ctlstr, const char defchar, va_list args)
annotation_level > 1 ? "\n\032\032query\n" : "");
make_cleanup (xfree, prompt);
- /* Used for calculating time spend waiting for user. */
- gettimeofday (&prompt_started, NULL);
+ /* Used to add duration we waited for user to respond to
+ prompt_for_continue_wait_time. */
+ using namespace std::chrono;
+ steady_clock::time_point prompt_started = steady_clock::now ();
prepare_to_handle_input ();
@@ -1307,10 +1304,7 @@ defaulted_query (const char *ctlstr, const char defchar, va_list args)
}
/* Add time spend in this routine to prompt_for_continue_wait_time. */
- gettimeofday (&prompt_ended, NULL);
- timeval_sub (&prompt_delta, &prompt_ended, &prompt_started);
- timeval_add (&prompt_for_continue_wait_time,
- &prompt_for_continue_wait_time, &prompt_delta);
+ prompt_for_continue_wait_time += steady_clock::now () - prompt_started;
if (annotation_level > 1)
printf_filtered (("\n\032\032post-query\n"));
@@ -1816,12 +1810,11 @@ prompt_for_continue (void)
{
char *ignore;
char cont_prompt[120];
+ struct cleanup *old_chain = make_cleanup (null_cleanup, NULL);
/* Used to add duration we waited for user to respond to
prompt_for_continue_wait_time. */
- struct timeval prompt_started, prompt_ended, prompt_delta;
- struct cleanup *old_chain = make_cleanup (null_cleanup, NULL);
-
- gettimeofday (&prompt_started, NULL);
+ using namespace std::chrono;
+ steady_clock::time_point prompt_started = steady_clock::now ();
if (annotation_level > 1)
printf_unfiltered (("\n\032\032pre-prompt-for-continue\n"));
@@ -1844,10 +1837,7 @@ prompt_for_continue (void)
make_cleanup (xfree, ignore);
/* Add time spend in this routine to prompt_for_continue_wait_time. */
- gettimeofday (&prompt_ended, NULL);
- timeval_sub (&prompt_delta, &prompt_ended, &prompt_started);
- timeval_add (&prompt_for_continue_wait_time,
- &prompt_for_continue_wait_time, &prompt_delta);
+ prompt_for_continue_wait_time += steady_clock::now () - prompt_started;
if (annotation_level > 1)
printf_unfiltered (("\n\032\032post-prompt-for-continue\n"));
@@ -1877,15 +1867,15 @@ prompt_for_continue (void)
void
reset_prompt_for_continue_wait_time (void)
{
- static const struct timeval zero_timeval = { 0 };
+ using namespace std::chrono;
- prompt_for_continue_wait_time = zero_timeval;
+ prompt_for_continue_wait_time = steady_clock::duration::zero ();
}
/* Fetch the cumulative time spent in prompt_for_continue. */
-struct timeval
-get_prompt_for_continue_wait_time (void)
+std::chrono::steady_clock::duration
+get_prompt_for_continue_wait_time ()
{
return prompt_for_continue_wait_time;
}
@@ -2308,21 +2298,20 @@ vfprintf_unfiltered (struct ui_file *stream, const char *format, va_list args)
old_cleanups = make_cleanup (xfree, linebuffer);
if (debug_timestamp && stream == gdb_stdlog)
{
- struct timeval tm;
- char *timestamp;
+ using namespace std::chrono;
int len, need_nl;
- gettimeofday (&tm, NULL);
+ steady_clock::time_point now = steady_clock::now ();
+ seconds s = duration_cast<seconds> (now.time_since_epoch ());
+ microseconds us = duration_cast<microseconds> (now.time_since_epoch () - s);
len = strlen (linebuffer);
need_nl = (len > 0 && linebuffer[len - 1] != '\n');
- timestamp = xstrprintf ("%ld:%ld %s%s",
- (long) tm.tv_sec, (long) tm.tv_usec,
- linebuffer,
- need_nl ? "\n": "");
- make_cleanup (xfree, timestamp);
- fputs_unfiltered (timestamp, stream);
+ std::string timestamp = string_printf ("%ld:%ld %s%s",
+ s.count (), us.count (),
+ linebuffer, need_nl ? "\n": "");
+ fputs_unfiltered (timestamp.c_str (), stream);
}
else
fputs_unfiltered (linebuffer, stream);
diff --git a/gdb/utils.h b/gdb/utils.h
index 36f5294..62091f0 100644
--- a/gdb/utils.h
+++ b/gdb/utils.h
@@ -23,6 +23,7 @@
#include "exceptions.h"
#include "common/scoped_restore.h"
+#include <chrono>
extern void initialize_utils (void);
@@ -51,7 +52,7 @@ extern const char *gdb_bfd_errmsg (bfd_error_type error_tag, char **matching);
/* Reset the prompt_for_continue clock. */
void reset_prompt_for_continue_wait_time (void);
/* Return the time spent in prompt_for_continue. */
-struct timeval get_prompt_for_continue_wait_time (void);
+std::chrono::steady_clock::duration get_prompt_for_continue_wait_time ();
\f
/* Parsing utilites. */
--
2.5.5
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] gdb: Use C++11 std::chrono
2016-11-17 17:15 [PATCH] gdb: Use C++11 std::chrono Pedro Alves
@ 2016-11-17 22:15 ` Simon Marchi
2016-11-23 0:59 ` [PATCH v2] " Pedro Alves
0 siblings, 1 reply; 11+ messages in thread
From: Simon Marchi @ 2016-11-17 22:15 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
On 2016-11-17 12:15, Pedro Alves wrote:
> This patch fixes a few problems with GDB's time handling.
> [...]
Indeed, it makes the code very nice and readable. My only opinion about
std::chrono so far was that it was so much more painful to do
std::this_thread::sleep_for(std::chrono::seconds(2)) than sleep(2). :)
> +/* Count the total amount of time spent executing in user mode. */
>
> +user_cpu_time_clock::time_point
> +user_cpu_time_clock::now () noexcept
> +{
> + return time_point (microseconds (get_run_time ()));
Looking at get_run_time() in libiberty, it seems like it returns user
time + system time. Doesn't it contradict the naming of this clock and
the comment of this method?
> - /* If gettimeofday doesn't exist, and as a portability solution
> it has
> - been replaced with, e.g., time, then it doesn't make sense to print
> - the microseconds field. Is there a way to check for that? */
> - fprintf (stderr, "%ld:%06ld ", (long) tm.tv_sec, (long)
> tm.tv_usec);
> + fprintf (stderr, "%ld:%06ld ", s.count (), us.count ());
Unrelated to your change, but I find it weird to format the time as
"seconds:useconds". I wouldn't object if you sneakily changed it to
"seconds.useconds". :)
> @@ -2390,8 +2383,8 @@ mi_load_progress (const char *section_name,
> unsigned long total_sent,
> unsigned long grand_total)
> {
> - struct timeval time_now, delta, update_threshold;
> - static struct timeval last_update;
> + using namespace std::chrono;
> + static steady_clock::time_point last_update;
> static char *previous_sect_name = NULL;
> int new_section;
> struct ui_out *saved_uiout;
> @@ -2416,18 +2409,9 @@ mi_load_progress (const char *section_name,
>
> uiout = current_uiout;
>
> - update_threshold.tv_sec = 0;
> - update_threshold.tv_usec = 500000;
> - gettimeofday (&time_now, NULL);
> -
> - delta.tv_usec = time_now.tv_usec - last_update.tv_usec;
> - delta.tv_sec = time_now.tv_sec - last_update.tv_sec;
> -
> - if (delta.tv_usec < 0)
> - {
> - delta.tv_sec -= 1;
> - delta.tv_usec += 1000000L;
> - }
> + microseconds update_threshold (500000);
> + steady_clock::time_point time_now = steady_clock::now ();
> + steady_clock::duration delta = time_now - last_update;
Can this be simplified to avoid having the delta variable? Since we can
easily compare two time_points, we can probably make it look like:
if (steady_clock::now () >= next_update)
{
next_update = steady_clock::now() + update_threshold;
...
}
or
if (steady_clock::now () >= (last_update + update_threshold))
{
last_update = steady_clock::now ();
...
}
Also, wouldn't "update_period" or "update_rate" be more precise than
"update_threshold"?
> static void
> timestamp (struct mi_timestamp *tv)
> {
> - gettimeofday (&tv->wallclock, NULL);
> -#ifdef HAVE_GETRUSAGE
> - getrusage (RUSAGE_SELF, &rusage);
> - tv->utime.tv_sec = rusage.ru_utime.tv_sec;
> - tv->utime.tv_usec = rusage.ru_utime.tv_usec;
> - tv->stime.tv_sec = rusage.ru_stime.tv_sec;
> - tv->stime.tv_usec = rusage.ru_stime.tv_usec;
> -#else
> - {
> - long usec = get_run_time ();
> -
> - tv->utime.tv_sec = usec/1000000L;
> - tv->utime.tv_usec = usec - 1000000L*tv->utime.tv_sec;
> - tv->stime.tv_sec = 0;
> - tv->stime.tv_usec = 0;
> - }
> -#endif
> + using namespace std::chrono;
It's not written in the coding style guide, but I think it would be nice
to have a newline between the "using" declaration and the following
lines, like we do with variable declarations.
> -/* Report how fast the transfer went. */
> +/* Report on STREAM the performance of a memory transfer operation,
> + such as 'load'. DATA_COUNT is the number of bytes transferred.
> + WRITE_COUNT is the number of separate write operations, or 0, if
> + that information is not available. TIME is the range of time in
> + which the operation lasted. */
>
> -void
> +static void
> print_transfer_performance (struct ui_file *stream,
> unsigned long data_count,
> unsigned long write_count,
> - const struct timeval *start_time,
> - const struct timeval *end_time)
> + const time_range &time)
Is there a reason you couldn't use a std::chrono::duration instead of a
time_range here? The function doesn't seem to care about the particular
time_points, only their difference.
> - timestamp = xstrprintf ("%ld:%ld %s%s",
> - (long) tm.tv_sec, (long) tm.tv_usec,
> - linebuffer,
> - need_nl ? "\n": "");
> - make_cleanup (xfree, timestamp);
> - fputs_unfiltered (timestamp, stream);
> + std::string timestamp = string_printf ("%ld:%ld %s%s",
Same comment about secs:usecs vs secs.usecs.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2] gdb: Use C++11 std::chrono
2016-11-17 22:15 ` Simon Marchi
@ 2016-11-23 0:59 ` Pedro Alves
2016-11-23 2:01 ` Simon Marchi
0 siblings, 1 reply; 11+ messages in thread
From: Pedro Alves @ 2016-11-23 0:59 UTC (permalink / raw)
To: Simon Marchi; +Cc: gdb-patches
On 11/17/2016 10:15 PM, Simon Marchi wrote:
> On 2016-11-17 12:15, Pedro Alves wrote:
>> This patch fixes a few problems with GDB's time handling.
>> [...]
>
> Indeed, it makes the code very nice and readable. My only opinion about
> std::chrono so far was that it was so much more painful to do
> std::this_thread::sleep_for(std::chrono::seconds(2)) than sleep(2). :)
:-)
>
>> +/* Count the total amount of time spent executing in user mode. */
>>
>> +user_cpu_time_clock::time_point
>> +user_cpu_time_clock::now () noexcept
>> +{
>> + return time_point (microseconds (get_run_time ()));
>
> Looking at get_run_time() in libiberty, it seems like it returns user
> time + system time. Doesn't it contradict the naming of this clock and
> the comment of this method?
Wow, talk about missing the obvious. I've fixed this now.
I added yet another clock (system_run_time, and renamed the files
accordingly) that has a "now ()" overload that returns both user and kernel
time, as separate time points, and deleted the "now()" method of the user
and kernel clocks, to force people to use the system_run_time
one (since the new clock can get the separate times with a single
system call). I kept the user and kernel clocks in order to keep
the distinct time point types.
>
>> - /* If gettimeofday doesn't exist, and as a portability solution
>> it has
>> - been replaced with, e.g., time, then it doesn't make sense to print
>> - the microseconds field. Is there a way to check for that? */
>> - fprintf (stderr, "%ld:%06ld ", (long) tm.tv_sec, (long)
>> tm.tv_usec);
>> + fprintf (stderr, "%ld:%06ld ", s.count (), us.count ());
>
> Unrelated to your change, but I find it weird to format the time as
> "seconds:useconds". I wouldn't object if you sneakily changed it to
> "seconds.useconds". :)
I don't mind either way. I changed it.
>
>> @@ -2390,8 +2383,8 @@ mi_load_progress (const char *section_name,
>> unsigned long total_sent,
>> unsigned long grand_total)
>> {
>> - struct timeval time_now, delta, update_threshold;
>> - static struct timeval last_update;
>> + using namespace std::chrono;
>> + static steady_clock::time_point last_update;
>> static char *previous_sect_name = NULL;
>> int new_section;
>> struct ui_out *saved_uiout;
>> @@ -2416,18 +2409,9 @@ mi_load_progress (const char *section_name,
>>
>> uiout = current_uiout;
>>
>> - update_threshold.tv_sec = 0;
>> - update_threshold.tv_usec = 500000;
>> - gettimeofday (&time_now, NULL);
>> -
>> - delta.tv_usec = time_now.tv_usec - last_update.tv_usec;
>> - delta.tv_sec = time_now.tv_sec - last_update.tv_sec;
>> -
>> - if (delta.tv_usec < 0)
>> - {
>> - delta.tv_sec -= 1;
>> - delta.tv_usec += 1000000L;
>> - }
>> + microseconds update_threshold (500000);
>> + steady_clock::time_point time_now = steady_clock::now ();
>> + steady_clock::duration delta = time_now - last_update;
>
> Can this be simplified to avoid having the delta variable? Since we can
> easily compare two time_points, we can probably make it look like:
>
> if (steady_clock::now () >= next_update)
> {
> next_update = steady_clock::now() + update_threshold;
Reading "now()" twice looks like an anti-pattern to me, since
that can return two different times.
> ...
> }
>
> or
>
> if (steady_clock::now () >= (last_update + update_threshold))
> {
> last_update = steady_clock::now ();
> ...
> }
>
> Also, wouldn't "update_period" or "update_rate" be more precise than
> "update_threshold"?
I was just using the old variable names. I didn't even really
try to understand the code. :-)
How about just getting rid of the variable altogether?
I did that in this version. I realized that I might as well write
it in terms of 'milliseconds (500)' instead of 'microseconds (500000)'
too.
>
>> static void
>> timestamp (struct mi_timestamp *tv)
>> {
>> - gettimeofday (&tv->wallclock, NULL);
>> -#ifdef HAVE_GETRUSAGE
>> - getrusage (RUSAGE_SELF, &rusage);
>> - tv->utime.tv_sec = rusage.ru_utime.tv_sec;
>> - tv->utime.tv_usec = rusage.ru_utime.tv_usec;
>> - tv->stime.tv_sec = rusage.ru_stime.tv_sec;
>> - tv->stime.tv_usec = rusage.ru_stime.tv_usec;
>> -#else
>> - {
>> - long usec = get_run_time ();
>> -
>> - tv->utime.tv_sec = usec/1000000L;
>> - tv->utime.tv_usec = usec - 1000000L*tv->utime.tv_sec;
>> - tv->stime.tv_sec = 0;
>> - tv->stime.tv_usec = 0;
>> - }
>> -#endif
>> + using namespace std::chrono;
>
> It's not written in the coding style guide, but I think it would be nice
> to have a newline between the "using" declaration and the following
> lines, like we do with variable declarations.
I don't mind. Done.
>
>> -/* Report how fast the transfer went. */
>> +/* Report on STREAM the performance of a memory transfer operation,
>> + such as 'load'. DATA_COUNT is the number of bytes transferred.
>> + WRITE_COUNT is the number of separate write operations, or 0, if
>> + that information is not available. TIME is the range of time in
>> + which the operation lasted. */
>>
>> -void
>> +static void
>> print_transfer_performance (struct ui_file *stream,
>> unsigned long data_count,
>> unsigned long write_count,
>> - const struct timeval *start_time,
>> - const struct timeval *end_time)
>> + const time_range &time)
>
> Is there a reason you couldn't use a std::chrono::duration instead of a
> time_range here? The function doesn't seem to care about the particular
> time_points, only their difference.
No real reason. I've changed it.
>
>> - timestamp = xstrprintf ("%ld:%ld %s%s",
>> - (long) tm.tv_sec, (long) tm.tv_usec,
>> - linebuffer,
>> - need_nl ? "\n": "");
>> - make_cleanup (xfree, timestamp);
>> - fputs_unfiltered (timestamp, stream);
>> + std::string timestamp = string_printf ("%ld:%ld %s%s",
>
> Same comment about secs:usecs vs secs.usecs.
Changed here too.
Here's the updated version. WDYT?
From f028dd3033f8174a851481e23d31e2b089cd1873 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Wed, 23 Nov 2016 00:47:40 +0000
Subject: [PATCH v2] gdb: Use C++11 std::chrono
This patch fixes a few problems with GDB's time handling.
#1 - It avoids problems with gnulib's C++ namespace support
On MinGW, the struct timeval that should be passed to gnulib's
gettimeofday replacement is incompatible with libiberty's
timeval_sub/timeval_add. That's because gnulib also replaces "struct
timeval" with its own definition, while libiberty expects the
system's.
E.g., in code like this:
gettimeofday (&prompt_ended, NULL);
timeval_sub (&prompt_delta, &prompt_ended, &prompt_started);
timeval_add (&prompt_for_continue_wait_time,
&prompt_for_continue_wait_time, &prompt_delta);
That's currently handled in gdb by not using gnulib's gettimeofday at
all (see common/gdb_sys_time.h), but that #undef hack won't work with
if/when we enable gnulib's C++ namespace support, because that mode
adds compile time warnings for uses of ::gettimeofday, which are hard
errors with -Werror.
#2 - But there's an elephant in the room: gettimeofday is not monotonic...
We're using it to:
a) check how long functions take, for performance analysis
b) compute when in the future to fire events in the event-loop
c) print debug timestamps
But that's exactly what gettimeofday is NOT meant for. Straight from
the man page:
~~~
The time returned by gettimeofday() is affected by
discontinuous jumps in the system time (e.g., if the system
administrator manually changes the system time). If you need a
monotonically increasing clock, see clock_gettime(2).
~~~
std::chrono (part of the C++11 standard library) has a monotonic clock
exactly for such purposes (std::chrono::steady_clock). This commit
switches to use that instead of gettimeofday, fixing all the issues
mentioned above.
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
* Makefile.in (SFILES): Add common/run-time-clock.c.
(HFILES_NO_SRCDIR): Add common/run-time-clock.h.
(COMMON_OBS): Add run-time-clock.o.
* common/run-time-clock.c, common/run-time-clock.h: New files.
* defs.h (struct timeval, print_transfer_performance): Delete
declarations.
* event-loop.c (struct gdb_timer) <when>: Now a
std::chrono::steady_clock::time_point.
(create_timer): use std::chrono::steady_clock instead of
gettimeofday. Use new instead of malloc.
(delete_timer): Use delete instead of xfree.
(duration_cast_timeval): New.
(update_wait_timeout): Use std::chrono::steady_clock instead of
gettimeofday.
gdb/gdbserver/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
* debug.c: Include <chrono> instead of "gdb_sys_time.h".
(debug_vprintf): Use std::chrono::steady_clock instead of
gettimeofday. Use '.' instead of ':'.
* tracepoint.c: Include <chrono> instead of "gdb_sys_time.h".
(get_timestamp): Use std::chrono::steady_clock instead of
gettimeofday.
* maint.c: Include <chrono> instead of "gdb_sys_time.h", <time.h>
and "timeval-utils.h".
(scoped_command_stats::~scoped_command_stats)
(scoped_command_stats::scoped_command_stats): Use
std::chrono::steady_clock instead of gettimeofday. Use
user_cpu_time_clock instead of get_run_time.
* maint.h: Include "run-time-clock.h" and <chrono>.
(scoped_command_stats): <m_start_cpu_time>: Now a
user_cpu_time_clock::time_point.
<m_start_wall_time>: Now a std::chrono::steady_clock::time_point.
* mi/mi-main.c: Include "run-time-clock.h" and <chrono> instead of
"gdb_sys_time.h" and <sys/resource.h>.
(rusage): Delete.
(mi_execute_command): Use new instead of XNEW.
(mi_load_progress): Use std::chrono::steady_clock instead of
gettimeofday.
(timestamp): Rewrite in terms of std::chrono::steady_clock,
user_cpu_time_clock and system_cpu_time_clock.
(timeval_diff): Delete.
(print_diff): Adjust to use std::chrono::steady_clock,
user_cpu_time_clock and system_cpu_time_clock.
* mi/mi-parse.h: Include "run-time-clock.h" and <chrono> instead
of "gdb_sys_time.h".
(struct mi_timestamp): Change fields types to
std::chrono::steady_clock::time_point, user_cpu_time_clock::time
and system_cpu_time_clock::time_point, instead of struct timeval.
* symfile.c: Include <chrono> instead of <time.h> and
"gdb_sys_time.h".
(struct time_range): New.
(generic_load): Use std::chrono::steady_clock instead of
gettimeofday.
(print_transfer_performance): Replace timeval parameters with a
std::chrono::steady_clock::duration parameter. Adjust.
* utils.c: Include <chrono> instead of "timeval-utils.h",
"gdb_sys_time.h", and <time.h>.
(prompt_for_continue_wait_time): Now a
std::chrono::steady_clock::duration.
(defaulted_query, prompt_for_continue): Use
std::chrono::steady_clock instead of
gettimeofday/timeval_sub/timeval_add.
(reset_prompt_for_continue_wait_time): Use
std::chrono::steady_clock::duration instead of struct timeval.
(get_prompt_for_continue_wait_time): Return a
std::chrono::steady_clock::duration instead of struct timeval.
(vfprintf_unfiltered): Use std::chrono::steady_clock instead of
gettimeofday. Use std::string. Use '.' instead of ':'.
* utils.h: Include <chrono>.
(get_prompt_for_continue_wait_time): Return a
std::chrono::steady_clock::duration instead of struct timeval.
---
gdb/Makefile.in | 5 ++-
gdb/common/run-time-clock.c | 58 ++++++++++++++++++++++++++++
gdb/common/run-time-clock.h | 75 ++++++++++++++++++++++++++++++++++++
gdb/defs.h | 14 -------
gdb/event-loop.c | 94 +++++++++++++++++++++------------------------
gdb/gdbserver/debug.c | 13 +++----
gdb/gdbserver/tracepoint.c | 10 ++---
gdb/maint.c | 31 +++++++--------
gdb/maint.h | 7 +++-
gdb/mi/mi-main.c | 73 +++++++++--------------------------
gdb/mi/mi-parse.h | 12 +++---
gdb/symfile.c | 39 +++++++++++--------
gdb/utils.c | 58 ++++++++++++----------------
gdb/utils.h | 3 +-
14 files changed, 282 insertions(+), 210 deletions(-)
create mode 100644 gdb/common/run-time-clock.c
create mode 100644 gdb/common/run-time-clock.h
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 3ea47a7..67db97d 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -895,6 +895,7 @@ SFILES = ada-exp.y ada-lang.c ada-typeprint.c ada-valprint.c ada-tasks.c \
common/errors.c common/common-debug.c common/common-exceptions.c \
common/btrace-common.c common/fileio.c common/common-regcache.c \
common/signals-state-save-restore.c common/new-op.c \
+ common/run-time-clock.c \
$(SUBDIR_GCC_COMPILE_SRCS)
LINTFILES = $(SFILES) $(YYFILES) $(CONFIG_SRCS) init.c
@@ -988,7 +989,7 @@ common/common-regcache.h fbsd-tdep.h nat/linux-personality.h \
common/fileio.h nat/x86-linux.h nat/x86-linux-dregs.h nat/amd64-linux-siginfo.h\
nat/linux-namespaces.h arch/arm.h common/gdb_sys_time.h arch/aarch64-insn.h \
tid-parse.h ser-event.h \
-common/signals-state-save-restore.h
+common/signals-state-save-restore.h common/run-time-clock.h
# Header files that already have srcdir in them, or which are in objdir.
@@ -1088,7 +1089,7 @@ COMMON_OBS = $(DEPFILES) $(CONFIG_OBS) $(YYOBJ) \
format.o registry.o btrace.o record-btrace.o waitstatus.o \
print-utils.o rsp-low.o errors.o common-debug.o debug.o \
common-exceptions.o btrace-common.o fileio.o \
- common-regcache.o new-op.o \
+ common-regcache.o new-op.o run-time-clock.o \
$(SUBDIR_GCC_COMPILE_OBS)
TSOBS = inflow.o
diff --git a/gdb/common/run-time-clock.c b/gdb/common/run-time-clock.c
new file mode 100644
index 0000000..f9dc7a0
--- /dev/null
+++ b/gdb/common/run-time-clock.c
@@ -0,0 +1,58 @@
+/* User/system CPU time clocks that follow the std::chrono interface.
+ Copyright (C) 2016 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#include "common-defs.h"
+#include "run-time-clock.h"
+#if defined HAVE_SYS_RESOURCE_H
+#include <sys/resource.h>
+#endif
+
+using namespace std::chrono;
+
+run_time_clock::time_point
+run_time_clock::now () noexcept
+{
+ return time_point (microseconds (get_run_time ()));
+}
+
+#ifdef HAVE_GETRUSAGE
+static std::chrono::microseconds
+timeval_to_microseconds (struct timeval *tv)
+{
+ return (seconds (tv->tv_sec) + microseconds (tv->tv_usec));
+}
+#endif
+
+void
+run_time_clock::now (user_cpu_time_clock::time_point &user,
+ system_cpu_time_clock::time_point &system) noexcept
+{
+#ifdef HAVE_GETRUSAGE
+ struct rusage rusage;
+
+ getrusage (RUSAGE_SELF, &rusage);
+
+ microseconds utime = timeval_to_microseconds (&rusage.ru_utime);
+ microseconds stime = timeval_to_microseconds (&rusage.ru_stime);
+ user = user_cpu_time_clock::time_point (utime);
+ system = system_cpu_time_clock::time_point (stime);
+#else
+ user = user_cpu_time_clock::time_point (microseconds (get_run_time ()));
+ system = system_cpu_time_clock::time_point (microseconds::zero ());
+#endif
+}
diff --git a/gdb/common/run-time-clock.h b/gdb/common/run-time-clock.h
new file mode 100644
index 0000000..ee531bb
--- /dev/null
+++ b/gdb/common/run-time-clock.h
@@ -0,0 +1,75 @@
+/* User/system CPU time clocks that follow the std::chrono interface.
+ Copyright (C) 2016 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#ifndef RUN_TIME_CLOCK_H
+#define RUN_TIME_CLOCK_H
+
+#include <chrono>
+
+/* Count the total amount of time spent executing in user mode. */
+
+struct user_cpu_time_clock
+{
+ using duration = std::chrono::microseconds;
+ using rep = duration::rep;
+ using period = duration::period;
+ using time_point = std::chrono::time_point<user_cpu_time_clock>;
+
+ static constexpr bool is_steady = true;
+
+ /* Use run_time_clock::now instead. */
+ static time_point now () noexcept = delete;
+};
+
+/* Count the total amount of time spent executing in kernel mode. */
+
+struct system_cpu_time_clock
+{
+ using duration = std::chrono::microseconds;
+ using rep = duration::rep;
+ using period = duration::period;
+ using time_point = std::chrono::time_point<system_cpu_time_clock>;
+
+ static constexpr bool is_steady = true;
+
+ /* Use run_time_clock::now instead. */
+ static time_point now () noexcept = delete;
+};
+
+/* Count the total amount of time spent executing in userspace+kernel
+ mode. */
+
+struct run_time_clock
+{
+ using duration = std::chrono::microseconds;
+ using rep = duration::rep;
+ using period = duration::period;
+ using time_point = std::chrono::time_point<run_time_clock>;
+
+ static constexpr bool is_steady = true;
+
+ static time_point now () noexcept;
+
+ /* Return the user/system time as separate time points, if
+ supported. If not supported, then the combined user+kernel time
+ is returned in USER and SYSTEM is set to zero. */
+ static void now (user_cpu_time_clock::time_point &user,
+ system_cpu_time_clock::time_point &system) noexcept;
+};
+
+#endif
diff --git a/gdb/defs.h b/gdb/defs.h
index 6d0d2dd..3d21f62 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -303,20 +303,6 @@ extern void symbol_file_command (char *, int);
/* * Remote targets may wish to use this as their load function. */
extern void generic_load (const char *name, int from_tty);
-/* * Report on STREAM the performance of memory transfer operation,
- such as 'load'.
- @param DATA_COUNT is the number of bytes transferred.
- @param WRITE_COUNT is the number of separate write operations, or 0,
- if that information is not available.
- @param START_TIME is the time at which an operation was started.
- @param END_TIME is the time at which an operation ended. */
-struct timeval;
-extern void print_transfer_performance (struct ui_file *stream,
- unsigned long data_count,
- unsigned long write_count,
- const struct timeval *start_time,
- const struct timeval *end_time);
-
/* From top.c */
typedef void initialize_file_ftype (void);
diff --git a/gdb/event-loop.c b/gdb/event-loop.c
index f94a6fa..9e8cf66 100644
--- a/gdb/event-loop.c
+++ b/gdb/event-loop.c
@@ -212,7 +212,7 @@ gdb_notifier;
first occasion after WHEN. */
struct gdb_timer
{
- struct timeval when;
+ std::chrono::steady_clock::time_point when;
int timer_id;
struct gdb_timer *next;
timer_handler_func *proc; /* Function to call to do the work. */
@@ -1097,33 +1097,22 @@ delete_async_event_handler (async_event_handler **async_handler_ptr)
*async_handler_ptr = NULL;
}
-/* Create a timer that will expire in MILLISECONDS from now. When the
- timer is ready, PROC will be executed. At creation, the timer is
- aded to the timers queue. This queue is kept sorted in order of
- increasing timers. Return a handle to the timer struct. */
+/* Create a timer that will expire in MS milliseconds from now. When
+ the timer is ready, PROC will be executed. At creation, the timer
+ is added to the timers queue. This queue is kept sorted in order
+ of increasing timers. Return a handle to the timer struct. */
+
int
-create_timer (int milliseconds, timer_handler_func * proc,
+create_timer (int ms, timer_handler_func *proc,
gdb_client_data client_data)
{
+ using namespace std::chrono;
struct gdb_timer *timer_ptr, *timer_index, *prev_timer;
- struct timeval time_now, delta;
-
- /* Compute seconds. */
- delta.tv_sec = milliseconds / 1000;
- /* Compute microseconds. */
- delta.tv_usec = (milliseconds % 1000) * 1000;
- gettimeofday (&time_now, NULL);
+ steady_clock::time_point time_now = steady_clock::now ();
- timer_ptr = XNEW (struct gdb_timer);
- timer_ptr->when.tv_sec = time_now.tv_sec + delta.tv_sec;
- timer_ptr->when.tv_usec = time_now.tv_usec + delta.tv_usec;
- /* Carry? */
- if (timer_ptr->when.tv_usec >= 1000000)
- {
- timer_ptr->when.tv_sec += 1;
- timer_ptr->when.tv_usec -= 1000000;
- }
+ timer_ptr = new gdb_timer ();
+ timer_ptr->when = time_now + milliseconds (ms);
timer_ptr->proc = proc;
timer_ptr->client_data = client_data;
timer_list.num_timers++;
@@ -1136,11 +1125,7 @@ create_timer (int milliseconds, timer_handler_func * proc,
timer_index != NULL;
timer_index = timer_index->next)
{
- /* If the seconds field is greater or if it is the same, but the
- microsecond field is greater. */
- if ((timer_index->when.tv_sec > timer_ptr->when.tv_sec)
- || ((timer_index->when.tv_sec == timer_ptr->when.tv_sec)
- && (timer_index->when.tv_usec > timer_ptr->when.tv_usec)))
+ if (timer_index->when > timer_ptr->when)
break;
}
@@ -1194,47 +1179,56 @@ delete_timer (int id)
;
prev_timer->next = timer_ptr->next;
}
- xfree (timer_ptr);
+ delete timer_ptr;
gdb_notifier.timeout_valid = 0;
}
+/* Convert a std::chrono duration to a struct timeval. */
+
+template<typename Duration>
+static struct timeval
+duration_cast_timeval (const Duration &d)
+{
+ using namespace std::chrono;
+ seconds sec = duration_cast<seconds> (d);
+ microseconds msec = duration_cast<microseconds> (d - sec);
+
+ struct timeval tv;
+ tv.tv_sec = sec.count ();
+ tv.tv_usec = msec.count ();
+ return tv;
+}
+
/* Update the timeout for the select() or poll(). Returns true if the
timer has already expired, false otherwise. */
static int
update_wait_timeout (void)
{
- struct timeval time_now, delta;
-
if (timer_list.first_timer != NULL)
{
- gettimeofday (&time_now, NULL);
- delta.tv_sec = timer_list.first_timer->when.tv_sec - time_now.tv_sec;
- delta.tv_usec = timer_list.first_timer->when.tv_usec - time_now.tv_usec;
- /* Borrow? */
- if (delta.tv_usec < 0)
- {
- delta.tv_sec -= 1;
- delta.tv_usec += 1000000;
- }
+ using namespace std::chrono;
+ steady_clock::time_point time_now = steady_clock::now ();
+ struct timeval timeout;
- /* Cannot simply test if delta.tv_sec is negative because time_t
- might be unsigned. */
- if (timer_list.first_timer->when.tv_sec < time_now.tv_sec
- || (timer_list.first_timer->when.tv_sec == time_now.tv_sec
- && timer_list.first_timer->when.tv_usec < time_now.tv_usec))
+ if (timer_list.first_timer->when < time_now)
{
/* It expired already. */
- delta.tv_sec = 0;
- delta.tv_usec = 0;
+ timeout.tv_sec = 0;
+ timeout.tv_usec = 0;
+ }
+ else
+ {
+ steady_clock::duration d = timer_list.first_timer->when - time_now;
+ timeout = duration_cast_timeval (d);
}
/* Update the timeout for select/ poll. */
if (use_poll)
{
#ifdef HAVE_POLL
- gdb_notifier.poll_timeout = delta.tv_sec * 1000;
+ gdb_notifier.poll_timeout = timeout.tv_sec * 1000;
#else
internal_error (__FILE__, __LINE__,
_("use_poll without HAVE_POLL"));
@@ -1242,12 +1236,12 @@ update_wait_timeout (void)
}
else
{
- gdb_notifier.select_timeout.tv_sec = delta.tv_sec;
- gdb_notifier.select_timeout.tv_usec = delta.tv_usec;
+ gdb_notifier.select_timeout.tv_sec = timeout.tv_sec;
+ gdb_notifier.select_timeout.tv_usec = timeout.tv_usec;
}
gdb_notifier.timeout_valid = 1;
- if (delta.tv_sec == 0 && delta.tv_usec == 0)
+ if (timer_list.first_timer->when < time_now)
return 1;
}
else
diff --git a/gdb/gdbserver/debug.c b/gdb/gdbserver/debug.c
index 54f2665..63fbe82 100644
--- a/gdb/gdbserver/debug.c
+++ b/gdb/gdbserver/debug.c
@@ -17,7 +17,7 @@
along with this program. If not, see <http://www.gnu.org/licenses/>. */
#include "server.h"
-#include "gdb_sys_time.h"
+#include <chrono>
/* Enable miscellaneous debugging output. The name is historical - it
was originally used to debug LinuxThreads support. */
@@ -41,14 +41,13 @@ debug_vprintf (const char *format, va_list ap)
if (debug_timestamp && new_line)
{
- struct timeval tm;
+ using namespace std::chrono;
- gettimeofday (&tm, NULL);
+ steady_clock::time_point now = steady_clock::now ();
+ seconds s = duration_cast<seconds> (now.time_since_epoch ());
+ microseconds us = duration_cast<microseconds> (now.time_since_epoch ()) - s;
- /* If gettimeofday doesn't exist, and as a portability solution it has
- been replaced with, e.g., time, then it doesn't make sense to print
- the microseconds field. Is there a way to check for that? */
- fprintf (stderr, "%ld:%06ld ", (long) tm.tv_sec, (long) tm.tv_usec);
+ fprintf (stderr, "%ld.%06ld ", (long) s.count (), (long) us.count ());
}
#endif
diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c
index 7700ad1..1444a0d 100644
--- a/gdb/gdbserver/tracepoint.c
+++ b/gdb/gdbserver/tracepoint.c
@@ -25,7 +25,7 @@
#include <ctype.h>
#include <fcntl.h>
#include <unistd.h>
-#include "gdb_sys_time.h"
+#include <chrono>
#include <inttypes.h>
#include "ax.h"
#include "tdesc.h"
@@ -7410,12 +7410,10 @@ getauxval (unsigned long type)
static LONGEST
get_timestamp (void)
{
- struct timeval tv;
+ using namespace std::chrono;
- if (gettimeofday (&tv, 0) != 0)
- return -1;
- else
- return (LONGEST) tv.tv_sec * 1000000 + tv.tv_usec;
+ steady_clock::time_point now = steady_clock::now ();
+ return duration_cast<microseconds> (now.time_since_epoch ()).count ();
}
void
diff --git a/gdb/maint.c b/gdb/maint.c
index 738571c..b43dbf9 100644
--- a/gdb/maint.c
+++ b/gdb/maint.c
@@ -24,8 +24,6 @@
#include "arch-utils.h"
#include <ctype.h>
#include <signal.h>
-#include "gdb_sys_time.h"
-#include <time.h>
#include "command.h"
#include "gdbcmd.h"
#include "symtab.h"
@@ -39,7 +37,6 @@
#include "objfiles.h"
#include "value.h"
#include "top.h"
-#include "timeval-utils.h"
#include "maint.h"
#include "selftest.h"
@@ -822,23 +819,21 @@ scoped_command_stats::~scoped_command_stats ()
if (m_time_enabled && per_command_time)
{
- long cmd_time = get_run_time () - m_start_cpu_time;
- struct timeval now_wall_time, delta_wall_time, wait_time;
+ using namespace std::chrono;
- gettimeofday (&now_wall_time, NULL);
- timeval_sub (&delta_wall_time,
- &now_wall_time, &m_start_wall_time);
+ run_time_clock::duration cmd_time
+ = run_time_clock::now () - m_start_cpu_time;
+ steady_clock::duration wall_time
+ = steady_clock::now () - 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);
+ wall_time -= get_prompt_for_continue_wait_time ();
printf_unfiltered (!m_msg_type
- ? _("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,
- (long) delta_wall_time.tv_sec,
- (long) delta_wall_time.tv_usec);
+ ? _("Startup time: %.6f (cpu), %.6f (wall)\n")
+ : _("Command execution time: %.6f (cpu), %.6f (wall)\n"),
+ duration<double> (cmd_time).count (),
+ duration<double> (wall_time).count ());
}
if (m_space_enabled && per_command_space)
@@ -892,8 +887,10 @@ scoped_command_stats::scoped_command_stats (bool msg_type)
if (msg_type == 0 || per_command_time)
{
- m_start_cpu_time = get_run_time ();
- gettimeofday (&m_start_wall_time, NULL);
+ using namespace std::chrono;
+
+ m_start_cpu_time = run_time_clock::now ();
+ m_start_wall_time = steady_clock::now ();
m_time_enabled = 1;
}
else
diff --git a/gdb/maint.h b/gdb/maint.h
index 8d03800..2af405d 100644
--- a/gdb/maint.h
+++ b/gdb/maint.h
@@ -19,6 +19,9 @@
#ifndef MAINT_H
#define MAINT_H
+#include "run-time-clock.h"
+#include <chrono>
+
extern void set_per_command_time (int);
extern void set_per_command_space (int);
@@ -48,8 +51,8 @@ class scoped_command_stats
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;
+ run_time_clock::time_point m_start_cpu_time;
+ std::chrono::steady_clock::time_point m_start_wall_time;
long m_start_space;
/* Total number of symtabs (over all objfiles). */
int m_start_nr_symtabs;
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 984a415..4d276c8 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -56,15 +56,8 @@
#include "observer.h"
#include <ctype.h>
-#include "gdb_sys_time.h"
-
-#if defined HAVE_SYS_RESOURCE_H
-#include <sys/resource.h>
-#endif
-
-#ifdef HAVE_GETRUSAGE
-struct rusage rusage;
-#endif
+#include "run-time-clock.h"
+#include <chrono>
enum
{
@@ -2169,7 +2162,7 @@ mi_execute_command (const char *cmd, int from_tty)
if (do_timings)
{
- command->cmd_start = XNEW (struct mi_timestamp);
+ command->cmd_start = new mi_timestamp ();
timestamp (command->cmd_start);
}
@@ -2390,8 +2383,8 @@ mi_load_progress (const char *section_name,
unsigned long total_sent,
unsigned long grand_total)
{
- struct timeval time_now, delta, update_threshold;
- static struct timeval last_update;
+ using namespace std::chrono;
+ static steady_clock::time_point last_update;
static char *previous_sect_name = NULL;
int new_section;
struct ui_out *saved_uiout;
@@ -2416,19 +2409,6 @@ mi_load_progress (const char *section_name,
uiout = current_uiout;
- update_threshold.tv_sec = 0;
- update_threshold.tv_usec = 500000;
- gettimeofday (&time_now, NULL);
-
- delta.tv_usec = time_now.tv_usec - last_update.tv_usec;
- delta.tv_sec = time_now.tv_sec - last_update.tv_sec;
-
- if (delta.tv_usec < 0)
- {
- delta.tv_sec -= 1;
- delta.tv_usec += 1000000L;
- }
-
new_section = (previous_sect_name ?
strcmp (previous_sect_name, section_name) : 1);
if (new_section)
@@ -2451,13 +2431,12 @@ mi_load_progress (const char *section_name,
gdb_flush (mi->raw_stdout);
}
- if (delta.tv_sec >= update_threshold.tv_sec &&
- delta.tv_usec >= update_threshold.tv_usec)
+ steady_clock::time_point time_now = steady_clock::now ();
+ if (time_now - last_update > milliseconds (500))
{
struct cleanup *cleanup_tuple;
- last_update.tv_sec = time_now.tv_sec;
- last_update.tv_usec = time_now.tv_usec;
+ last_update = time_now;
if (current_token)
fputs_unfiltered (current_token, mi->raw_stdout);
fputs_unfiltered ("+download", mi->raw_stdout);
@@ -2480,23 +2459,10 @@ mi_load_progress (const char *section_name,
static void
timestamp (struct mi_timestamp *tv)
{
- gettimeofday (&tv->wallclock, NULL);
-#ifdef HAVE_GETRUSAGE
- getrusage (RUSAGE_SELF, &rusage);
- tv->utime.tv_sec = rusage.ru_utime.tv_sec;
- tv->utime.tv_usec = rusage.ru_utime.tv_usec;
- tv->stime.tv_sec = rusage.ru_stime.tv_sec;
- tv->stime.tv_usec = rusage.ru_stime.tv_usec;
-#else
- {
- long usec = get_run_time ();
+ using namespace std::chrono;
- tv->utime.tv_sec = usec/1000000L;
- tv->utime.tv_usec = usec - 1000000L*tv->utime.tv_sec;
- tv->stime.tv_sec = 0;
- tv->stime.tv_usec = 0;
- }
-#endif
+ tv->wallclock = steady_clock::now ();
+ run_time_clock::now (tv->utime, tv->stime);
}
static void
@@ -2517,23 +2483,20 @@ mi_print_timing_maybe (struct ui_file *file)
print_diff_now (file, current_command_ts);
}
-static long
-timeval_diff (struct timeval start, struct timeval end)
-{
- return ((end.tv_sec - start.tv_sec) * 1000000L)
- + (end.tv_usec - start.tv_usec);
-}
-
static void
print_diff (struct ui_file *file, struct mi_timestamp *start,
struct mi_timestamp *end)
{
+ using namespace std::chrono;
+
+ duration<double> wallclock = end->wallclock - start->wallclock;
+ duration<double> utime = end->utime - start->utime;
+ duration<double> stime = end->stime - start->stime;
+
fprintf_unfiltered
(file,
",time={wallclock=\"%0.5f\",user=\"%0.5f\",system=\"%0.5f\"}",
- timeval_diff (start->wallclock, end->wallclock) / 1000000.0,
- timeval_diff (start->utime, end->utime) / 1000000.0,
- timeval_diff (start->stime, end->stime) / 1000000.0);
+ wallclock.count (), utime.count (), stime.count ());
}
void
diff --git a/gdb/mi/mi-parse.h b/gdb/mi/mi-parse.h
index d5595b3..dc8d21b 100644
--- a/gdb/mi/mi-parse.h
+++ b/gdb/mi/mi-parse.h
@@ -20,15 +20,17 @@
#ifndef MI_PARSE_H
#define MI_PARSE_H
-#include "gdb_sys_time.h"
+#include "run-time-clock.h"
+#include <chrono>
/* MI parser */
/* Timestamps for current command and last asynchronous command. */
-struct mi_timestamp {
- struct timeval wallclock;
- struct timeval utime;
- struct timeval stime;
+struct mi_timestamp
+{
+ std::chrono::steady_clock::time_point wallclock;
+ user_cpu_time_clock::time_point utime;
+ system_cpu_time_clock::time_point stime;
};
enum mi_command_type
diff --git a/gdb/symfile.c b/gdb/symfile.c
index f524f56..517c277 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -61,8 +61,7 @@
#include <fcntl.h>
#include <sys/stat.h>
#include <ctype.h>
-#include <time.h>
-#include "gdb_sys_time.h"
+#include <chrono>
#include "psymtab.h"
@@ -2066,11 +2065,15 @@ clear_memory_write_data (void *arg)
VEC_free (memory_write_request_s, vec);
}
+static void print_transfer_performance (struct ui_file *stream,
+ unsigned long data_count,
+ unsigned long write_count,
+ std::chrono::steady_clock::duration d);
+
void
generic_load (const char *args, int from_tty)
{
bfd *loadfile_bfd;
- struct timeval start_time, end_time;
char *filename;
struct cleanup *old_cleanups = make_cleanup (null_cleanup, 0);
struct load_section_data cbdata;
@@ -2131,13 +2134,15 @@ generic_load (const char *args, int from_tty)
bfd_map_over_sections (loadfile_bfd, load_section_callback, &cbdata);
- gettimeofday (&start_time, NULL);
+ using namespace std::chrono;
+
+ steady_clock::time_point start_time = steady_clock::now ();
if (target_write_memory_blocks (cbdata.requests, flash_discard,
load_progress) != 0)
error (_("Load failed"));
- gettimeofday (&end_time, NULL);
+ steady_clock::time_point end_time = steady_clock::now ();
entry = bfd_get_start_address (loadfile_bfd);
entry = gdbarch_addr_bits_remove (target_gdbarch (), entry);
@@ -2160,32 +2165,32 @@ generic_load (const char *args, int from_tty)
print_transfer_performance (gdb_stdout, total_progress.data_count,
total_progress.write_count,
- &start_time, &end_time);
+ end_time - start_time);
do_cleanups (old_cleanups);
}
-/* Report how fast the transfer went. */
+/* Report on STREAM the performance of a memory transfer operation,
+ such as 'load'. DATA_COUNT is the number of bytes transferred.
+ WRITE_COUNT is the number of separate write operations, or 0, if
+ that information is not available. TIME is how long the operation
+ lasted. */
-void
+static void
print_transfer_performance (struct ui_file *stream,
unsigned long data_count,
unsigned long write_count,
- const struct timeval *start_time,
- const struct timeval *end_time)
+ std::chrono::steady_clock::duration time)
{
- ULONGEST time_count;
+ using namespace std::chrono;
struct ui_out *uiout = current_uiout;
- /* Compute the elapsed time in milliseconds, as a tradeoff between
- accuracy and overflow. */
- time_count = (end_time->tv_sec - start_time->tv_sec) * 1000;
- time_count += (end_time->tv_usec - start_time->tv_usec) / 1000;
+ milliseconds ms = duration_cast<milliseconds> (time);
ui_out_text (uiout, "Transfer rate: ");
- if (time_count > 0)
+ if (ms.count () > 0)
{
- unsigned long rate = ((ULONGEST) data_count * 1000) / time_count;
+ unsigned long rate = ((ULONGEST) data_count * 1000) / ms.count ();
if (ui_out_is_mi_like_p (uiout))
{
diff --git a/gdb/utils.c b/gdb/utils.c
index 751f099..8ca0a2e 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -37,7 +37,6 @@
#endif
#include <signal.h>
-#include "timeval-utils.h"
#include "gdbcmd.h"
#include "serial.h"
#include "bfd.h"
@@ -61,8 +60,7 @@
#include "readline/readline.h"
-#include "gdb_sys_time.h"
-#include <time.h>
+#include <chrono>
#include "gdb_usleep.h"
#include "interps.h"
@@ -98,7 +96,7 @@ static void set_width (void);
Modified in prompt_for_continue and defaulted_query.
Used in report_command_stats. */
-static struct timeval prompt_for_continue_wait_time;
+static std::chrono::steady_clock::duration prompt_for_continue_wait_time;
/* A flag indicating whether to timestamp debugging messages. */
@@ -1184,9 +1182,6 @@ defaulted_query (const char *ctlstr, const char defchar, va_list args)
int def_value;
char def_answer, not_def_answer;
char *y_string, *n_string, *question, *prompt;
- /* Used to add duration we waited for user to respond to
- prompt_for_continue_wait_time. */
- struct timeval prompt_started, prompt_ended, prompt_delta;
struct cleanup *old_chain;
/* Set up according to which answer is the default. */
@@ -1261,8 +1256,10 @@ defaulted_query (const char *ctlstr, const char defchar, va_list args)
annotation_level > 1 ? "\n\032\032query\n" : "");
make_cleanup (xfree, prompt);
- /* Used for calculating time spend waiting for user. */
- gettimeofday (&prompt_started, NULL);
+ /* Used to add duration we waited for user to respond to
+ prompt_for_continue_wait_time. */
+ using namespace std::chrono;
+ steady_clock::time_point prompt_started = steady_clock::now ();
prepare_to_handle_input ();
@@ -1307,10 +1304,7 @@ defaulted_query (const char *ctlstr, const char defchar, va_list args)
}
/* Add time spend in this routine to prompt_for_continue_wait_time. */
- gettimeofday (&prompt_ended, NULL);
- timeval_sub (&prompt_delta, &prompt_ended, &prompt_started);
- timeval_add (&prompt_for_continue_wait_time,
- &prompt_for_continue_wait_time, &prompt_delta);
+ prompt_for_continue_wait_time += steady_clock::now () - prompt_started;
if (annotation_level > 1)
printf_filtered (("\n\032\032post-query\n"));
@@ -1816,12 +1810,11 @@ prompt_for_continue (void)
{
char *ignore;
char cont_prompt[120];
+ struct cleanup *old_chain = make_cleanup (null_cleanup, NULL);
/* Used to add duration we waited for user to respond to
prompt_for_continue_wait_time. */
- struct timeval prompt_started, prompt_ended, prompt_delta;
- struct cleanup *old_chain = make_cleanup (null_cleanup, NULL);
-
- gettimeofday (&prompt_started, NULL);
+ using namespace std::chrono;
+ steady_clock::time_point prompt_started = steady_clock::now ();
if (annotation_level > 1)
printf_unfiltered (("\n\032\032pre-prompt-for-continue\n"));
@@ -1844,10 +1837,7 @@ prompt_for_continue (void)
make_cleanup (xfree, ignore);
/* Add time spend in this routine to prompt_for_continue_wait_time. */
- gettimeofday (&prompt_ended, NULL);
- timeval_sub (&prompt_delta, &prompt_ended, &prompt_started);
- timeval_add (&prompt_for_continue_wait_time,
- &prompt_for_continue_wait_time, &prompt_delta);
+ prompt_for_continue_wait_time += steady_clock::now () - prompt_started;
if (annotation_level > 1)
printf_unfiltered (("\n\032\032post-prompt-for-continue\n"));
@@ -1877,15 +1867,15 @@ prompt_for_continue (void)
void
reset_prompt_for_continue_wait_time (void)
{
- static const struct timeval zero_timeval = { 0 };
+ using namespace std::chrono;
- prompt_for_continue_wait_time = zero_timeval;
+ prompt_for_continue_wait_time = steady_clock::duration::zero ();
}
/* Fetch the cumulative time spent in prompt_for_continue. */
-struct timeval
-get_prompt_for_continue_wait_time (void)
+std::chrono::steady_clock::duration
+get_prompt_for_continue_wait_time ()
{
return prompt_for_continue_wait_time;
}
@@ -2308,21 +2298,21 @@ vfprintf_unfiltered (struct ui_file *stream, const char *format, va_list args)
old_cleanups = make_cleanup (xfree, linebuffer);
if (debug_timestamp && stream == gdb_stdlog)
{
- struct timeval tm;
- char *timestamp;
+ using namespace std::chrono;
int len, need_nl;
- gettimeofday (&tm, NULL);
+ steady_clock::time_point now = steady_clock::now ();
+ seconds s = duration_cast<seconds> (now.time_since_epoch ());
+ microseconds us = duration_cast<microseconds> (now.time_since_epoch () - s);
len = strlen (linebuffer);
need_nl = (len > 0 && linebuffer[len - 1] != '\n');
- timestamp = xstrprintf ("%ld:%ld %s%s",
- (long) tm.tv_sec, (long) tm.tv_usec,
- linebuffer,
- need_nl ? "\n": "");
- make_cleanup (xfree, timestamp);
- fputs_unfiltered (timestamp, stream);
+ std::string timestamp = string_printf ("%ld.%06ld %s%s",
+ (long) s.count (),
+ (long) us.count (),
+ linebuffer, need_nl ? "\n": "");
+ fputs_unfiltered (timestamp.c_str (), stream);
}
else
fputs_unfiltered (linebuffer, stream);
diff --git a/gdb/utils.h b/gdb/utils.h
index 36f5294..62091f0 100644
--- a/gdb/utils.h
+++ b/gdb/utils.h
@@ -23,6 +23,7 @@
#include "exceptions.h"
#include "common/scoped_restore.h"
+#include <chrono>
extern void initialize_utils (void);
@@ -51,7 +52,7 @@ extern const char *gdb_bfd_errmsg (bfd_error_type error_tag, char **matching);
/* Reset the prompt_for_continue clock. */
void reset_prompt_for_continue_wait_time (void);
/* Return the time spent in prompt_for_continue. */
-struct timeval get_prompt_for_continue_wait_time (void);
+std::chrono::steady_clock::duration get_prompt_for_continue_wait_time ();
\f
/* Parsing utilites. */
--
2.5.5
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] gdb: Use C++11 std::chrono
2016-11-23 0:59 ` [PATCH v2] " Pedro Alves
@ 2016-11-23 2:01 ` Simon Marchi
2016-11-23 2:17 ` Pedro Alves
0 siblings, 1 reply; 11+ messages in thread
From: Simon Marchi @ 2016-11-23 2:01 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
> +#ifndef RUN_TIME_CLOCK_H
> +#define RUN_TIME_CLOCK_H
> +
> +#include <chrono>
> +
> +/* Count the total amount of time spent executing in user mode. */
> +
> +struct user_cpu_time_clock
> +{
> + using duration = std::chrono::microseconds;
> + using rep = duration::rep;
> + using period = duration::period;
> + using time_point = std::chrono::time_point<user_cpu_time_clock>;
> +
> + static constexpr bool is_steady = true;
> +
> + /* Use run_time_clock::now instead. */
> + static time_point now () noexcept = delete;
> +};
> +
> +/* Count the total amount of time spent executing in kernel mode. */
> +
> +struct system_cpu_time_clock
> +{
> + using duration = std::chrono::microseconds;
> + using rep = duration::rep;
> + using period = duration::period;
> + using time_point = std::chrono::time_point<system_cpu_time_clock>;
> +
> + static constexpr bool is_steady = true;
> +
> + /* Use run_time_clock::now instead. */
> + static time_point now () noexcept = delete;
> +};
> +
> +/* Count the total amount of time spent executing in userspace+kernel
> + mode. */
> +
> +struct run_time_clock
> +{
> + using duration = std::chrono::microseconds;
> + using rep = duration::rep;
> + using period = duration::period;
> + using time_point = std::chrono::time_point<run_time_clock>;
> +
> + static constexpr bool is_steady = true;
> +
> + static time_point now () noexcept;
> +
> + /* Return the user/system time as separate time points, if
> + supported. If not supported, then the combined user+kernel time
> + is returned in USER and SYSTEM is set to zero. */
> + static void now (user_cpu_time_clock::time_point &user,
> + system_cpu_time_clock::time_point &system) noexcept;
> +};
From what I understand, {user,system}_cpu_time_clock are only defined in
order to be able to use their time_point type? It feels a bit
overengineered, unless you expect those types to differ at some point.
Is there an advantage of having different types over having a single
clock type and this
run_time_clock::now (run_time_clock::time_point &user,
run_time_clock::time_point &system)
?
> @@ -2390,8 +2383,8 @@ mi_load_progress (const char *section_name,
> unsigned long total_sent,
> unsigned long grand_total)
> {
> - struct timeval time_now, delta, update_threshold;
> - static struct timeval last_update;
> + using namespace std::chrono;
> + static steady_clock::time_point last_update;
> static char *previous_sect_name = NULL;
> int new_section;
> struct ui_out *saved_uiout;
> @@ -2416,19 +2409,6 @@ mi_load_progress (const char *section_name,
>
> uiout = current_uiout;
>
> - update_threshold.tv_sec = 0;
> - update_threshold.tv_usec = 500000;
> - gettimeofday (&time_now, NULL);
> -
> - delta.tv_usec = time_now.tv_usec - last_update.tv_usec;
> - delta.tv_sec = time_now.tv_sec - last_update.tv_sec;
> -
> - if (delta.tv_usec < 0)
> - {
> - delta.tv_sec -= 1;
> - delta.tv_usec += 1000000L;
> - }
> -
> new_section = (previous_sect_name ?
> strcmp (previous_sect_name, section_name) : 1);
> if (new_section)
> @@ -2451,13 +2431,12 @@ mi_load_progress (const char *section_name,
> gdb_flush (mi->raw_stdout);
> }
>
> - if (delta.tv_sec >= update_threshold.tv_sec &&
> - delta.tv_usec >= update_threshold.tv_usec)
> + steady_clock::time_point time_now = steady_clock::now ();
> + if (time_now - last_update > milliseconds (500))
> {
> struct cleanup *cleanup_tuple;
>
> - last_update.tv_sec = time_now.tv_sec;
> - last_update.tv_usec = time_now.tv_usec;
> + last_update = time_now;
> if (current_token)
> fputs_unfiltered (current_token, mi->raw_stdout);
> fputs_unfiltered ("+download", mi->raw_stdout);
It looks nice like this.
Unrelated: for future work, it looks like an std::priority_queue would
be a nice match for timer_list.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] gdb: Use C++11 std::chrono
2016-11-23 2:01 ` Simon Marchi
@ 2016-11-23 2:17 ` Pedro Alves
2016-11-23 2:27 ` Simon Marchi
0 siblings, 1 reply; 11+ messages in thread
From: Pedro Alves @ 2016-11-23 2:17 UTC (permalink / raw)
To: Simon Marchi; +Cc: gdb-patches
On 11/23/2016 02:01 AM, Simon Marchi wrote:
> From what I understand, {user,system}_cpu_time_clock are only defined in
> order to be able to use their time_point type? It feels a bit
> overengineered, unless you expect those types to differ at some point.
> Is there an advantage of having different types over having a single
> clock type and this
>
> run_time_clock::now (run_time_clock::time_point &user,
> run_time_clock::time_point &system)
>
> ?
I kept them (from v1) for type-safety: it makes it impossible to swap
the arguments to this new now() method by mistake, or compare old
system time with new user time by mistake. E.g.:
/usr/include/c++/5.3.1/chrono:650:7: note: candidate: template<class _Clock, class _Dur1, class _Dur2> constexpr typename std::common_type<_Duration1, _Duration2>::type std::chrono::operator-(const std::chrono::time_point<_Clock, _Duration1>&, const std::chrono::time_point<_Clock, _Duration2>&)
operator-(const time_point<_Clock, _Dur1>& __lhs,
^
/usr/include/c++/5.3.1/chrono:650:7: note: template argument deduction/substitution failed:
src/gdb/mi/mi-main.c:2493:48: note: deduced conflicting types for parameter ‘_Clock’ (‘system_cpu_time_clock’ and ‘user_cpu_time_clock’)
duration<double> utime = end->stime - start->utime;
^
Would you still prefer I remove those?
> Unrelated: for future work, it looks like an std::priority_queue would
> be a nice match for timer_list.
Maybe, but we'd need to inherit from it in order to be able to
delete timers that are not at the top of the heap. A set or
multiset would be other options.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] gdb: Use C++11 std::chrono
2016-11-23 2:17 ` Pedro Alves
@ 2016-11-23 2:27 ` Simon Marchi
2016-11-23 2:36 ` Pedro Alves
0 siblings, 1 reply; 11+ messages in thread
From: Simon Marchi @ 2016-11-23 2:27 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
On 2016-11-22 21:17, Pedro Alves wrote:
> I kept them (from v1) for type-safety: it makes it impossible to swap
> the arguments to this new now() method by mistake, or compare old
> system time with new user time by mistake. E.g.:
>
> /usr/include/c++/5.3.1/chrono:650:7: note: candidate: template<class
> _Clock, class _Dur1, class _Dur2> constexpr typename
> std::common_type<_Duration1, _Duration2>::type
> std::chrono::operator-(const std::chrono::time_point<_Clock,
> _Duration1>&, const std::chrono::time_point<_Clock, _Duration2>&)
> operator-(const time_point<_Clock, _Dur1>& __lhs,
> ^
> /usr/include/c++/5.3.1/chrono:650:7: note: template argument
> deduction/substitution failed:
> src/gdb/mi/mi-main.c:2493:48: note: deduced conflicting types for
> parameter ‘_Clock’ (‘system_cpu_time_clock’ and ‘user_cpu_time_clock’)
> duration<double> utime = end->stime - start->utime;
> ^
>
> Would you still prefer I remove those?
That sounds useful, I'm convinced.
>> Unrelated: for future work, it looks like an std::priority_queue would
>> be a nice match for timer_list.
>
> Maybe, but we'd need to inherit from it in order to be able to
> delete timers that are not at the top of the heap. A set or
> multiset would be other options.
You're right:
https://stackoverflow.com/questions/19467485/c-priority-queue-removing-element-not-at-top
Perhaps that's a bit too much for the amount of timers we usually have
though...
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] gdb: Use C++11 std::chrono
2016-11-23 2:27 ` Simon Marchi
@ 2016-11-23 2:36 ` Pedro Alves
2016-11-23 2:57 ` Simon Marchi
0 siblings, 1 reply; 11+ messages in thread
From: Pedro Alves @ 2016-11-23 2:36 UTC (permalink / raw)
To: Simon Marchi; +Cc: gdb-patches
On 11/23/2016 02:27 AM, Simon Marchi wrote:
> That sounds useful, I'm convinced.
Alright. Thanks for the review!
Since this adds new files to the Makefile.in lists, I'll wait
for you to push your series in first.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] gdb: Use C++11 std::chrono
2016-11-23 2:36 ` Pedro Alves
@ 2016-11-23 2:57 ` Simon Marchi
2016-11-23 14:48 ` Simon Marchi
0 siblings, 1 reply; 11+ messages in thread
From: Simon Marchi @ 2016-11-23 2:57 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
On 2016-11-22 21:36, Pedro Alves wrote:
> Alright. Thanks for the review!
>
> Since this adds new files to the Makefile.in lists, I'll wait
> for you to push your series in first.
Hmm weird, I thought it was pushed (and said so on the list), but I see
that it's not. Perhaps somebody managed to push a commit just before me
and I didn't notice that the push fail... I'll verify tomorrow morning
(it's on my work computer).
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] gdb: Use C++11 std::chrono
2016-11-23 2:57 ` Simon Marchi
@ 2016-11-23 14:48 ` Simon Marchi
2016-11-23 16:02 ` Pedro Alves
0 siblings, 1 reply; 11+ messages in thread
From: Simon Marchi @ 2016-11-23 14:48 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
On 2016-11-22 21:57, Simon Marchi wrote:
> On 2016-11-22 21:36, Pedro Alves wrote:
>> Alright. Thanks for the review!
>>
>> Since this adds new files to the Makefile.in lists, I'll wait
>> for you to push your series in first.
>
> Hmm weird, I thought it was pushed (and said so on the list), but I
> see that it's not. Perhaps somebody managed to push a commit just
> before me and I didn't notice that the push fail... I'll verify
> tomorrow morning (it's on my work computer).
Ok, it's now pushed for real!
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] gdb: Use C++11 std::chrono
2016-11-23 14:48 ` Simon Marchi
@ 2016-11-23 16:02 ` Pedro Alves
2016-11-23 16:11 ` Simon Marchi
0 siblings, 1 reply; 11+ messages in thread
From: Pedro Alves @ 2016-11-23 16:02 UTC (permalink / raw)
To: Simon Marchi; +Cc: gdb-patches
On 11/23/2016 02:47 PM, Simon Marchi wrote:
> On 2016-11-22 21:57, Simon Marchi wrote:
>> On 2016-11-22 21:36, Pedro Alves wrote:
>>> Alright. Thanks for the review!
>>>
>>> Since this adds new files to the Makefile.in lists, I'll wait
>>> for you to push your series in first.
>>
>> Hmm weird, I thought it was pushed (and said so on the list), but I
>> see that it's not. Perhaps somebody managed to push a commit just
>> before me and I didn't notice that the push fail... I'll verify
>> tomorrow morning (it's on my work computer).
>
> Ok, it's now pushed for real!
I've pushed mine in too, then, with the new files listed in
their respective places:
diff --git c/gdb/Makefile.in w/gdb/Makefile.in
index e3e3ce4..44a743e 100644
--- c/gdb/Makefile.in
+++ w/gdb/Makefile.in
@@ -1201,6 +1201,7 @@ SFILES = \
common/print-utils.c \
common/ptid.c \
common/rsp-low.c \
+ common/run-time-clock.c \
common/signals.c \
common/signals-state-save-restore.c \
common/vec.c \
@@ -1485,6 +1486,7 @@ HFILES_NO_SRCDIR = \
common/ptid.h \
common/queue.h \
common/rsp-low.h \
+ common/run-time-clock.h \
common/signals-state-save-restore.h \
common/symbol.h \
common/vec.h \
@@ -1739,6 +1741,7 @@ COMMON_OBS = $(DEPFILES) $(CONFIG_OBS) $(YYOBJ) \
registry.o \
reverse.o \
rsp-low.o \
+ run-time-clock.o \
rust-lang.o \
selftest.o \
sentinel-frame.o \
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] gdb: Use C++11 std::chrono
2016-11-23 16:02 ` Pedro Alves
@ 2016-11-23 16:11 ` Simon Marchi
0 siblings, 0 replies; 11+ messages in thread
From: Simon Marchi @ 2016-11-23 16:11 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
On 2016-11-23 11:02, Pedro Alves wrote:
> I've pushed mine in too, then, with the new files listed in
> their respective places:
>
> diff --git c/gdb/Makefile.in w/gdb/Makefile.in
> index e3e3ce4..44a743e 100644
> --- c/gdb/Makefile.in
> +++ w/gdb/Makefile.in
> @@ -1201,6 +1201,7 @@ SFILES = \
> common/print-utils.c \
> common/ptid.c \
> common/rsp-low.c \
> + common/run-time-clock.c \
> common/signals.c \
> common/signals-state-save-restore.c \
> common/vec.c \
> @@ -1485,6 +1486,7 @@ HFILES_NO_SRCDIR = \
> common/ptid.h \
> common/queue.h \
> common/rsp-low.h \
> + common/run-time-clock.h \
> common/signals-state-save-restore.h \
> common/symbol.h \
> common/vec.h \
> @@ -1739,6 +1741,7 @@ COMMON_OBS = $(DEPFILES) $(CONFIG_OBS) $(YYOBJ) \
> registry.o \
> reverse.o \
> rsp-low.o \
> + run-time-clock.o \
> rust-lang.o \
> selftest.o \
> sentinel-frame.o \
Splendid!
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2016-11-23 16:11 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-17 17:15 [PATCH] gdb: Use C++11 std::chrono Pedro Alves
2016-11-17 22:15 ` Simon Marchi
2016-11-23 0:59 ` [PATCH v2] " Pedro Alves
2016-11-23 2:01 ` Simon Marchi
2016-11-23 2:17 ` Pedro Alves
2016-11-23 2:27 ` Simon Marchi
2016-11-23 2:36 ` Pedro Alves
2016-11-23 2:57 ` Simon Marchi
2016-11-23 14:48 ` Simon Marchi
2016-11-23 16:02 ` Pedro Alves
2016-11-23 16:11 ` Simon Marchi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).