[poly_collection] Small review

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

[poly_collection] Small review

Boost - Dev mailing list

>
> Dear Ion, Joaquin and boost mailing list,
>  
> I hope this brief review is of some value to you. I don’t expect to be a recognised name on the mailing list but have contributed to it intermittently over the years. I have also used the author’s MultiIndex library in production for about a decade.
>  
> I would recommend that PolyCollection be accepted into Boost.
>  
> I think the design pattern is common enough to be Boost-ified. I have done something very similar to boost::base_collection myself in the past and have achieved useful speed ups over a solution like boost::ptr_vector in a production system. My speed-ups were not quite as good as Joaquin documents, but I can’t tell if this was due to the ineptitude of my own solution or due to lab conditions of the Boost.PolyCollection tests.
>  
> I spent a couple of hours reading the docs and running basic examples on my default compiler (VS2015 x64) and did not experience any significant problems.
>  
> It may be worth remarking in the docs (sorry if I missed it) what facilities are used for the type registries that are presumably behind the scenes. From the reference section I think RTTI is required. Is this a hard limit or could Boost.TypeIndex be used?
>  
> NB the reference section has a note “REVIEW THIS DESIGN DECISION” in it.
>  
> Personally I would attach value to the fact that the author has been a great servant to Boost over the years (as have  many others) so it is likely that this would be a well-maintained library. E.g. if algorithms were added to <algorithm> in the future, they would likely end up in <boost/poly_collection/algorithm.hpp> too.
>  
> Regards,
> Pete
>

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [poly_collection] Small review

Boost - Dev mailing list
El 09/05/2017 a las 12:20, Pete Bartlett via Boost escribió:
>
>> Dear Ion, Joaquin and boost mailing list,
>>  
>> I hope this brief review is of some value to you. I don’t expect to be a recognised name on the mailing list but have contributed to it intermittently over the years. I have also used the author’s MultiIndex library in production for about a decade.
>>  
>> I would recommend that PolyCollection be accepted into Boost.

Thank you for your interest!

>> [...]
>>  
>> It may be worth remarking in the docs (sorry if I missed it) what facilities are used for the type registries that are presumably behind the scenes. From the reference section I think RTTI is required. Is this a hard limit or could Boost.TypeIndex be used?


typeid is used extensively throughout the library, both the static
(typeid(T)) and the dynamic
version (typeid(x)) --the latter only in base_collection and in one spot in
boost/poly_collection/detail/segment.hpp. std::type_index is part of the
public interface.
Boost.TypeIndex could be in principle optionally used for RTTI-less
scenarios, but I don't really
know whether this is a common enough / requested scenario.

>>   NB the reference section has a note “REVIEW THIS DESIGN DECISION” in it.

Yes, the behavior of max_size() and capacity() is something I'm far from
satisfied with and plan
to fix before inclusion into Boost (if accepted). Please follow this
thread for further info:

https://lists.boost.org/Archives/boost/2016/11/231681.php

Joaquín M López Muñoz

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [poly_collection] Small review

Boost - Dev mailing list
On 09/05/2017 13:39, Joaquin M López Muñoz via Boost wrote:
> typeid is used extensively throughout the library, both the static
> (typeid(T)) and the dynamic
> version (typeid(x)) --the latter only in base_collection and in one spot in
> boost/poly_collection/detail/segment.hpp. std::type_index is part of the
> public interface.
> Boost.TypeIndex could be in principle optionally used for RTTI-less
> scenarios, but I don't really
> know whether this is a common enough / requested scenario.

I was planning my own review after the library review was closed (so
that the review manager does not influence other reviewers) and one of
the comments was related to RTTI, so I'm making my comments now for
discussion.

Basic examples in the documentation are based on games concepts (sprite,
warrior, goblin, ...) and usually in game development builds RTTI is
disabled although polymorphism is widely used. My comment is about
making the runtime-identification configurable by the user

I can imagine an application where all classes are known and a custom
typeid can be generated and stored in the abstract base class of every
type, so that it can be retrieved via an inline function, without
noticeable run-time overhead. It could just cache a std::type_info pointer.

