PolyCollection requiring copyable

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

PolyCollection requiring copyable

Boost - Users mailing list
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
_______________________________________________
Boost-users mailing list
[hidden email]
https://lists.boost.org/mailman/listinfo.cgi/boost-users
Reply | Threaded
Open this post in threaded view
|

Re: PolyCollection requiring copyable

Boost - Users mailing list
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

_______________________________________________
Boost-users mailing list
[hidden email]
https://lists.boost.org/mailman/listinfo.cgi/boost-users
Reply | Threaded
Open this post in threaded view
|

Re: PolyCollection requiring copyable

Boost - Users mailing list
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
_______________________________________________
Boost-users mailing list
[hidden email]
https://lists.boost.org/mailman/listinfo.cgi/boost-users
Reply | Threaded
Open this post in threaded view
|

Re: PolyCollection requiring copyable

Boost - Users mailing list
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
_______________________________________________
Boost-users mailing list
[hidden email]
https://lists.boost.org/mailman/listinfo.cgi/boost-users
Reply | Threaded
Open this post in threaded view
|

Re: PolyCollection requiring copyable

Boost - Users mailing list
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
_______________________________________________
Boost-users mailing list
[hidden email]
https://lists.boost.org/mailman/listinfo.cgi/boost-users
Reply | Threaded
Open this post in threaded view
|

Re: PolyCollection requiring copyable

Boost - Users mailing list
On 10/3/17, Steven Watanabe via Boost-users
<[hidden email]> 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 <[hidden email]> 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
_______________________________________________
Boost-users mailing list
[hidden email]
https://lists.boost.org/mailman/listinfo.cgi/boost-users
Reply | Threaded
Open this post in threaded view
|

Re: PolyCollection requiring copyable

Boost - Users mailing list
On 04/10/2017 21:10, d3fault via Boost-users wrote:

> On 10/3/17, Steven Watanabe via Boost-users
> <[hidden email]> 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
_______________________________________________
Boost-users mailing list
[hidden email]
https://lists.boost.org/mailman/listinfo.cgi/boost-users
Reply | Threaded
Open this post in threaded view
|

Re: PolyCollection requiring copyable

Boost - Users mailing list
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
>> <[hidden email]> 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
_______________________________________________
Boost-users mailing list
[hidden email]
https://lists.boost.org/mailman/listinfo.cgi/boost-users
Reply | Threaded
Open this post in threaded view
|

Re: PolyCollection requiring copyable

Boost - Users mailing list
In reply to this post by Boost - Users mailing list
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
_______________________________________________
Boost-users mailing list
[hidden email]
https://lists.boost.org/mailman/listinfo.cgi/boost-users
Reply | Threaded
Open this post in threaded view
|

Re: PolyCollection requiring copyable

Boost - Users mailing list
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_collection.future_work.copy_traits

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
_______________________________________________
Boost-users mailing list
[hidden email]
https://lists.boost.org/mailman/listinfo.cgi/boost-users
Reply | Threaded
Open this post in threaded view
|

Re: PolyCollection requiring copyable

Boost - Users mailing list
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_collection.future_work.copy_traits 
>
>
> 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



_______________________________________________
Boost-users mailing list
[hidden email]
https://lists.boost.org/mailman/listinfo.cgi/boost-users
Reply | Threaded
Open this post in threaded view
|

Re: PolyCollection requiring copyable

Boost - Users mailing list
In reply to this post by Boost - Users mailing list
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é!
_______________________________________________
Boost-users mailing list
[hidden email]
https://lists.boost.org/mailman/listinfo.cgi/boost-users