Subject: Re: [boost] [Review] Formal Review of Proposed Boost.Chrono Library Starts TOMORROW
From: John Bytheway (jbytheway+boost_at_[hidden])
Date: 2010-11-10 14:40:20
On 10/11/10 00:04, vicente.botet wrote:
> Thanks John for your contribution. ----- Original Message ----- From:
> "John Bytheway" <jbytheway+boost_at_[hidden]> To:
>> On 05/11/10 17:25, Anthony Williams wrote:
>>> Here are some questions you might want to answer in your review:
>>> - What is your evaluation of the design?
>> It looks mostly well thought out and sensible, and I guess there's
>> not much to say about the standardized portion, but I have a few
>> There is various compile-time arithmetic happening implicitly in
>> this library (e.g. in the many common_type return values). Are
>> integer overflows in this arithmetic detected and reported? I
>> think doing so would be valuable.
> The boost ratio library avoids all kind of overflow that coudl
> resulting of arithmetic operation and can be simplified. The typedefs
> durations don't detect overflow. You will need a representation that
> handles with. I don't think the library should change the standard
> behavior, but could use representation the application gives for
> debug. Would this responds to your needs?
Sorry, I don't follow. I think you're talking about runtime arithmetic
here; I'm talking about compile-time arithmetic. For example
uint64_t const big = uint64_t(1)<<62;
typedef duration<uint64_t, ratio<1, big>> D1;
typedef duration<uint64_t, ratio<1, big-1>> D2;
auto d = d1+d2;
Will the above compile? If so, what is the type of d?
This is really a ratio library question; perhaps if I read the docs
thereof it would tell me. But I'm concerned because of all the implicit
combination of ratios in Boost.Chrono.
>> This function puzzles me:
>> template <class Rep1, class Rep2, class Period> double
>> operator/(const Rep1& s, const duration<Rep2, Period>& d);
> This overloading is used to get the frequency. I added it because I
> needed it for Boost.Stopwatch which report could report the frequency
> of an event.
OK, that makes sense. I can see why it's useful; it just makes me a
little uneasy because the units of the return value depend on the units
of d in a way that most of the operations strive to avoid.
If you added it for Boost.Stopwatch do I take it that it's not in the
>> What is its intended purpose? It seems to violate the type system
>> somewhat by returning a double rather than an 'inverse duration'.
> What would you expect as result?
Unless you returned something from Boost.Units there really isn't a
sensible option (and I'm certainly not suggesting you return something
>> You seem to be guaranteeing that high_resolution_clock is a typedef
>> of one of the other two clocks? Wouldn't it be better to leave
>> open the possibility that it uses some other, more high-resolution
> Yes I could. I don't see nothing in the standard that forbid it. Do
> you have a better implementation for high-resolution_clock and for
> which platform? I could include it if you have one.
For Windows there's QueryPerformanceCounter and
QueryPerformanceFrequency that people are always suggesting for use in
Boost.Timer. I've never used them, but I get the impression they're
higher precision. OTOH, I think that their precision is not known at
compile-time, which may be troublesome.
I believe there are similar things on other platforms. One thing I
would like is a common interface akin to libfftw's getticks. (See the
"Cycle Counters" part of http://www.fftw.org/download.html). This is
also in arbitrary units, not seconds, though, and I'm not sure how many
of the underlying APIs have a known compile-time precision.
So perhaps all these things do not belong in Boost.Chrono.
>>> - What is your evaluation of the documentation?
>> The docs say that the code fragments have an implicit "using
>> namespace boost::chrono;". In fact in practice many of them use
>> the namespace explicitly or include "using namespace chrono;".
>> Could you use a namespace alias so that it is clear exactly which
>> names are taken from this namespace.
> This will take some time and will make the code and the documentation
> more weigth but I understand your concern, as it allows to see
> clearly what is part of the library. I will do it if other think it
> is better for the library.
That's reasonable :).
>> As others have said, I would appreciate a description of what the
>> different clocks are. In particular, I've always found 'monotonic'
>> a bizarre adjective in this context; in what way is it more
>> monotonic than the others? (I think the answer is buried in the
>> reference, under the definition of Clock::is_monotonic).
> I have used the name of the standard. But maybe the name will change
> to steady_clock.
Are you saying the standardised name might change?
> A monotonic clock is a clock for which values of
> time_point never decrease as physical time advances as steady clock
> represent clocks for which values of time_point advance at a steady
> rate relative to real time. That is, the clock may not be adjusted.
> The system_clock could be adjusted to a time_point that decresase the
> last time_point so is is not monotonic.
> I will add the distinction in the documentation.
Thanks. It puzzled me for years :).
>> Along similar lines, I think that somewhere (perhaps the FAQ or
>> the rationale) should explain which APIs have been chosen to
>> implement each clock on each platform, and why.
> I could add something to the Implementation Notes section.
>> If there are any popular APIs for which clocks have not been
>> written, are there plans to do so in the future?
> I'm not aware of all the popular clocks API, but I don't know other
> than the provided by the library.
>> I think it would be worth mentioning (probably in the FAQ)
>> something about benchmarking, since that is a common requirement
>> leading people to want access to clocks. For example it could say
>> which, if any, of the provided clocks are appropriate for
>> benchmarking, and what to watch out for.
> Each clock has his own features. It depends on what do you need to
> benchmark. Most of the time, you could be interested in using a
> thread clock, but if you need to measure code subject to
> synchronization a process clock would be better. If you have a
> multi-process application, a system-wide clock will be needed.
Yes, that kind of information is worth putting n the FAQ, I think.
>> Some time ago Edward Grace proposed on this list some well thought
>> out benchmarking techniques
>> <http://article.gmane.org/gmane.comp.lib.boost.devel/191711>. It
>> would be nice if that or something like it found more prominence
>> (but Boost.Chrono is not the place for that).
> There is Boost.Stopwatch which will be reviewed 1Q/2Q 2011 that
> should cover the needs. Please take a look at and let me know if you
> fid there what you are looking for.
I tried, but the documentation link on LibrariesUnderConstruction
doesn't seem to work (I get a blank page).
>>> And finally, every review should answer this question:
>>> - Do you think the library should be accepted as a Boost
>> Yes, but the docs need more work (both on the high level and in
>> detail). All the reviewers have commented on this so far, and I
>> recommend that when the documentation is revised it should be
>> offered for comment again (preferably to be examined by people who
>> didn't look at it this time :)).
> Thanks a lot. I appreciate all your comment and expect they will help
> me to improve the library documentation and usability.
You're welcome :).
> I will do it of course, but I think that it will be diffcult to find
> other volunters, so it will be great if you could review it again
I doubt I could as well a job on a second reading, but perhaps I could
Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk