Re: [boost] [auto_buffer] Interest check: container with preallocated buffer

Subject: Re: [boost] [auto_buffer] Interest check: container with preallocated buffer
From: Stewart, Robert (Robert.Stewart_at_[hidden])
Date: 2009-05-19 09:09:54

Thorsten Ottosen wrote
On Monday, May 18, 2009 9:05 AM
> Dmitry Vinogradov skrev:
>
> > Does any container exist to offer functionality like
> Boost.Array but
> > allowing to store from 0 to N elements? Is there any
> interest in such
> > container?
> >
> > PS. It's similar to fixed_string but as a generic container.
>
> Please see
>
> http://www.cs.aau.dk/~nesotto/boost/auto_buffer.zip
>
> http://www.cs.aau.dk/~nesotto/boost/trunk/libs/auto_buffer/doc/html/

auto_buffer looks really useful, Thorsten, but I'd like to pick at it a little.

boost::default_grow_policy is troubling.  First, s/grow/growth/ in the name.  Did you mean to that policy as appropriate for all of Boost?  You put it in the boost namespace.  Third, new_capacity() ought to be "reserve" because that more clearly associates it with auto_buffer<>::reserve() and the well understood behavior of, for example, std::vector<>::reserve().

auto_buffer's "StackBufferPolicy" template parameter should be named "SizingPolicy" because it has to do with computing the buffer's size.  The "GrowPolicy" template parameter should be named "GrowthPolicy."

The auto_buffer nested value "N" is confusing as it will change based upon the SizingPolicy, but won't match that policy's "value" if using boost::store_n_bytes, and it isn't the number of elements in, or capacity of, the auto_buffer, particularly when memory is allocated from the allocator.  Renaming it to "stack_capacity" should work, though.

Is there a compile time assertion for N > 0 for store_n_objects?  How about that store_n_bytes<N>::value >= sizeof(T)?  Those will help to diagnose misuse.

There should be a debug build assertion that GrowthPolicy::reserve(n) returns a value >= n to help diagnose mistakes.

"optimized_const_reference" would be better named "by_value_type" or something.  For small types, optimized_const_reference isn't a reference at all, so the name is misleading.  Better to name it for the usage rather than the type.

What's the point of push_back() with no argument?  What's the motivation for that versus making the caller explicitly default construct a T and pass that to push_back(by_value_type)?  Are you simply optimizing the copying?  If so, isn't it better to mimic vector and gain the optimization with move semantics?

Shouldn't pop_back_n() be an algorithm rather than a member function?  Is there really efficiency to be gained by making it a member and is there a good use case for that?

Rather than reserve_precisely(), which should be named "reserve_exactly" if retained, why not just document reserve() as allocating what GrowthPolicy::reserve() returns and leave the behavior to the policy?  That leaves room for reserve() allocating exactly what's requested, or rounding up according to some computation, as the policy dictates.

The remarks for the uninitialized_*() functions use the phrase, "depending on the application," but that doesn't explain when the user is responsible to initialize or destroy the n elements.  Change the remarks to note that if T is POD, the user need not initialize/destroy the affected elements, whereas not doing so for non-POD types results in undefined behavior.

The unchecked_*() functions deserve remarks that explain the danger in their use and that the precondition is not checked.  If you don't already, though, you should check the precondition in debug builds to help clients use these dangerous functions correctly.

The description of should_shrink(), on the default_grow_policy class, mentions shrink_to_fit() which is not mentioned elsewhere in the documentation.  I find shrink_to_fit() in Boost.Interprocess, but nowhere else.  Did you forget something?

shrink_to_stack() might be a useful addition.  You probably inferred the purpose: to shrink the size to what will fit on the stack and move the contents to the stack from allocator-supplied memory, if used.  That seems the logical converse of dynamically growing beyond the stack allocation, though I admit I can't think of a motivating use case beyond simply trying to keep memory consumption low in a long running function.

There are numerous things to fix in the docs, plus more to add, of course, but I don't want to address those points now.  I think I can find some time to help you flesh out the docs so you can submit auto_buffer for review, if you're interested.

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