PolyCollection requiring copyable
I'm impressed with PolyCollection. I use the list<interface*> strategy tons in my projects, and I didn't think NOT using a pointer there was even possible... but somehow you managed. It also makes sense why PolyCollection is faster to execute etc, the tutorial was well written. The only 'con' is that it requires a copy constructor. You mention later that some copy traits thing can be used to move the "copy code" outside of the class, but I am questioning the need for copying altogether. Is "copying" of the polymorphic types necessary for: a) Technical requirement otherwise it wouldn't compile b) Design decision because you think it's superior and helpful c) To facilitate copying of the PolyCollection object itself ....? If (b) or (c), can there be a slightly altered version of PolyCollection which does not allow/require copying? I'm completely fine (and even happier) _not_ being able to copy the PolyCollection. When I use the list<interface*> strategy, I tend to consider the list to be the owner of it's objects (and delete them when the list is deleted). There should be PolyCollection::emplace and the poly instances should "live" in the PolyCollection. Accessing the poly instances should be by [const] reference only (isn't this already the case? what benefit does the copy constructor bring?!?). I didn't examine the source so maybe it's (a) after all, in which case I apologize for wasting precious time, bandwidth, and disk space. d3fault
AMDG On 10/03/2017 06:56 PM, d3fault via Boost-users wrote:
I'm impressed with PolyCollection. I use the list<interface*> strategy tons in my projects, and I didn't think NOT using a pointer there was even possible... but somehow you managed. It also makes sense why PolyCollection is faster to execute etc, the tutorial was well written.
The only 'con' is that it requires a copy constructor. You mention later that some copy traits thing can be used to move the "copy code" outside of the class, but I am questioning the need for copying altogether.
PolyCollection uses contiguous storage (aka std::vector). This requires copying when the segment is resized.
Is "copying" of the polymorphic types necessary for: a) Technical requirement otherwise it wouldn't compile b) Design decision because you think it's superior and helpful c) To facilitate copying of the PolyCollection object itself ....?
If (b) or (c), can there be a slightly altered version of PolyCollection which does not allow/require copying? I'm completely fine (and even happier) _not_ being able to copy the PolyCollection. When I use the list<interface*> strategy, I tend to consider the list to be the owner of it's objects (and delete them when the list is deleted). There should be PolyCollection::emplace and the poly instances should "live" in the PolyCollection. Accessing the poly instances should be by [const] reference only (isn't this already the case? what benefit does the copy constructor bring?!?).
I didn't examine the source so maybe it's (a) after all, in which case I apologize for wasting precious time, bandwidth, and disk space.
In Christ, Steven Watanabe
El 04/10/2017 a las 4:16, Steven Watanabe via Boost-users escribió:
AMDG
On 10/03/2017 06:56 PM, d3fault via Boost-users wrote:
I'm impressed with PolyCollection. I use the list<interface*> strategy tons in my projects, and I didn't think NOT using a pointer there was even possible... but somehow you managed. It also makes sense why PolyCollection is faster to execute etc, the tutorial was well written.
The only 'con' is that it requires a copy constructor. You mention later that some copy traits thing can be used to move the "copy code" outside of the class, but I am questioning the need for copying altogether. PolyCollection uses contiguous storage (aka std::vector). This requires copying when the segment is resized.
Adding to Steven's answer, it'd have been feasible to not require move constructibilty/assignability (which is the minimum requisite) but I decided that the resulting available API was not worth it. You can simulate that scenario as follows: #include <boost/poly_collection/base_collection.hpp> struct not_moveable:std::logic_error { not_moveable():std::logic_error("type not moveable"){} }; struct base { virtual ~base()=default; base()=default; base(base&&){throw not_moveable{};} base& operator=(base&&){throw not_moveable{};} }; struct derived1:base{}; struct derived2:base{}; int main() { boost::base_collection<base> c; c.reserve<derived1>(100); c.reserve<derived2>(100); for(int i=0;i<100;++i){ c.emplace<derived1>(); c.emplace<derived2>(); } } This compiles and works fine; of course, if a segment reallocation is tried (when adding the 101st element) a not_moveable exception is thrown. Joaquín M López Muñoz
On 04/10/2017 9:09, Joaquin M López Muñoz via Boost-users wrote:
This compiles and works fine; of course, if a segment reallocation is tried (when adding the 101st element) a not_moveable exception is thrown.
But I understand users don't care about the internal reallocation, they just want to add more elements to the container, and they maybe can't modify the definition of the base class / interface. Ion
El 04/10/2017 a las 16:28, Ion Gaztañaga via Boost-users escribió:
On 04/10/2017 9:09, Joaquin M López Muñoz via Boost-users wrote:
This compiles and works fine; of course, if a segment reallocation is tried (when adding the 101st element) a not_moveable exception is thrown.
But I understand users don't care about the internal reallocation, they just want to add more elements to the container, and they maybe can't modify the definition of the base class / interface.
Absolutely, this was just an example of the resulting behavior with a non-moveable class. I still am of the opinion that the available interface would be too restrictive to consider the case with the lib. Joaquín M López Muñoz
On 10/3/17, Steven Watanabe via Boost-users <boost-users@lists.boost.org> wrote:
PolyCollection uses contiguous storage (aka std::vector). This requires copying when the segment is resized.
Ahh that makes sense. Just throwing a random idea out here: would a semi-contiguous design be possible? PolyCollection could maintain a list of [contiguous] segments, something along the lines of how "extents" work in the filesystem world? You could get 99% cache hits and only a few misses when jumping to the next segments. idk maybe this idea sucks xD. On 10/4/17, Ion Gaztañaga via Boost-users <boost-users@lists.boost.org> wrote:
But I understand users don't care about the internal reallocation,
Usually that's the case, but PolyCollection is... more or less... just a more efficient way of storing/processing a list<interface*>. If you don't care about the internals, then you don't care about efficiency, and you should just use list<interface*> (or vector or whatever). Thanks for that workaround Joaquin M López Muñoz. It's interesting, but not quite what I'm looking for (want both move and copy constructors deleted). d3fault
On 04/10/2017 21:10, d3fault via Boost-users wrote:
On 10/3/17, Steven Watanabe via Boost-users <boost-users@lists.boost.org> wrote:
PolyCollection uses contiguous storage (aka std::vector). This requires copying when the segment is resized.
Ahh that makes sense. Just throwing a random idea out here: would a semi-contiguous design be possible? PolyCollection could maintain a list of [contiguous] segments, something along the lines of how "extents" work in the filesystem world? You could get 99% cache hits and only a few misses when jumping to the next segments. idk maybe this idea sucks xD.
I don't remember if this was commented in the review for future improvements, but it's an idea I wanted to share. If the internal sequence type to hold the collection of objects of the same type could be specified as an option, a deque-like sequence could allow non-movable and non-copyable types, at least when insertion position is not specified, and even stable references and pointers for such insertions. A stable_vector based storage would allow even more operations for such types, but we would loose element contiguity. Ion
El 04/10/2017 a las 22:38, Ion Gaztañaga via Boost-users escribió:
On 04/10/2017 21:10, d3fault via Boost-users wrote:
On 10/3/17, Steven Watanabe via Boost-users <boost-users@lists.boost.org> wrote:
PolyCollection uses contiguous storage (aka std::vector). This requires copying when the segment is resized.
Ahh that makes sense. Just throwing a random idea out here: would a semi-contiguous design be possible? PolyCollection could maintain a list of [contiguous] segments, something along the lines of how "extents" work in the filesystem world? You could get 99% cache hits and only a few misses when jumping to the next segments. idk maybe this idea sucks xD.
I don't remember if this was commented in the review for future improvements, but it's an idea I wanted to share. If the internal sequence type to hold the collection of objects of the same type could be specified as an option, a deque-like sequence could allow non-movable and non-copyable types, at least when insertion position is not specified, and even stable references and pointers for such insertions.
What about mid deleteions?
A stable_vector based storage would allow even more operations for such types, but we would loose element contiguity.
Exactly, At that point, you're probably better off with Boost.PointerContainer performancewise. Joaquín M López Muñoz
On 04/10/2017 22:59, Joaquin M López Muñoz via Boost-users wrote:
If the internal sequence type to hold the collection of objects of the same type could be specified as an option, a deque-like sequence could allow non-movable and non-copyable types, at least when insertion position is not specified, and even stable references and pointers for such insertions.
What about mid deleteions?
Touché!
Den 04-10-2017 kl. 21:10 skrev d3fault via Boost-users:
Thanks for that workaround Joaquin M López Muñoz. It's interesting, but not quite what I'm looking for (want both move and copy constructors deleted).
Perhaps http://www.boost.org/doc/libs/1_64_0/libs/ptr_container/doc/reference.html can help. boost::ptr_vector should be one's default choice. kind regards Thorsten
El 05/10/2017 a las 10:08, Thorsten Ottosen via Boost-users escribió:
Den 04-10-2017 kl. 21:10 skrev d3fault via Boost-users:
Thanks for that workaround Joaquin M López Muñoz. It's interesting, but not quite what I'm looking for (want both move and copy constructors deleted).
Perhaps
http://www.boost.org/doc/libs/1_64_0/libs/ptr_container/doc/reference.html
can help. boost::ptr_vector should be one's default choice.
Absolutely :-) Thorsten, regarding Boost.PolyCollection I've given a thought or two to the request that copy/assignment of elements be externalized to a configurable traits class: http://www.boost.org/doc/html/poly_collection/future_work.html#poly_collecti... and I'm not sure how/whether this can be done. The typical copy interface of a virtual hierarchy is through some clone() member function like this: virtual base* clone()const; which is BTW what you adopt in Boost.PointerContainer. The problem with this is that clone() not only constructs the derived object but also allocates memory for it, which is against Boost.PolyCollection approach, where memory is allocated through the collection's allocator and passed to construction code. This implies the cloning interface should look like: // OOP member function virtual void construct(void *p)const; // p alocated/aligned OK for derived // As embodied in the copy traits void construct(void* p,cont derived& x){x.construct(p);} which doesn't seem like OOP usual practice. In particular, I don't see how clone() can be used (without changing the OOP code) to implement copy_traits::construct. Thoughts on this? Thank you, Joaquín M López Muñoz
Den 05-10-2017 kl. 13:30 skrev Joaquin M López Muñoz via Boost-users:
El 05/10/2017 a las 10:08, Thorsten Ottosen via Boost-users escribió:
Thorsten, regarding Boost.PolyCollection I've given a thought or two to the request that copy/assignment of elements be externalized to a configurable traits class:
http://www.boost.org/doc/html/poly_collection/future_work.html#poly_collecti...
and I'm not sure how/whether this can be done. The typical copy interface of a virtual hierarchy is through some clone() member function like this:
virtual base* clone()const;
which is BTW what you adopt in Boost.PointerContainer. The problem with this is that clone() not only constructs the derived object but also allocates memory for it, which is against Boost.PolyCollection approach, where memory is allocated through the collection's allocator and passed to construction code.
Right. I have thought about using the allocator to allocate all memory, but did not find it worth the trouble for ptr_container.
This implies the cloning interface should look like:
// OOP member function virtual void construct(void *p)const; // p alocated/aligned OK for derived
// As embodied in the copy traits void construct(void* p,cont derived& x){x.construct(p);}
which doesn't seem like OOP usual practice. In particular, I don't see how clone() can be used (without changing the OOP code) to implement copy_traits::construct. Thoughts on this?
Well, this is not trivial. First of, can we assume that the class hierarchy supports clone() in some form? If not, then you can use e.g. ptr_vector even without knowing the number of objects in advance, though you can't deep copy the container. And one can use PolyCollection as you outlined above with throwing move/copy operations (though here we need to know the size). So let's assume the hierarchy has some notion of clone(). Then it must have /protected/ copy-constructors (at least that would be the easiest and safest way to do it, possibly just using = default). If it has that, then there should be nothing wrong with having /protected/ move operations. Now if it has those copy/move operations internally, can't we just grant PolyCollection access to it? friend class boost::poly_collection::access; it needs to be defined in every class in hierarchy (except perhaps the root of the hierarchy), but so does the virtual function that clones (or constructs). I would rather do that compared to seeing stuff like void* in the class. There is a difference though: in PolyCollection reference stability can be broken when the segments resize or erases elements and not just when the /actual/ element is erased (deleted) ... this holds even for moves. So one has to be a little more careful of handing out base* objects to other pieces of code. So, returning to your actual question: can we do anything without changing/adding something to the hierarchy? I don't see how that could work either. Nor how an existing clone() would help (because it mixes memory and copy-construction as you said). There is no free lunch ... kind regards Thorsten
participants (1)
-
d3fault -
Ion Gaztañaga -
Joaquin M López Muñoz -
Steven Watanabe -
Thorsten Ottosen