Re: [boost] [chrono] v0.4.5 Documentation update + warnings removal + bug fixes

Subject: Re: [boost] [chrono] v0.4.5 Documentation update + warnings removal + bug fixes
From: Stewart, Robert (Robert.Stewart_at_[hidden])
Date: 2010-08-11 08:47:02

vicente.botet wrote:
>
> Even if the review's date is not annonced yet, it will be
> great if some of you make a pre-review, so the conflicting
> issues are managed before the review.

Here are comments from my reading of the initial sections of the document.  Hopefully, I can offer comments on the User's Guide another time.

_____________
Overview

This section appears to be a sibling of the immediately following Description section, but includes a one-item TOC leading to the Motivation section which is also found by following the navigation icons to the next section.  This is confusing.

I'd expect the Overview section to include a TOC leading to the Motivation section first, and the Description section second, which means moving Description to its own page.  I'd expect the How to Use This Documentation section to remain on the same page as the Overview section.

_____________
Description

Para 1, bullet 1:

The durations order is a bit jarring: "minutes, seconds, and nanoseconds" would read better.  As written, that bullet suggests that days might not be included, while duration's interface clearly means that year cannot be, though year360 might be.  Therefore, you might change the bullet to read, "A means to represent time durations managed by the generic [duration] class.  Examples include days, minutes, and nanoseconds, which can be represented with a fixed number of clock ticks per unit."  That better explains what might be included and not.

Para 1, bullet 2:

s/One facility for/A type/

Para 1, bullet 3:

s/any given/a particular/
s/symbolic bundle/pairing/

What is a "native time_point?"  That phrase suggests there are others, so why the distinction here?

Para 2:

Change to read, "The library also provides some infrastructure to support the main features but which may prove useful in user applications.

Para 2, bullet 1:

This same text is repeated in the description of common_type found by following the link.  In both cases, change the description to, "a traits class used to deduce a type common to as many as three types, useful as the return type of functions operating on multiple input types such as in mixed-mode arithmetic."

Para 2, bullet 2:

This can be improved somewhat and note that there is no description like this before the ratio class template documentation found by following the link here.  I suggest this rewording: A class template, [ratio], for specifying compile time, rational constants such as 1/3 of a nanosecond or the number of inches per meter.  ratio represents a compile time ratio of compile time constants with support for compile time arithmetic with overflow and division by zero protection."

Para 3, bullet 1:

You mention a "middle layer" but there has been no previous mention of any layers.  Stopwatch is the first part of the library I would describe as a "facility" given that there is a concept and some class templates.  Hence, "A facility to measure elapsed time with the ability to start, stop, suspend, or resume measurement."

Para 3, bullet 2:

Given this reference to a "higher layer," one might now infer that the features described in the first two paragraphs were the lower layer, but it would be better to avoid references to layers, at least within the Description section.  You also use the term "stopclock" as though its meaning is apparent.  It isn't.  The description doesn't clearly convey the meaning either.  I looked at the documentation for basic_stopclock and stopclock and found nothing to help.  I can infer that it is to measure elapsed time like a stopwatch, but, perhaps, with less precision, though I have no confidence in that inference.

I can't suggest how to rewrite this bullet without understanding "stopclock."

Para 3, bullet 2, subbullet 3:

This bullet isn't of much value in this introductory text, particularly as there is no explicit mention of any characters until this point.

Para 4:

The phrasing is awkward and paragraph five is closely related, yet orphaned.  Try, "To make the timing facilities more generally useful, Boost.Chrono provides a number of clocks that are thin wrappers around the operating system's time APIs, thereby allowing the extraction of read (wall clock) time, user CPU time, system CPU time, etc.:"

Para 4, bullet 1:

s/, capturing real-CPU/ captures real (wall clock) CPU/

Para 4, bullet 2:

s/, capturing user-CPU/ captures user CPU/

Para 4, bullet 3:

s/, capturing system-CPU/ captures system CPU/

Para 4, bullet 4:

Change to, "A tuple-like class, [process_cpu_clock], that captures real, user CPU, and system CPU times together."

Para 5:

Change to a fifth bullet of paragraph four, rewritten as, "Thread clocks, when supported by a platform."

Para 6:

Awkward phrasing.  Try, "Lastly, Boost.Chrono includes typeof registration for [duration] and [time_point] to permit using emulated auto with C++03 compilers."

This should probably reference the Typeof library.

_____________
How to Use This Documentation

Para 1, bullets 3-5:

This are worded repetitively.  Change to this pattern, "Free functions are rendered in the code font followed by (), as in free_function()."

Para 2:

Is there no include all header for Chrono?

_____________
Motivation

This section should precede the Description section.  It is here that you're trying to sell the library.  Once hooked, the reader will want to read a description.

_____
Time

Join the two paragraphs.

Para 2:

The series in the last sentence is written wrongly.  Try, "However, overly simplistic solutions can be dangerous and inefficient, and won't adapt as the computer industry evolves."

Para 4:

The point of referencing N2661 is because it is the basis for time support in C++0x and, therefore, the interfaces provided in Boost.Chrono, unless I'm mistaken.  Therefore, reword this paragraph to, "Boost.Chrono aims to implement the new time facilities in C++0x, as proposed in [N2661: A Foundation to Sleep On].  That document provides background and motivation for key design decisions and is the source of a good deal of information in this documentation."

_____
Wall clock versus system and user time

As with the Description section, this could be worded better.  Try, "To make the timing facilities of Boost.Chrono more generally useful, the library provides a number of clocks that are thin wrappers around the operating system's process time API, thereby allowing the extraction of read (wall clock) time, user CPU time, and system CPU time.  (On POSIX-like systems, this relies on times().  On Windows, it relies on GetProcessTimes().)"

_____
Reporting slapsed[sic] time

s/slapsed/elapsed/ in the heading

Para 1:

s/block/code block/
s/captures the mechanism to measure the/is a mechanism to measure/
s/eleapsed/elapsed/

Para 1, last sentence:

I'm not sure what you mean by "allowing to make a single measure."

Para 2, sentence 1:

Try, "It is often necessary to report elapsed time on a user display or in a log file.  [stopwatch_reporter<>] provides a runtime reporting mechanism for this purpose which can be invoked in just one line of code."

_____
Caveat emptor

This section doesn't belong in Motivation.  This should be in the Description section, I think.

_____
How reliable are these measures?

This section doesn't belong in Motivation.  This should be in the Getting Started section, I think.

Para 1:

s/context on which you can get/cases which can lead to/

Para 1, bullet 1:

This can be worded more clearly: "It is not possible to measure events that transpire at rates of the same order of magnitude as the clock's precision with any reliability.  For example, a 10ms clock cannot be used reliably to measure elapsed times of tens of milliseconds.  The library provides a [high_resolution_clock] that gives you the highest resolution time available on your platform.  That will give the best precision, but can only be used for reliable measurement of events that elapse about an order of magnitude slower than that clock's precision."

Para 1, bullet 2:

Try, "Using a process clock in a multithreaded application will give elapsed time for the process as a whole, including threads other than the calling thread.  To get time elapsed for a specific thread, use the supplied [thread_clock] which returns time elapsed for the calling thread only, if supported by the platform."

Para 1, bullet 3:

This can be expressed more clearly: "When stopclocks are nested, usually from stopclocks appearing in each of several nested function calls, the overhead of the stopclock processing begins to be significant relative to run time of the code being measured.  The innermost measurements remain accurate, but those in the outermost layers can measure too much overhead to be trustworthy."

Para 2:

Try this rephrasing: Most of the stopclock overhead is likely due to logging.

Para 2, bullet 1:

The wording is awkward.  Try, "Don't flush log information while measuring elapsed time.  A [stopwatch_accumulator] can make that simple.  Alternatively, an asynchronous stream would permit normal logging but by a thread other than the one being measured."

Para 2, bullet 2:

s/intrinsec/intrinsic/

This could be worded more clearly: "Add a mechanism to track the difference between the application time and stopclock time.  If a [Clock] models [SuspendibleClock] and its precision is sufficiently fine, this mechanism could suspend the [Clock]'s counting while reporting measurements and resume it thereafter."

_____________
Template Class stopwatch_reporter<>

s/a everything/everything/
s/precission/precision/

Why is stopwatch_reporter a stopwatch plus reporting?  That is, why doesn't stopwatch include the reporting and have done or stopwatch_reporter reference a stopwatch?  There should be something describing such design decisions in your documentation, whether here or elsewhere is unimportant.

_____
Rob Stewart                           robert.stewart_at_[hidden]
Software Engineer, Core Software      using std::disclaimer;
Susquehanna International Group, LLP  http://www.sig.com

IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.