Re: [PATCH v2] gdb: Use C++11 std::chrono

[Simon Marchi <simon dot marchi at polymtl dot ca>]

> +#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.