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

Subject: Re: [boost] [review] Review of PolyCollection starts today (May 3rd)
From: Thorsten Ottosen (tottosen_at_[hidden])
Date: 2017-05-15 14:13:18

Den 15-05-2017 kl. 00:14 skrev Joaquin M López Muñoz via Boost:
> El 11/05/2017 a las 18:18, Thorsten Ottosen escribió:
> 
>  > Den 11-05-2017 kl. 09:53 skrev Joaquin M López Muñoz via Boost:
>  >>
>  >> El 10/05/2017 a las 22:33, Thorsten Ottosen via Boost escribió:
>  >>>
>  >>> D. Implementation of equality: do we need to be set like semantics
[snip]
> 
> Sorry, I'm not getting you. The current implementation enforces exact
> segment match.
> Could you maybe elaborate/reword your question?

My bad. Your code does what the docs say. You have this

  // TODO: can we do better than two passes?

By that, do you mean that the first pass is the size() comparison?

> 
>  >>> E. should the test check (perhaps via static_assert) the conditions
>  >>> under which move-construction and move-assignment is noexcept? (sorry

>  > I don't get it. [...]
> 
> My bad, I misread your question. Of course move construction and move
> assignment do not throw or copy elements etc., but I didn't mark them as
> noexcept following the no-noexcept signature of std unordered associative
> containers. I don't know why the std has not made those noexcept.

I guess some vendors wanted freedom, and then gave users the near 
impossible task of tracking no-except/except impacts through their code 
bases. Now, you use =default for all the move operations, so I think 
they are required to be deduced to be noexcept exactly when they can be. 
Though a static assertion in the tests can track any fault in that logic.

If one ever puts base_collection's into a container, you are in for a 
huge penalty when the container operation falls back on copying.

> 
>  >>> S. The test are testing best possible scenario; a more realistic test
>  >>> would be to copy pointers to an std::vector<base*>, shuffle it and run
>  >>> update on that.
>  >>
>  >> Sorry if I am not getting your question, but is this not the line
>  >> labeled "shuffled_ptr_vector"
>  >> in the plot?
>  >
>  > Yes, sort of, but I'm saying that very often one wants to copy
>  > pointers from objects in base_collection into std::vector<base*> to
>  > rearrange the elements somehow (say, your game entities need to be
>  > sorted wrt to distance from the avatar). I still expects
>  > base_collection + std::vector<base*> + shuffle to perform somewhat
>  > better than ptr_vector + shuffle, but it would be interesting to see.
> 
> Sorry again but I don't get it yet. I don't quite understand which 
> particular
> data structure you'd like me to compare base_collection against.

So take the processing test. You already compare against a shuffled 
ptr_vector<T>. So far so good. Now take a base_collection<T> as is also 
done. Then take the address of every element in the base collection and 
put them into a vector<T*>. Now shuffle this vector<T*>'s content like 
you did for ptr_vector. Then compare the processing time over a 
ptr_vector with the processing over the vector<T*>.

I claim this is a slightly more realistic test because one often wants 
to impose an order of some or all of the elements in the 
base_collection<T> before processing/iterating them somehow. So you 
won't always access the items in the order induced by the 
base_collection<T>. That's why I said that the current test is a 
best-case scenario, and there is no need to change that test. But I 
think it would be useful to see how much better your library is when 
access is not segment-sequential even though storage is.

kind regards

Thorsten