Re: [boost] [review] Review of PolyCollection starts today (May 3rd)

El 10/05/2017 a las 17:56, Hans Dembinski via Boost escribió:

Hi,

I am not a Boost author, but I think I am able to give a review of PolyCollection, so here it goes.

Thanks for your review!

[...]

...

- What is your evaluation of the documentation? [...]

boost::function_collection was not so clear to me. It is nice to keep the narrative of game development going, but like someone said before, I think it would help to give a simpler example. I felt that the benefit of using boost::function_collection was not so clearly presented as for boost::base_collection. The function pointers are all the same size, so how exactly does the optimisation enter? I assume it is not about memory allocation here, but about sorting equal function signatures into homogeneous blocks to improve branch prediction and locality, but I am not sure.

I have to think over the boost::function_collection given that it does't look as clear as I expected... As you correctly guess my aim was to have a single narrative, but seems like that's not working so well. Regarding the benefits of boost::function_collection<Signature> with respect to std::vector, memory allocation improves as well: except where small object optimization applies, each std::function object stores a pointer to a heap-allocated copy of the wrapped callable entity passed by the user; boost::base_collection, on the contrary, stores same-type callable entities contiguously. (If you are really curious, a segment of a boost::function_collection is composed of *two* vectors, one for the callable entities proper and another storing std::function-like objects pointing to the former. Think of it as a std::vector constructed with such a smart memory allocator that (same-type) wrapped callable entities all happen to lie sequentially in memory.)

[...]

...

- Did you try to use the library? With what compiler? Did you have any problems? I tried to compile the tests on a Mac with Apple-clang 8.0.0. It worked after I manually specified "cxxflags=-std=c++11" in the b2 call. By the way, it would be great if b2 used the highest standard that is available by default.

It's weird because the Jamfile does have this flag already: https://github.com/joaquintides/poly_collection/blob/master/test/Jamfile.v2 I guess the snapshot provided from the Incubator might be oldish and not include that... I can't check right now because blincubator.com is not responding :-(

[...]

...

- Do you think the library should be accepted as a Boost library? Yes.

Thank you! Joaquín M López Muñoz