I think a lot of game engines use their own RTTI framework that could be
easily plugged into PolyCollection (maybe as an additional TypeIdTraits
template parameter). Something like:

class typeid_traits
{
    typedef /**/ type_info_t;
    typedef /**/ type_index_t;

    template<class T>
    static type_info_t typeid_of(T *p);

    //Usage: const type_info_t &ti = typeid_of<MyClass>();
    template<class T>
    static type_info_t typeid_of();
};

type_index_t should be constructible from type_info, hashable and
comparable.

This type of abstraction could support Boost.TypeInfo and many other
approaches. Anyone sees this customization useful?

Best,

Ion

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [poly_collection] Small review

Boost - Dev mailing list
El 09/05/2017 a las 15:46, Ion Gaztañaga via Boost escribió:

> On 09/05/2017 13:39, Joaquin M López Muñoz via Boost wrote:
>> typeid is used extensively throughout the library, both the static
>> (typeid(T)) and the dynamic
>> version (typeid(x)) --the latter only in base_collection and in one
>> spot in
>> boost/poly_collection/detail/segment.hpp. std::type_index is part of the
>> public interface.
>> Boost.TypeIndex could be in principle optionally used for RTTI-less
>> scenarios, but I don't really
>> know whether this is a common enough / requested scenario.
>
> [...]
>
> Basic examples in the documentation are based on games concepts
> (sprite, warrior, goblin, ...) and usually in game development builds
> RTTI is disabled although polymorphism is widely used. My comment is
> about making the runtime-identification configurable by the user
>
> I can imagine an application where all classes are known and a custom
> typeid can be generated and stored in the abstract base class of every
> type, so that it can be retrieved via an inline function, without
> noticeable run-time overhead. It could just cache a std::type_info
> pointer.
>
> I think a lot of game engines use their own RTTI framework that could
> be easily plugged into PolyCollection (maybe as an additional
> TypeIdTraits template parameter). Something like:
>
> class typeid_traits
> {
>    typedef /**/ type_info_t;
>    typedef /**/ type_index_t;
>
>    template<class T>
>    static type_info_t typeid_of(T *p);
>
>    //Usage: const type_info_t &ti = typeid_of<MyClass>();
>    template<class T>
>    static type_info_t typeid_of();
> };
>
> type_index_t should be constructible from type_info, hashable and
> comparable.
>
> This type of abstraction could support Boost.TypeInfo and many other
> approaches. Anyone sees this customization useful?

I'm no expert in game development so I can't really assess how common
this custom
typeid thing is. In any case, if the community deems this worth
including I can
definitely work on it; one thing I don't get about your proposed design
is the
insistence on the type_info_t/type_index_t distinction: to the best of
my knowledge
std::type_index should have been what typeid() returned and was added as
a patch
over less useful typeinfo so as not to break backwards compatibility, so
here we can just
as well learn from the past and simply use type_index_t (with typeid_of
directly
returning type_index_'s rather than  type_info_t's), right? Two further
doubts:

* How does this combine with usage of Boost.TypeIndex? I mean, can usage of
Boost.TypeIndex be shoehorned into this typeid_traits framework as an
alternative
RTTI system (among say others custom-designed by game developers)?
* function_collection and any_collection share 99% of the code with
base_collection
(the three containers are basically the same with different so-called
"model"
policies), so this traits thing could extend almost automatically to
them. But I fail
to see what the use of it would be. Except when the selected traits are
those of
Boost.TypeIndex... well, possibilities abound here.

Joaquín M López Muñoz

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

[poly_collection] Custom RTTI with PolyCollection (was: Small review)

Boost - Dev mailing list
On 11/05/2017 8:26, Joaquin M López Muñoz via Boost wrote:

> El 09/05/2017 a las 15:46, Ion Gaztañaga via Boost escribió:
>> On 09/05/2017 13:39, Joaquin M López Muñoz via Boost wrote:
>>> typeid is used extensively throughout the library, both the static
>>> (typeid(T)) and the dynamic
>>> version (typeid(x)) --the latter only in base_collection and in one
>>> spot in
>>> boost/poly_collection/detail/segment.hpp. std::type_index is part of the
>>> public interface.
>>> Boost.TypeIndex could be in principle optionally used for RTTI-less
>>> scenarios, but I don't really
>>> know whether this is a common enough / requested scenario.
>>
>> [...]
>>
>> Basic examples in the documentation are based on games concepts
>> (sprite, warrior, goblin, ...) and usually in game development builds
>> RTTI is disabled although polymorphism is widely used. My comment is
>> about making the runtime-identification configurable by the user
>>
>> I can imagine an application where all classes are known and a custom
>> typeid can be generated and stored in the abstract base class of every
>> type, so that it can be retrieved via an inline function, without
>> noticeable run-time overhead. It could just cache a std::type_info
>> pointer.
>>
>> I think a lot of game engines use their own RTTI framework that could
>> be easily plugged into PolyCollection (maybe as an additional
>> TypeIdTraits template parameter). Something like:
>>
>> class typeid_traits
>> {
>>    typedef /**/ type_info_t;
>>    typedef /**/ type_index_t;
>>
>>    template<class T>
>>    static type_info_t typeid_of(T *p);
>>
>>    //Usage: const type_info_t &ti = typeid_of<MyClass>();
>>    template<class T>
>>    static type_info_t typeid_of();
>> };
>>
>> type_index_t should be constructible from type_info, hashable and
>> comparable.
>>
>> This type of abstraction could support Boost.TypeInfo and many other
>> approaches. Anyone sees this customization useful?
>
> I'm no expert in game development so I can't really assess how common
> this custom
> typeid thing is. In any case, if the community deems this worth
> including I can
> definitely work on it; one thing I don't get about your proposed design
> is the
> insistence on the type_info_t/type_index_t distinction: to the best of
> my knowledge
> std::type_index should have been what typeid() returned and was added as
> a patch
> over less useful typeinfo so as not to break backwards compatibility, so
> here we can just
> as well learn from the past and simply use type_index_t (with typeid_of
> directly
> returning type_index_'s rather than  type_info_t's), right?

Yes, it was just a quick sketch, a single opaque type would work better.
The idea is to synthesize the minimum abstraction that PolyCollection needs.

Two further
> doubts:
>
> * How does this combine with usage of Boost.TypeIndex? I mean, can usage of
> Boost.TypeIndex be shoehorned into this typeid_traits framework as an
> alternative

I have no previous experience but Boost.TypeIndex claims to be a drop-in
replacement for std::type_index:

typeid(T) -> boost::typeindex::type_id<T>()
typeid(variable) -> boost::typeindex::type_id_runtime(variable)

so I expect that Boost.TypeIndex integration will be fine. In general, a
very simple scheme of:

- Obtaining the typeid from a type
- Obtaining the typeid from a variable
- Be able to compare and hash the typeid.

could work with most alternatives. The scheme where the abstract base
class stores the id (say, an integer or a pointer) and offers it using
an inline function is certainly easily implementable with this
straightforward concept.

> * function_collection and any_collection share 99% of the code with
> base_collection
> (the three containers are basically the same with different so-called
> "model"
> policies), so this traits thing could extend almost automatically to
> them. But I fail
> to see what the use of it would be. Except when the selected traits are
> those of
> Boost.TypeIndex... well, possibilities abound here.

In general, if RTTI is disabled and any heterogeneous callback container
is needed (which is also typical in some environments) an alternative
must be used to distinguish between callable types at runtime. I have
never used a custom RTTI with callable types but I can imagine an scheme
where different function objects are required to have some kind of
protocol (say, base class) that returns the typeid. I don't know if raw
function pointers or "member to function types + this" can be easily
detected and an implicit typeid can be used. But I haven't thought much
about it, the main use case, I think, is related to inheritance
polymorphism.

The "model" policy is a very interesting design that is (until now)
hidden in the implementation. It seems extremely powerful. I will study
it a bit further because it might provide a radical generalization
opportunity for user-defined polymorphism models beyond the three
provided by default by PolyCollection.

Ion

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost