Re: [boost] Pool Review.

From: Jens Maurer (Jens.Maurer_at_[hidden])
Date: 2000-11-30 17:56:34

Here are my comments.

The library seems to be useful, at least as an example how to program
a Standard allocator.  It should be accepted into boost.

Some more detailed comments (looking at non_template_pool.zip)

Documentation
-------------

 - The documentation needs some boost-ification.  At least the index.html
page should have the standard boost logo and menu.
 - I would rather move the copyright/warranty notice to the bottom.
 - The symbol "T" is often used for rather complex to-be-described
types such as sized_pool.  I find this odd, considering that T is
usually the element type in descriptions of the STL.  What about "A"
(for "allocator") or "P" ("pool")?  Or just use the full name?

concepts.html
 - "synce" -> "since"
 - … is not recognized by Netscape 4.72.  Please check with the
HTML standard to make sure it's legal.
 - nice explanations
 - "Simple Segregated Storage is the basic idea behind the Pool": this
is the first time the term "Pool" is introduced in a defining context.
However, it doesn't really define the term.  So, what's meant by "Pool"?
"Boost Pool Allocator Library"?
 - Some of the HTML table-based diagrams are centred, others arent't,
at least in my Netscpae 4.72.  I opt for consistency.
 - In the diagrams: "Chunk X; points to NULL".  First, using NULL in
C++ is not stylish, second "NULL" is not an object, but an address.
Curiously enough, an address where no object will ever be.  So
"points to" seems inappropriate.  What about "end of chain" or
"points to no other chunk"=

pool_alloc.html
 - Introduction: Please add a reference to the section number in the
Standard where the Standard Allocator requirements are laid down
 - Make the default template argument FirstArrayBound either
explicitly implementation-defined or provide a specific value.

pool.html
 - The return type for "t.free(p)" and "t.destroy(p)" is "void"
and not "not used".  In particular, these return types are different
from the constructor and destructor return types (which have none
at all, not even void).
 - Please highlight in the "Introduction" section or somewhere else
that the allocation functions return 0 on out-of-memory (instead of
throwing std::bad_alloc) (yes, it's in the notes, but it may be
overlooked easily).

sized_pool.html
 - Semantics: The symbol table says that "u" must be of type
"void * const".  However, top-level constness is ignored for
function parameter matching, so this seems useless.
 - Semantics: Is the exponential growth strategy only applied if I
use the two-argument constructor? The description gives that
impression.  If not, move that to a general section.

sized_pool_impl.html
 - Are memory blocks (allocated by new[]) ever freed again?  If
not, please say so.
 - Some more "pointer to NULL" in the HTML tables.

Organization
------------

 - The final directory structure should clearly separate the required
source files from examples and documentation, since this library is
not headers-only.  See libs/python for an example.

 - "singleton" looks like a separate concept; it should be separate.

 - "mutex"/"guard" is hopefully obsoleted by the efforts of the
boost thread synchronization library.  Have we agreed on an interface
yet?

 - Likewise, "postulate" should be obsoleted by the boost
compile-time assertions.

Jens Maurer