On Tue, Sep 13, 2011 at 12:24 PM, Beman Dawes <bdawes@acm.org> wrote:

Greg wrote:

...

The second question is, does this allow using a format string consisting of a single letter from the set ("wustp") followed by a NULL as a format string (e.g. "w\0")?  It seems like it does, but again I haven't yet tested the code.  I'm not sure that it's a problem, but it just stood out since it doesn't seem to fit the intent.

Works as expected. So does "%w\0", which is what I suspect you meant to write:-)

Oh right, nevermind... it will just write 'w' to the stream if the format string is "w\0". It wasn't a typo, I'm just a little slow :P -G

Le 13/09/11 19:55, John Maddock a écrit :

...

"The best is the enemy of the good."

Voltaire's famous bon mot applies to a replacement for Boost.Timer. I've had a "good", although not "best", replacement ready since circa 2006. It has always been set aside waiting for something better. The data-time, std::chrono, boost::chrono, boost::stopwatch, and probably others, were all candidates.

I'm tired of waiting.

Me too!!

...

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.

See https://github.com/Beman/timer

I'd like to move the header into boost/system/pending or boost/system/detail, and the rest into the appropriate libs/system subdirectories. The library will be proposed for a full review, or a mini-review if enough other Boost libraries start using it. If Endian gets accepted, for example, it certainly needs timing benchmarks.

My only comment is that it would be a shame to have duplicate functionality in the System and Chrono libraries - especially for such a seemingly simple utility.

Only today I through together:

template <class Clock> struct stopwatch { typedef typename Clock::duration duration; stopwatch() { m_start = Clock::now(); } duration elapsed() { return Clock::now() - m_start; } void reset() { m_start = Clock::now(); }

private: typename Clock::time_point m_start; };

Is there any reason something like this couldn't be added to Chrono right away (I'm not so bothered by the reporting ability)?

I could add a basic stopwatch quite soon, as there is no reporting. Best, Vicente

On Mon, Sep 19, 2011 at 4:39 PM, Stewart, Robert <Robert.Stewart@sig.com>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

On Tue, Sep 20, 2011 at 10:36 AM, Stewart, Robert <Robert.Stewart@sig.com>wrote:

<snip>

Many examples and discussion </snip>

The destructor-triggered reporting can still be provided via run_timer (with whatever name), but provide the basic means to produce output, including the format string rendering, in timer or a derivate.

I've started a branch "format-report-in-timers" to try out the approach you are advocating. I'll work on it a bit more this afternoon, but it will be sometime tomorrow before it's ready for experimentation. Thanks, --Beman

On Tue, Sep 20, 2011 at 10:56 AM, Vicente Botet <vicente.botet@wanadoo.fr> wrote:

Stewart, Robert wrote:

...

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.

Yes, reporting on the destructor would imply to catch any exception.

I have used the word stopwatch instead. From wikipedia: "A timer can be used to control the sequence of an event or process. Whereas a stopwatch counts upwards from zero for measuring elapsed time, a timer counts down from a specified time interval, like an hourglass."

I've decided that the best place for the functionality is the current timer library, so will stick with the "timer" terminology. Also, real stopwatches (I own one!) often have "lap" functionality, too, and that isn't supplied by these classes. But "stopwatch" might be a great name for Boost.Chrono use.

...

"run_timer" doesn't indicating the principle feature: reporting.  You might call it "reporting_timer" or "reportable_timer"?

I have used stopwatch_reporter.

I ended up folding that functionality into the base class.

...

"report" does not imply a call to stop().

Agreed. But leaving the timer running while doing I/O seems likely to produce misleading results. I've added a note to the docs.

...

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); };

I guess that we need reporters that are independent of the timer/stopwatch.

I've added independent format() functions to deal with that need.

...

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 }

Yes, this will be safer. I had some scoped runner, stopper, suspender, resumer that could be used for these purpose

stopwatch<> sw(dont_start); {   scoped_runner<> _(sw); //starts on construction + stop on destruction   // do stuff  {     scoped_suspender<> _(sw); // suspend on construction + resume on destruction     // do some stuff we don't want to measure, as e.g. i/o.  } }

We could provide some specific stopwatch formatters that could make easier the reporting as for example

cout << times_stopwatch_fmt("xxxx real=%1% user=%2% system=%3") % sw;

The single line run_timer from Beman is yet quite attractive and the library could warn to the user of its limitations. I have tried to make a kind of 'generic' stopwatch reporter but at the end it relies always on a Formatter class which of course can have a valid default.

Yes. Boost.Chrono is the right place for stopwatches, reporters, and formatters with a wide selection of features. I want to keep timer use simple. --Beman