Re: [boost] Does Boost.Pool have a maintainer ?

Subject: Re: [boost] Does Boost.Pool have a maintainer ?
From: DUPUIS Etienne (e.dupuis_at_[hidden])
Date: 2012-07-03 02:43:19

> I have a few comments on your changes:
>
> - Why did you remove the valgrind sections?

See ticket #3976 (https://svn.boost.org/trac/boost/ticket/3976), closed as "won't fix" with explanations by John Maddock.

The valgrind sections is a reimplementation of pool that does no pooling to help users of valgrind validate their code (in this implementation a newly requested pool buffer is always the result of a malloc thus the memory is seen as unititialized by valgrind). This being said, the standard implementation of pool causes no problem with valgrind; it is only that since we are reusing again and again the same buffers (which is the purpose of pool), the second time a buffer is used valgrind considers its content as being initialized and thus would not warn the user of uninitialized memory errors.

Removing the valgrind sections is of course completely orthogonal to the bug fixes; this change can be reverted.

>
> - You used the textbook implementation of merge
>   sort.  Mergesort for lists generally looks
>   more like this (untested):
>
> [...]
>
> - You've duplicated sort in simple_segregated_storage
>   and PODPtr.
>

I wished to use a sort done by someone else and not code one myself. After a quick search it turned out the the simplest way seemed to include a sort in both the simple_segregated_storage and PODPtr classes, which are two different (albeit very similar) implementations of a single linked list. Having no particular knowledge of sort routines, I used a didactical implementation to guide me.

I assume that the code snippet you have included is more efficient ?

> - You should use std::less<void*> in sort to match the
>   sorting done by ordered_free.

Instead of std::less<char *> ? Ok.

>
> - pool::ordered should be set in the constructor
>   initializer list.

Ok.

>
> - I don't think that boost::pool should track whether
>   it is ordered.  This tracking is only needed by
>   object_pool, so object_pool should maintain it.
>   Also, I don't think that this flag affects the
>   behavior of object_pool at all, since object_pool
>   will always call pool::malloc, thus setting ordered
>   to false.

I made Boost::pool track the ordered state to allow boost::pool::release_memory() to be called. Previously, this method could be called only on ordered pool. Now it can be called on any pool; if the pool is no longer ordered, it will be ordered. You are right that when using object_pool, this flag will always be set to false.

>
> - I don't think that a user specified value of
>   next_size or max_size which is too large should
>   be silently truncated.  It should be an error.
>

The current implementation of pool crashes silently if given erroneous parameters. I think this is bad and that the behavior should be one of
(i) Assertion fails if assertions are enabled (and crash if not);
(ii) Returns an error or throws an exception;
(iii) Silently correct the input parameters.

I have used (iii) but I may as well use (i) or (ii), whichever is preferred by the community.

>
> - I have no idea what static_pool is supposed to do.
>

I have not yet completely updated the documentation; I failed to build it. Paul Bristow, who updated it last year, offered to help.

The static_pool is a simple subclass that allocates all memory when the constructor is called; the pool thus can not grow dynamically. It was only intended as an "helper" class. On the boost mailing list someone suggest adding a policy for pool growth, which is a better idea than having a specific class.

Perhaps the best would be to remove this class, start by addressing the open tickets and then, if needed, add new features.

> - array_pool::memory has to be aligned.

Right. I intended to remove this class that I added but think is useless.

>
> In Christ,
> Steven Watanabe
>

Thank you for taking the time to look; looking forward to read your comments.
Étienne Dupuis

Ce message et toutes les pièces jointes sont confidentiels et établis à l'intention exclusive de ses destinataires. Toute modification, édition, utilisation ou diffusion non autorisée est interdite. Si vous avez reçu ce message par erreur, merci de nous en avertir immédiatement. ATEME décline toute responsabilité au titre de ce message s'il a été altéré, déformé, falsifié ou encore édité ou diffusé sans autorisation.
This message and any attachments are confidential and intended solely for the addressees. Any unauthorized modification, edition, use or dissemination is prohibited. If you have received this message by mistake, please notify us immediately. ATEME decline all responsibility for this message if it has been altered, deformed, falsified or even edited or disseminated without authorization.

Note: To protect against computer viruses, e-mail programs may prevent sending or receiving certain types of file attachments.