Subject: Re: [boost] [Review] Formal Review of Proposed Boost.Chrono Library Starts TOMORROW
From: David Deakins (ddeakins_at_[hidden])
Date: 2010-11-16 02:31:08
On 11/5/2010 11:25 AM, Anthony Williams wrote:
> - What is your evaluation of the design?
I think the design is well thought out. It cleanly separates out the
basic operations for capturing, constructing, and comparing points in
time from the higher-level operations of relating times to things like
the Gregorian calendar. At the same time, it provides a good platform
upon which these higher-level operations can be built.
> - What is your evaluation of the implementation?
I have not delved into all parts of the implementation, but what I have
looked at seems sound.
> - What is your evaluation of the documentation?
The documentation is good. It gives a clear explanations of the basic
components of the design (durations, time_points, and clocks). It also
does a good job of explaining the more subtle issues of converting
between different types of durations and when exact conversions are
guaranteed vs. when approximation will be performed.
There are small typographical or grammatical errors here and there in
the documentation, but I believe others have already covered these
> - What is your evaluation of the potential usefulness of the library?
Very useful. Chrono provides the potential for an extensible and
efficient way to standardize use of time across the many Boost libraries
that traffic in timing, timers, and waiting (Boost.Thread, Boost.Asio, etc).
> - Did you try to use the library? With what compiler? Did you have any problems?
Yes, with VC9 SP1 on Win7. I ran into some minor issues with pieces of
the ratio library triggering internal compiler errors on VC9, but I was
able to resolve these. I also noticed that the jam files for the
regression tests reference test files that did not seem to be included
in the distribution in the sandbox. Both of these should be easily fixed.
I also did some testing with VC9 SP1 cross-compiling for Windows CE 5.0.
I ran into some minor issues (as expected) mainly related to needing
the combination of GetSystemTime and SystemTimeToFileTime calls instead
of GetSystemTimeAsFileTime (which the WinCE libraries sadly do not
supply). Other than this, it functioned well.
> - How much effort did you put into your evaluation? A glance? A quick
> - reading? In-depth study?
I wouldn't say an in-depth study, but some fairly involved work. We had
integrated some of the earlier versions of Boost.Chrono into our local
Boost tree and had made adaptations to Boost.Thread and Boost.Asio to
make use of the Chrono functionality. The Chrono-based Asio was used in
some applications that do automated data collection from web pages, and
Chrono-based Thread and Chrono itself were used within Win32 and WinCE
applications for automated control systems. Some of the basic Chrono
components were used for performance timing in tuning of application code.
> - Are you knowledgeable about the problem domain?
Somewhat, but certainly not an expert.
> And finally, every review should answer this question:
> - Do you think the library should be accepted as a Boost library?
Yes, I think Chrono should be accepted into Boost.
Thanks, Vincente, for all your hard work in crafting this into a formal
library submission, seeing through the restructuring to properly
partition all the components, and filling in the documentation and testing.
Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk