Re: [boost] [system] Boost.Timer replacement

Subject: Re: [boost] [system] Boost.Timer replacement
From: Beman Dawes (bdawes_at_[hidden])
Date: 2011-09-20 09:28:21

On Mon, Sep 19, 2011 at 4:39 PM, Stewart, Robert <Robert.Stewart_at_[hidden]>wrote:

> Beman Dawes wrote:
> >
> > http://beman.github.com/timer/ documents a useful replacement
> > for Boost.Timer. The full library has been in the sandbox for
> > years, but current development is on GitHub.
>
> You use "_t" for a typedef and a struct.  Odd.
>

Historical artifact. typedef changed to nanosecond_type.

struct changed to cpu_times. See later response to naming questions for
rationale.

> Why doesn't times() return a times_t?  I realize times_t comprises three
> 64b values, but do you suppose that function will be called in high
> performance contexts in which returning 192 bits would be problematic?
>

Interesting question. I don't see a performance concern, and the interface
would be cleaner if the result is just returned.

The issue is actually moot for times() because times() has been
eliminated.It makes much more sense in a C++11 world to implement in terms
of std <chrono> as implemented by Boost.Chrono, and that has been done and
is working well.

>
> I may be unobservant, but this is the first time I've noticed a function
> taking *and* returning a system::error_code &.  Is that beneficial?  (If
> that's an idiom you'd encourage, it should be documented in Boost.System.)

No, that's a holdover from years ago when I was experimenting with different
ways to use system::error_code.

 Changing the one-argument times() to return a times_t would mean this one,
> too, should return a times_t, but in the case of error, would it be a copy
> of an uninitialized instance?  Maybe the times_t & argument isn't so bad. :)
>

There is also the C convention of returning times of -1 on an error. See
destructor discussion below.

>
> timer::stop() is inconsistent.  If times() and timer::elapsed() take a
> times_t & argument, then so should timer::stop().
>

I'll make it consistent, probably by returning the result rather than taking
an argument.

>
> s/stopped/is_stopped/
>

Changed.

>
> I dislike the idea of relying on a destructor to report timing information
> as you've done in run_timer.  The no-throw demands of a destructor make I/O
> questionable since there is no means to report failure.  I realize that
> there is a report() member function that can be called separately, but using
> the destructor just seems too odd.
>

It comes down to ease of use. In practice, the idiom "just works".

>
> "run_timer" doesn't indicating the principle feature: reporting.  You might
> call it "reporting_timer" or "reportable_timer"?
>
> "report" does not imply a call to stop().
>

I've also been concerned about names. Some background:

* The resolution of timer/run_timer was distressingly low on
Windows/Linux/Mac platforms. As low as 1/10 of a second, and never much
better.

* Wall clock-time (AKA real-time) could be improved to sub-microsecond
resolution by using platform specific high resolution APIs, as Boost.Chrono
was already doing.

* Tests showed that use of high resolution timers for wall clock time was a
big step forward, but the cost of always getting user and system time, even
if not used, was quite noticeable, particularly with GCC.

The response to that was to provided two timers, one just reporting wall
clock time, and the other reporting wall clock, user, and system time.

What should these be named? I settled on:

class high_resolution_timer;          // wall-clock time
class auto_high_resolution_timer; // wall-clock time, with reporting
class cpu_timer;                             // wall-clock, user, and system
time
class auto_cpu_timer;                    // wall-clock, user, and system
time, with reporting

Applying your suggestion yields reporting_high_resolution_timer and
reporting_cpu_timer.

reporting_high_resolution_timer is kinda long, so we might use hi_res_timer
and reporting_hi_res_timer.

I'm OK with any of those, but have a mild preference for auto_ rather than
reporting_. I waver back and forth on high_resolution vs hi_res.

>
> Because of all of those strikes against run_timer, I'd prefer something
> like this:
>
> class timer
> {
> public:
>   // as before
>
>   void
>   report(int _places = 2); // writes to std::cout
>
>   void
>   report(int _places, std::ostream & _stream);
>
>   void
>   report(std::string const & _format, int _places = 2);
>
>   void
>   report(std::string const & _format, int _places,
>      std::ostream & _stream);
> };
>
> Those member functions would capture a snapshot of the elapsed time and
> report it.  (That means the I/O would affect further timing values; an
> accumulated elapsed time and a current start time could be used to ignore
> the time required for I/O.)  A call to stop() followed by a call to report()
> would be needed to report the elapsed time.
>
> It still might be useful to provide an RAII class to ensure a timer stops
> at the end of a scope:
>
> timer t;
> {
>   stopper _(t);
>   // do stuff
> }
>

Interesting, but mixing timing and reporting in the same class doesn't seem
desirable to me.

My most frequent use of timers has been for reporting, and I really want to
keep that usage simple.

The difference between:

int main()
{
  timer t;
  {
    stopper _(t);
    // do stuff
   }
}

and:

int main()
{
  auto_timer t;
  // do stuff
}

is really major and would make the usage harder to teach and less likely to
be used, IMO.

Thanks for these comments! Very helpful.

--Beman