Re: [boost] [Review] Formal Review of Proposed Boost.Chrono Library Starts TOMORROW

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 
fairly comprehensively.

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

-Dave Deakins